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

Allow Trigger Filter to filter on any Cloud Event Attribute (except data) #1610

Closed
vaikas-google opened this issue Jul 31, 2019 · 4 comments · Fixed by #1643

Comments

@vaikas-google
Copy link
Contributor

commented Jul 31, 2019

Problem
Currently in Knative you can filter Trigger events based on two fields of a Cloud Event:
Source
Type
As discussed in the #1381 we have decided to introduce a convention for defining an instance of an importer (or function) by adding an attribute to CloudEvent called ‘sourceid’ (working name). In order to support filtering based on that, we’ll need to modify the Trigger to be able to filter on that field.
Additionally, there are classes of applications that are built using a decorator pattern, where a function takes in an event and then decorates it (pre-processes it, adds some semantic meaning to it, etc.) and then sends it back to the broker. With the current model, the developer has to create a new event type rather than use the original event type in order to be able to create triggers to it.

Proposal

Currently the TriggerFilter looks like this:
type TriggerFilter struct { SourceAndType *TriggerFilterSourceAndTypejson:”sourceAndType,omitEmpty”}
We propose that we mark SourceAndType as deprecated, add a map that allows the user to specify any CloudEvent Context attribute field to match exactly the specified string. Excludes data attribute.

type TriggerFilter struct { // Deprecated SourceAndType *TriggerFilterSourceAndTypejson:”sourceAndType,omitEmpty”Attributes map[string]stringjson:"attributes,omitEmpty"}
The attributes map matches the event if all fields specified in the attributes map are present on the event, and all have equal values to their value on the event.

Persona:
Developer

Exit Criteria
Once a developer can create Triggers that filter based on any Cloud Event attribute (except data).

Time Estimate (optional):
1-2

Additional context (optional)
Add any other context about the feature request here.

@mikehelmick

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

Proposal LGTM

For the sourceId/importerid - that would still be auto populated by an objectRef to the importer.

we should make sure that attribute isn't specified in both places (objectRef + attributes map)

@grantr

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

For the sourceId/importerid - that would still be auto populated by an objectRef to the importer.

Seems like the design @mikehelmick mentions (filter by objectref to an importer object) makes the proposal in this PR (Trigger-specified attribute filters) unrelated functionality, because an objectref can't be an attribute filter.

@mikehelmick

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

@grantr - see this comment

My only comment here was that we wouldn't want someone using the objectRef for sourceid and then adding that same sourceid to the attribute map.

@grantr

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

Are you referring to mutual exclusion of filter types? In other words, will it be possible to specify multiple filter types, e.g. a Trigger with both importer and attributes filters?

spec:
  filter:
    importer:
      ref:
       apiVersion: sources.knative.dev/v1alpha1
       kind: AwesomeSource
       name: awesome-source-importer
    attributes:
      decorated: "true"

If this isn't possible, then we rely on importers to implement ALL filtering that a user might ever need, because the only option for an importer filter is "give me every event from this importer".

If this is possible, we need to be clear that filters are conjunctive. Maybe it's just me, but specifying multiple filters as a map looks confusing to me. It's hard to see where each filter begins and ends. A clearer way to specify this might be an array of oneofs:

spec:
  filters:
  - importer:
      ref:
       apiVersion: sources.knative.dev/v1alpha1
       kind: AwesomeSource
       name: awesome-source-importer
  - attributes:
      decorated: "true"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.