RFC : CDEvents Webhook Adapter design review

[rjalander]

Creating this PR, to review the current design approach and propose any changes that might help in creating a common cdevents-translator library.

Adding a page for Gerrit-CDEvents integration and mapping for various Gerrit events to CDEvents.

[rjalander]

Thank you @xibz for directing various approaches for the translator.

@afrittoli @xibz please let us know your views on these design proposals and review.

[rjalander]

This looks much better @rjalander! One thing I noticed missing was a final "Conclusions" to summarize which was preferred by you, etc. Or did you want to leave that up for discussion?

Thank you @xibz for reviewing this PR,

Yes I think we can discuss in CDEvents Working Group meeting and reach a conclusion to select the most suitable one

[e-backmark-ericsson]

Some general comments on this RFC:

• A basic principle in CDEvents should be that the events are sent by their source itself, or as close to that source as possible. I understand that it will be challenging to get all tools (SCM tools in this case mostly) to adapt sending CDEvents, so we might need something to fill that gap. At least shor/mid-term. It would be good to see some statement about that in the RFC.
• The RFC seems to propose a one-directional translation only, from XYZ to CDEvents. Are there ideas on translating events the other way around as well? I believe it would be a valid limitation to only support one-way translation, and thus that can be clearly stated in the introduction to the RFC. Translating from CDEvents to XYZ could require a much more complex set up, depending on what downstream systems/tools to support.
• Could/should we limit what interface protocols this translator would support towards the SCM (or other) systems? Most (all?) systems support webhooks, so maybe we should limit to that? Gerrit also supports their own 'stream-events' over ssh (and also Kafka through a plugin), but I don't think we need to support that as the webhooks support is there..
• I'd like to see a clearer scope of the intended solution. 99% of it talks about SCM related events. Could/should we limit this service to only manage SCM events, or should it be kept open for other types of CDEvents as well?
• Based on my reflections above, I'm not sure about the term 'cdevents-translator' and 'translations' here. I believe the service should have a name that reflect better the intended scope of it. Some suggestions: 'scm_event_producer', 'scm_adapter', 'any_to_cdevents_adapter', 'webhook_cdevents_producer'
• It will be easier to grasp the proposal if a simple picture was included, showing where services are deployed and their interfaces to the SCM system(s) and event brokers etc.
• A consumer of these events will eventually want to provide feedback on the source change in the SCM system, so it would be great to see that included in use cases and in a schematic picture of this proposal. I don't expect this service itself to manage that feedback, or is that considered as an option as well?

Bottom line: I propose that this initiative is limited to manage SCM related events only, and that it is one-directional. The name should preferably in some way reflect that.

[rjalander]

Some general comments on this RFC:

• A basic principle in CDEvents should be that the events are sent by their source itself, or as close to that source as possible. I understand that it will be challenging to get all tools (SCM tools in this case mostly) to adapt sending CDEvents, so we might need something to fill that gap. At least shor/mid-term. It would be good to see some statement about that in the RFC.
• The RFC seems to propose a one-directional translation only, from XYZ to CDEvents. Are there ideas on translating events the other way around as well? I believe it would be a valid limitation to only support one-way translation, and thus that can be clearly stated in the introduction to the RFC. Translating from CDEvents to XYZ could require a much more complex set up, depending on what downstream systems/tools to support.
• Could/should we limit what interface protocols this translator would support towards the SCM (or other) systems? Most (all?) systems support webhooks, so maybe we should limit to that? Gerrit also supports their own 'stream-events' over ssh (and also Kafka through a plugin), but I don't think we need to support that as the webhooks support is there..
• I'd like to see a clearer scope of the intended solution. 99% of it talks about SCM related events. Could/should we limit this service to only manage SCM events, or should it be kept open for other types of CDEvents as well?
• Based on my reflections above, I'm not sure about the term 'cdevents-translator' and 'translations' here. I believe the service should have a name that reflect better the intended scope of it. Some suggestions: 'scm_event_producer', 'scm_adapter', 'any_to_cdevents_adapter', 'webhook_cdevents_producer'
• It will be easier to grasp the proposal if a simple picture was included, showing where services are deployed and their interfaces to the SCM system(s) and event brokers etc.
• A consumer of these events will eventually want to provide feedback on the source change in the SCM system, so it would be great to see that included in use cases and in a schematic picture of this proposal. I don't expect this service itself to manage that feedback, or is that considered as an option as well?

Bottom line: I propose that this initiative is limited to manage SCM related events only, and that it is one-directional. The name should preferably in some way reflect that.

@e-backmark-ericsson Thank you very much for your comments,

• Updated the design including the first 4 comments as part of Goals, Non-Goals and Motivation and Rationale
• And about the name change we discussed in the CDEvents Workgroup meeting yesterday, is that fine to change as webhook_cdevents_translator or webhook_cdevents_adapter, please comment.
• Will try to add a picture for this proposal as suggested
• I think a feedback from the consumer will be provided back to the message-broker with an another CDEvent, but the event will not be translated/send to SCM system, as stated above this translator supports one-way translation.

[rjalander]

@e-backmark-ericsson @afrittoli @xibz Thank you all for reviewing and providing your comments. Please approve this RFC If no other comments.

[rjalander]

Yes I requested @afrittoli to create project under cdevents org with the name "webhook-cdevents-adapter" (replaced _ with -), just to follow other projects naming conventions under cdevents org

And about using translator, referring plugin implementation projects for various SCM tools as <scm_tool_name>-translator-cdevents(can be a separate project under different org.)

Ex: gerrit-translator-cdevents and the interface name EventTranslator.TranslateEvent() to be implemented.

Please suggest, If I can rename this too with gerrit-adaptor-cdevents and the interface name EventAdaptor.TranslateEvent() to implement ?

[rjalander]

Yes I requested @afrittoli to create project under cdevents org with the name "webhook-cdevents-adapter" (replaced _ with -), just to follow other projects naming conventions under cdevents org

And about using translator, referring plugin implementation projects for various SCM tools as <scm_tool_name>-translator-cdevents(can be a separate project under different org.)

Ex: gerrit-translator-cdevents and the interface name EventTranslator.TranslateEvent() to be implemented.

Please suggest, If I can rename this too with gerrit-adaptor-cdevents and the interface name EventAdaptor.TranslateEvent() to implement ?

As discussed in one of the SIG event meets, keeping the plugins name with translator word and using adapter for CDEvents webhook.

[rjalander]

@afrittoli @xibz can you please re-review and approve If no other comments with this RFC.

[rjalander]

Can we merge this PR If no other comments. Thank you.