Skip to content
This repository has been archived by the owner on Dec 11, 2023. It is now read-only.

Heads-up: potential issue with use of "source" in filters? #74

Open
jmcx opened this issue Oct 14, 2022 · 5 comments
Open

Heads-up: potential issue with use of "source" in filters? #74

jmcx opened this issue Oct 14, 2022 · 5 comments
Assignees
Labels

Comments

@jmcx
Copy link

jmcx commented Oct 14, 2022

I'm calling this a heads-up because I'd like to finish the proposal for event registry to properly explain.

But the basic idea is I think we need to clarify what an event is from the user's perspective (and document it in our docs etc) and am thinking we could more explicitly align with CloudEvents, and include the event's source and type as attributes the user knows about and can manipulate.

Current behaviour
Currently, the CLI seems to match the value of source with the name of the source component, so if I create a source called foo-webhooksource, then I can create a Trigger with --source foo-webhooksource.

Potential issue
To illustrate, does this fall apart when I'm using a CloudEvents source? With CloudEvents source, I can define the type and source of my event at runtime as an attribute in the context metadata, e.g. type = mycustomtypeand source = mycustomruntimesource. And then I might want to filter on this source value in a Trigger, not the name of the CloudEventsSource component I created. E.g.:

% tmcli create source cloudevents --name foo-cloudeventssource
% tmcli create target cloudevents --name foo-cloudeventstarget --endpoint https://env1minz9xsqi.x.pipedream.net
% tmcli create trigger --target foo-cloudeventstarget --sources mycustomruntimesource
2022/10/14 10:08:28 event types filter: event producer "mycustomruntimesource" is not available

BTW I still get an issue when trying to use the source component name:

% tmcli create trigger --target foo-cloudeventstarget --sources foo-cloudeventssource
2022/10/14 11:17:50 event types filter: "foo-cloudeventssource" event types: event types: unexpected end of JSON input

So essentially, maybe source is not the name of the source component, but something that more generally describes what external system the event came from.

So as a user, if I start working with a Broker locally and I know there are events coming from Salesforce, I can lookup the value of source for events coming from Salesforce in the Registry. This would be independent of the name of the SalesforceSource component being used. That means my definitions of Triggers and Rules aren't coupled to component names, but to Event types and schemas, events being the "contract" between producer and consumer.

In terms of the "default events schemas" that TriggerMesh knows about (e.g. that Fran is adding in the CRDs), we could also hardcode a value for source when it makes sense, alongside the type and description and payload schema as he has started doing. So e.g. for SQS, we could have the following predefined schema:

{
  "type": "com.amazon.sqs.message",
  "source": "com.amazon.sqs",
  "schema": "https://raw.githubusercontent.com/triggermesh/triggermesh/main/schemas/com.amazon.sqs.message.json",
  "description": ""
}
@jmcx
Copy link
Author

jmcx commented Oct 14, 2022

Next step: I need to better structure these ideas in a proposal we can discuss as a group (in progress).

@jmcx
Copy link
Author

jmcx commented Oct 14, 2022

I wonder if Timur this is kind of what you're pointing to in this issue: triggermesh/triggermesh#1145
that you need "internal" attributes (not user facing) to implement internal logic, without necessarily using the user-facing attributes like source and type for this?

@tzununbekov
Copy link
Member

triggermesh/triggermesh#1145 suggests a couple of event dispatcher updates that would definitely extend our abilities to control event attributes, and possibly improve routing options. But I haven't thought thoroughly about how exactly we could employ these attributes yet.

Regarding the rest, I'm not 100% sure that I understand the idea, let me try to clarify it:
Currently, tmcli create target * commands accept optional --source argument, which is a name reference to another component. You're worried that --source argument may be confusing because the CloudEvent structure also has source attribute and those obviously have different values/meanings? If this is the case, does the replacement --source argument with something like --producer would make it better?
I got a bit lost in the part with the registry, could you provide some examples of how the user experience may look like?
Also, I don't know if you saw that, but CLI has an alternative to the --source argument called --eventTypes which works with the type attribute of CloudEvents produced by our components. I feel like this is similar to what you meant in this issue but for source CE attribute.

@jmcx
Copy link
Author

jmcx commented Oct 14, 2022

Yes, that was my concern, you've made it clear that a CloudEvent source is not the same thing as this source CLI argument 👍

What I'm thinking for the registry is that something like tmcli list schemas could show both type and source for the available event schemas, and could reveal the full schema as well if you want.

Then, I would use those values in my Trigger filters: source = this-source & type = that-type.

So having a different name to reference the source component could certainly help disambiguate. I don't have an opinion right now on what the best alternative name would be though.

@jmcx jmcx self-assigned this May 25, 2023
@jmcx jmcx added the p2 label May 26, 2023
@jmcx
Copy link
Author

jmcx commented May 26, 2023

After a quick re-think on this, long term I still think CloudEvents and their parameters (source, type, id, time, extensions, ...) are/should be first class concepts that the user manipulates and could use in the definition of a trigger filter from tmctl. This is possible today, but there is a conflict because I can do something like this:

tmctl create trigger --target tm-cloudeventstarget --source tm-cloudeventssource --filter '{"prefix": {"source": "com.mycompany.myapp.myproject"}}'

where the --source parameter does not mean exactly the same thing as the source used in the filter, the former is the name of the TriggerMesh source component instance, the latter is the value of the CloudEvent's source attribute, which may or may not have some correlation with the name of the component instance.

Possible paths are:

  1. Repurpose the --source parameter in the CLI, so that it actually refers to the value of the CloudEvent source attribute (not the name of a TriggerMesh component). We don't document the value of CE source well today, and it doesn't contain consistent and useful values in the events we produce, so this solution would require more work to get those values cleaned up and documented well.
  2. Remove the --source parameter in the CLI, only rely on explicit CloudEvent attributes for filtering. Would provide the same concept as when using TriggerMesh in K8s
  3. Rename the source parameter to something that doesn't overlap with CE source, for example source-component

Low priority right now IMO but worth keeping open.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants