-
Notifications
You must be signed in to change notification settings - Fork 584
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
Add some guidance on how to construct CloudEvents #404
Conversation
@@ -248,6 +248,60 @@ in the serialization for unkown, or even new, properties. It was also | |||
noted that the HTTP specification is now following a similar pattern by | |||
no longer suggesting that extension HTTP headers be prefixed with `X-`. | |||
|
|||
## Creating CloudEvents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are attempting to write in guidance for namespace shadowing on event types per our conversation during the sdk call.
I strongly believe that it is bad practice to create an event type that corresponds to what the event producer would like to use if they produced cloudevents natively.
For context, this comes from a disagreement that Knative project Event Sources are doing where an event originates from GitHub, GitHub sends the event payload via webhook to an application running in the cluster local. That application marshals the request into a struct (using a lib that is not owned by GitHub) and then takes the GitHub event type and adds it as a post-fix like dev.knative.eventing.{GitHubEvent}
.
Doug believes we should use com.github.{GitHubEvent}
. The issue is that if GitHub ever decides to produce cloudevents, there could be events in the cluster that match in terms of Source + EventType + ID but do not match in terms of implementation choice for the data payload. GitHub is a good example of a complicated event struct, and they mix event types into an over shadowed struct, plus auth components in the headers. They could choose to implement their payload differently for easier consumption or routing.
My argument is the source application that runs in the cluster is not just a simple proxy, it is a full micro-service with logic and choices. We get to drop some of the auth headers because we validate them, we may not forward the payload correctly if GitHub adds or removes parts of their API, they don't control the lib we use...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, that's not quite what I'm advocating. I'm not suggesting the CE generator try to co-opt the event producer's namespace, rather I think the CE generator should reuse data coming from the event producer rather than inventing data that makes it look like the CE is from the CE generator rather than the original event producer.
So, I'm not suggesting to use com.github.push
, rather it should be just push
if that is what github lists the event types as. If I mentioned com.github.xxx
in previous chats it was just as an example of how it should not be something with knative
in it because consumers don't care if knative is in the flow - I was not formally saying it should start with com.github.
.
While in the Kn case it isn't just a 'proxy' because there is some logic involved in generating the CE attributes, it is much closer to a proxy than it is an "event producer" because 1) it doesn't materially change the data
or shouldn't, and 2) it is generating CE attributes for the purpose of spec compliance, meaning it needs to fill it in with something, and not doing so to convey that this Kn component is in the flow. IOW, the consumer doesn't not, and should not, care that Kn is involved w.r.t. the data it consumes. The app is subscribing to and wants to process Github events, not Kn events.
Since not all event producers will generate CloudEvents natively, it might be good to provide some guidance around how the CE attributes should be populated when the CE producer isn't the event producer. Signed-off-by: Doug Davis <dug@us.ibm.com>
Rebased this one, given our recent talks I think this alignes with @deissnerk's PR #420 |
Signed-off-by: Doug Davis <dug@us.ibm.com>
Signed-off-by: Doug Davis <dug@us.ibm.com>
Made some minor updates based on the new definitions of source and producer. PTAL. |
Since not all event producers will generate CloudEvents natively, it might
be good to provide some guidance around how the CE attributes should be
populated when the CE producer isn't the event producer.
Signed-off-by: Doug Davis dug@us.ibm.com