Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split filters into read and write branches in configuration. #637

Open
XAMPPRocky opened this issue Oct 31, 2022 · 1 comment
Open

Split filters into read and write branches in configuration. #637

XAMPPRocky opened this issue Oct 31, 2022 · 1 comment
Labels
kind/feature New feature or request

Comments

@XAMPPRocky
Copy link
Collaborator

As I've been writing more complex filtering schemas the current single stream has struggle scaling. One example, is I have a protocol that goes through nine filters (through capturing, matching, and then using in something like TokenRouter) when reading packets from downstream, but only goes through one filter when writing data to downstream. This makes hard to see and reason with (should the write filter be at the top or bottom or in the middle, given that neither filters relate to each other and any position would be valid). This also simplifies filter configuration, as filters will no longer have to remember both their "read" version and "write" version of their config, instead they can have a single configuration that works for both cases.

I think we should split it into filters.read and filters.write to make it more clear what "stream" you're filtering at a given moment.

filters:
  read:
      - name: quilkin.filters.capture.v1alpha1.Capture
        config:
          metadataKey: quilkin.dev/load_balancer/version
          suffix:
            size: 1
            remove: true
  write:
      - name: quilkin.filters.concatenate_bytes.v1alpha1.ConcatenateBytes
        config:
          strategy: Append
          bytes: [0, 0, 5, 5]
@XAMPPRocky XAMPPRocky added the kind/feature New feature or request label Oct 31, 2022
@markmandel
Copy link
Contributor

I can definitely see the value - above it far more explicit and easier to follow.

Some thoughts that might impact the design:

  1. When filters register, I figure they should be able to register as read/write or both - that way it can be validated on which position they can be added (i.e. I can't add a Rate Limiter to write, since it's not supported there).
  2. Some filters have on_read and on_write configurations (Compress, ConcatenateBytes, Firewall, etc), I think in almost all circumstances the config for each is the same (we should double check, and also, do we allow different configurations on read vs write?), but we'll need to have a way to pass to the Filter whether it's in read mode or write mode with the configuration data.
  3. if I use a Filter such as Match for both on_read and on_write in the current incarnation, I only get one instance of the Match filter. In the new format, I assume we would be creating two instances of the Filter? (so likely two separate FilterChains? or maybe we merge everything into one internally?) - not the end of the world, but slightly more Filter instances per Quilkin instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants