camaraproject / Commonalities

Repository to describe, develop, document and test the common guidelines and assets for CAMARA APIs
Apache License 2.0
9 stars 24 forks source link

x-correlator in CloudsEvent notification? #164

Closed bigludo7 closed 2 months ago

bigludo7 commented 4 months ago

Problem description We took the decision to add x-correlator in our API which is fine but I have a doubt for notification.

Do we have to add the x-correlator as well in the POST {$request.body#/webhook/notificationUrl}? This is the notification send to the listener.

In order to stay fully compliant with CloudsEvents (I did not find any mention x-correlator in CloudsEvents) I tend to think that we must not have this x-correlator in the notification response & endpoint (for CloudsEvent compliance and to get it easy for the developer)

Expected behavior

Add this specific comment in the guidelines

Alternative solution

Additional context

PedroDiez commented 4 months ago

From Telefonica view, the indication of the support of x-correlator header in the POST notification sent to the listener is good to be reflected in the APIs. Indeed we consistently can incorporate as well in https://github.com/camaraproject/Commonalities/blob/main/artifacts/notification-as-cloud-event.yaml.

In CloudEvents spec there is no reference to x-correlator header (as well as other HTTP headers), because it is focused in the metadata model for the event not in the use of the underlying HTTP protocol.

The indication of the support of the x-correlator header provides alignment within the CAMARA APIs Architecture:

Therefore, indicating it within the POST notification, we are not adding something additional but just reflecting the possible use of this header within this communication flow, with the purpose given to this header

bigludo7 commented 4 months ago

Thanks @PedroDiez Got your points - we did not have any drawback to have the x-correlator in the notification as well, this is not inconsistent with CloudEvents so better to keep it. Works for me. @shilpa-padgaonkar @rartych as you were a lot involved in subscriptions & cloudEvents discussions no blocker for you to have the x-correlator in the notification yaml?

shilpa-padgaonkar commented 4 months ago

Fine from my side.

hdamker commented 3 months ago

Therefore, indicating it within the POST notification, we are not adding something additional but just reflecting the possible use of this header within this communication flow, with the purpose given to this header

@PedroDiez @bigludo7 I have here a slightly different view: If we are indicating the X-Correlator header within the POST notification, it gets mandatory for the API Consumer (the party receiving the POST notification) to implement the logic for the X-Correlator. It is not optional for them. The question is if we want to add this additional effort to the API consumer. Until now it was optional for the API consumer to use (the non-standard) X-Correlator header.

Also on the API Provider side: if we indicate that there could be an X-Correlator header in the POST Notification and the API Consumer has to implement the logic anyway, shouldn't it be then maybe mandatory for an API Provider to send an UUID to be consistent across API implementations?

BTW: X-Correllator seem to be the correct according to the Guideline, not x-correlator

patrice-conil commented 3 months ago

@PedroDiez @bigludo7 @hdamker I'm not sure I fully understand the use case. Do we want to correlate notifications with their subscription (inject the X-Correllator provided by the subscriber when requesting a subscription) or correlate the notification with its response? In the case of the CloudEvent, if it is indeed a question of associating all the events with the subscription, then it seems more judicious for the subscriber to manage it by the URL of the subscription. What do you think?

PedroDiez commented 3 months ago

Hi,

@hdamker Understood you point, and from our view the fact that an API Consumer indicates back the X-Correlator in the response when indicated in request by API Server is not a big effort, and the indication of it in the request is not mandatory for the API Server. At the end is the same commitment as in the other way, if an API Consumer indicates it in the request it is mandatory for the API Server to send back in the response.

So, even it has some impact in API Consumers, it is needed to be able to perform the correlation in these kind of flows and give value to the purpose of this header.

@patrice-conil It refers to correlate the notification with its response

PedroDiez commented 3 months ago

As per commented in Commonalities Meeting 25th March, for the notifications case, the indication fo the correlator by the API Consumer will be indicated as recommended behaviour, and indicated properly in issued PR