Document how to adopt and adapt CDEvents #77
Conversation
Document a series of scenarios about adopting CDEvents producer and consumer side. Describe how to adapt CDEvents with customData and how to contribute new fields and events to the community. Document declarative vs. imperative events and the subscriber model. Fixes: cdevents#66 Fixes: cdevents#42 Signed-off-by: Andrea Frittoli <[email protected]>
afrittoli
force-pushed
the
adopting_cdevents
branch
from
36f3d67 to
71d1feb
Compare
There was a problem hiding this comment.
@afrittoli Awesome job on this! I really like the level of detail of each section. I had several comments, but this could be due to not understanding the full picture for the particular implementation.
|
|
||
| We plan to extend the data model to support more complex scenarios in upcoming | ||
| releases. | ||
| A behavior similar to that of imperative events can be achieved by moving part |
There was a problem hiding this comment.
Imperative events could also be achieved with point to point, right. Maybe this is a good use case for it? If it is, we may want to call that out, while explaining most events should be handled with a message bus, but in this particular use case, it does make sense for p2p. At least multiple ways to skin this cat, and the benefit with going to p2p here is they dont need to move any code anywhere, so may be easier approach?
There was a problem hiding this comment.
Yes, I tend to associate imperative with p2p and API calls as well, however there are systems - like Keptn - that do imperative via events. In Keptn one defines the sequence of events like build test deploy and Keptn orchestrates that via events. It sends a "do a build" event, or "do a deployment" event, and one or more systems may take the job and do it an report back through a the original context ID provided by Keptn.
There was a problem hiding this comment.
Ah, right, Temporal also behaves this way except a little more vaguely. Okay, cool, I think we should also highlight imperative being a valid use case for p2p since we call out not doing it, but it may make sense for this particular instance.
There was a problem hiding this comment.
ok, I will expand on this one.
What we discussed in the past is that we may include imperative events in future as a dedicated subgroup of the spec, but that may be something for the next release - if someone takes ownership of specifying them.
primer.md
Outdated
|
|
||
|  | ||
|
|
||
| This is approach is certainly valid to build a proof-of-concept or experiment |
There was a problem hiding this comment.
I dont know how I feel about this approach. Maybe if I had some usecases, but what doesn't make much sense to me is users would need to log the necessary information to even have it ingested and then have this agent produce the events. If users are already changing these logs, why not just use the SDK to produce the events directly?
There was a problem hiding this comment.
These scenarios are mostly provided as interim steps towards native support of CDEvents.
If I receive events from a source that I don't control, I can use an adapter to convert them to CDEvents. Once the source can send CDEvents I won't need the adapter any longer.
If I'm building a POC, rather than changing the tool code and adding eventing capabilities, I can watch an output and send events based on that.
In some cases, it can be used as a long-term strategy. In Tekton for instance we are building a cloudevent / cdevents controller that watches Tekton resources and sends events based on their condition changes. This offloads the job of sending events from the core controller and allows the development of a reacher event-specific feature set.
Other examples could be simply CLI tools. For those I would not add eventing capabilities, but I could send the event directly from a shell script that executes the CLI tool.
I hope this clarifies a bit, but happy to discuss further on this.
There was a problem hiding this comment.
Hmm, yea, so the only issue that I have with this is it seems a little hand wavy to me, because in order for an adapter to work some context is needed from the application, still, you can't just send the same payload, and expect the adapter to know what/how to make a CdEvent from it, unless you had a unique adapter for every user that knows implicitly how to deal with the payloads, and if users are building this adapter, at that point I'd say just use CdEvents directly. I think that's more of what I am getting at. Then, if there are plans to have a single adapter, having some context is needed, which again would require the user's service to change to send the necessary context. So I am just trying to understand how do you envision this being an interim step, because those are the only ways I can see an adapter working, unless I completely missed something. Which is totally possible!
There was a problem hiding this comment.
You can have an adapter that is flexible and let you define rules for every user.
The CDEvents data model is still rather thin, so it's not so difficult to find the required data already in there.
The CDEventer tool that I created for the DORA PoC works together with Tekton to adapt existing events and webhook into CDEvent - checkout the architecture diagram.
In the POC I used it to adapt events from Gitea, from Azure container registry and from the Knative API source, and I can do that by mapping input values into CDEvents fields.
Ultimately I would like Gitea and other SCM systems to produce CDEvents directly. The same goes for various container registries and other sources of events, but this worked well as an interim solution for the PoC.
I tried using docker hub webhooks and they only provided me with the image tag (no sha) so it didn't work for the POC. This is an example of why having the CDEvents data model at the source is better of course, for the POC I had to pick a different container registry.
|
|
||
|  | ||
|
|
||
| Similar to the [previous case](#external-event-producer), incoming events may be |
There was a problem hiding this comment.
Similar to the comment above, wouldn't the adapter need to have some context to what the event is? Why couldn't this just be a handler injected in the HTTP client's instead? It doesn't need to be external, from what I am gathering.
There was a problem hiding this comment.
The lack of context is the main gotcha with this kind of external adapters approaches, I should do a better job of expressing that in the doc. Similar to the case above, if one does not control the source, adapting is a good interim solution.
I can adapt GitHub webhooks to CDEvents for instance.
primer.md
Outdated
|
|
||
| #### Multiple event formats through adapter | ||
|
|
||
| In a variation of the previous two producer scenarios, the tool produces only |
There was a problem hiding this comment.
This makes it sound like it needs to be internal, but like above, we could have a HTTP handler that lives inside the service deal with this right?
There was a problem hiding this comment.
The adapter can leave in many different places, inside the producer, in front of the producer, it can be in the "middle", subscribe to events and resubmit them for whoever is listening (like in this scenario).
Where it sits depends on the kind of control we have on the event producers and consumers, and other factors too. For instance, the adapter could be a single component that receives N kinds of events and produces CDEvents out of each.
There was a problem hiding this comment.
Right, I guess having it a little ambiguous doesn't hurt, but I think the docs makes it seem like this adapter is a separate thing, especially from the diagrams. So I just dont want users to get the wrong idea.
There was a problem hiding this comment.
Heh, diagrams did not intend to be ambiguous - when the adapter is a box on its own it is separate from producer / consumer.
|
Page rendered with images available at https://github.com/afrittoli/cdevents-spec/blob/adopting_cdevents/primer.md |
primer.md
Outdated
| implement producer specific logic and sacrifice part of the interoperability | ||
| benefit of using CDEvents. | ||
|
|
||
| Adding a new field to existing an existing CDEvent type is a backward compatible |
There was a problem hiding this comment.
I guess this depends on who's perspective we take? Adding a mandatory field to a CDEvent wouldn't be backwards compatible for the senders, right?
For the receivers, having this backwards compatible implies that whatever strategy they use for "parsing" the CDEvent (from for instance JSON) must ignore keys it doesn't recognize. If this is a requirement on receivers, maybe we can state it somewhere?
There was a problem hiding this comment.
Yes, I think we should expand the statement saying that it is backwards compatible from consumer point of view, and assuming that consumers neglect additional fields that they don't understand.
There was a problem hiding this comment.
ok, I could add that. We do specify all this in the versioning section linked above and I was trying to provide just a quick mention to avoid repeating the whole backward compatibility definition.
There was a problem hiding this comment.
It could be enough to add a link here to the versioning section, so we don't duplicate information unnecessarily
primer.md
Outdated
| implement producer specific logic and sacrifice part of the interoperability | ||
| benefit of using CDEvents. | ||
|
|
||
| Adding a new field to existing an existing CDEvent type is a backward compatible |
There was a problem hiding this comment.
Yes, I think we should expand the statement saying that it is backwards compatible from consumer point of view, and assuming that consumers neglect additional fields that they don't understand.
Signed-off-by: Andrea Frittoli <[email protected]>
|
During the last WG, we agreed to merge the PR as it is now. |
Yea, I think you got most of what I wanted. Been super busy with prepping for Spinnaker Summit! But yea, Im sure this is going to evolve incrementally either way :) |
Changes
Document a series of scenarios about adopting CDEvents producer and consumer side. Describe how to adapt CDEvents with customData and how to contribute new fields and events to the community.
Document declarative vs. imperative events and the subscriber model.
Fixes: #66
Fixes: #42
Signed-off-by: Andrea Frittoli [email protected]
Submitter Checklist
As the author of this PR, please check off the items in this checklist: