Define CAMARA Event using cloudevents in OAS notification

[patrice-conil]

What type of PR is this?

• enhancement/feature

What this PR does / why we need it:

This PR increase interoperability between our ecosystem and rest of world using a defacto standard

Which issue(s) this PR fixes:

Fixes #8

Special notes for reviewers:

BREAKING CHANGES:

• Impacts APIs that define Event notifications (QoD, device status, ...)
• Need to update guidelines to integrate cloudevents references.

Changelog input

Integrates the cloudevents specification as the basis for CAMARA notifications

Additional documentation

[rartych]

Thank you @patrice-conil for this input, it is good starting point to evaluate Cloudevents (CE) for event notification format.
From the quick look I have found some CE parameters that need to be discussed and decided about CAMARA defined content (based on Cloudevents specification ):

source

• Type: URI-reference
• An absolute URI is RECOMMENDED
  Should it reference general CAMARA Project, specific API or specific Provider Implementation?

type

• Type: String
• SHOULD be prefixed with a reverse-DNS name. The prefixed domain dictates the organization which defines the semantics of this event type.
  Should it be something like org.camaraproject.api.qod or com.orange.camara.qod ?

subject

• Type: String
• OPTIONAL; If present, MUST be a non-empty string
• Description: This describes the subject of the event in the context of the event producer (identified by source).
  Can we here specify the exact event type like QOS_STATUS_CHANGED ?

Let's wait for evaluation and feedback from subprojects interested in Event notification.

[patrice-conil]

Hi @rartych,
Thanks for your questions.
Thanks to @shilpa-padgaonkar for pointing this spec and @hdamker for insisting on reviewing it.
I probably missed some subtleties in the specs which are very rich, so don't hesitate to correct me.
I hope we will have many comments to enrich our OAS specification.

Thank you @patrice-conil for this input, it is good starting point to evaluate Cloudevents (CE) for event notification format. From the quick look I have found some CE parameters that need to be discussed and decided about CAMARA defined content (based on Cloudevents specification ):

source

* Type: `URI-reference`

* An absolute URI is RECOMMENDED
  _Should it reference general CAMARA Project, specific API or specific Provider Implementation?_

I think it should refer to the specific vendor implementation, i.e. the context in which the fact occurred

type

* Type: String

* SHOULD be prefixed with a reverse-DNS name. The prefixed domain dictates the organization which defines the semantics of this event type.
  _Should it be something like org.camaraproject.api.qod or com.orange.camara.qod ?_

I think reverse DNS here is used to include package name with type to have fully qualified class name of the event type. Of course to have same reference for same type in all implementations we should use something like org.camara.api.qod.QosStatusChangedEvent

subject

* Type: String

* OPTIONAL; If present, MUST be a non-empty string

* Description: This describes the subject of the event in the context of the event producer (identified by source).
  _Can we here specify the exact event type like QOS_STATUS_CHANGED ?_

It's open to discussion.

Let's wait for evaluation and feedback from subprojects interested in Event notification.

[akoshunyadi]

Shouldn't the name contain 'cloudevent'? Currently the name is general but the description is cloudevent-specific

[akoshunyadi]

I think we should use, as in request example, a fully qualified class name as type. The cloudevents spec also suggests to use a version.

[patrice-conil]

done

[akoshunyadi]

I think we will also need the property dataschema, otherwise how could the consumer process the data?

[rartych]

dataschema is optional attribute, so it is not needed to send it with events.
Of course the schema for the data attribute needs to be defined - in API Design Guidelines or as a separate artifact to be available for both API and application developers, but not necessarily under specific URI.

[patrice-conil]

Hi @rartych, @akoshunyadi,
If type contains fully qualified class name we can use it as discriminator and apply inheritance rules.

[akoshunyadi]

@patrice-conil yes, but this spec doesn't contain any specific event details. And if we add a cloudevents subscription API that one is also generic. So we need a place for the definition of specific events, which we can do with inheritance from CloudEvent.

[patrice-conil]

Hi @akoshunyadi,
The idea is to provide a definition of EventDetails in each API (perhaps extend this specification in each API that provides Eventing?).
Perhaps we will also consolidate/aggregate all the definitions provided by the APIs into this specification.

[rartych]

The CloudEvent example should be aligned with the final decisions on the content of each attribute in PR #56

[PedroDiez]

Same coment as: #41 (comment)

[patrice-conil]

Done

[PedroDiez]

imho, I would set a name "transparent" to the underlying model

[PedroDiez]

In order to align with paths design, think we need to rename to:

POST /your_webhook_notificationUrl

[patrice-conil]

Done

[PedroDiez]

When agreed (just doubt):

• Do we set initially to v0.5.0 (removing wip), doesn't?

[patrice-conil]

Set to 0.1.0

[rartych]

Closing backtick for URI-reference

Suggested change

	description: "Identifies the context in which an event happened - be a non-empty `URI-reference."
	description: "Identifies the context in which an event happened - be a non-empty `URI-reference`."

[patrice-conil]

Done

[rartych]

Example of Absolute URI in source attribute:

Suggested change

	source: "com.orange.camara.qod"
	source: https://notificationSendServer12.supertelco.com

[patrice-conil]

Done

[jlurien]

To make the template even more useful, I would add also a sample schema for children events something like:

    ChildEventWithData:
      allOf:
        - $ref: "#/components/schemas/CloudEvent"
        - type: object
          properties:
            data:
              type: object
              description: Event details depending on the event type
              properties: {}
              required: {}
          required:
            - data

[jlurien]

We should also add a discriminator explicitly to this base class schema

      discriminator:
        propertyName: type
        mapping:
          CHILD_EVENT_NAME: "#/components/schemas/ChildEventWithData"
`` 

[jlurien]

There is a discussion in camaraproject/QualityOnDemand#224 (comment), about whether 1) to define data in the base class, as optional and with a generic type: object, or 2) to define data only in the child events which has data. Both would be correct, but not sure about which one would work better with code generators.

[rartych]

I do not have experience with code generators, but currently we do not have any example of subproject using events without data object. So for simplicity I prefer approach 1) where we do not have to define child object ChildEventWithData in each subproject using eventing, but data is in base class as optional.

[jlurien]

Indeed for that reason we have to define "a ChildEventWithData" schema, including data for each Child event. The name is just a templated one, real names we already have, among others:

QoD:

 EventQosStatusChanged:
      allOf:
        - $ref: "#/components/schemas/CloudEvent"
        - type: object
          properties:
            data: {}

DeviceStatus:

    EventRoamingStatus:
      description: event structure for roaming status change
      allOf:
        - $ref: "#/components/schemas/CloudEvent"
        - type: object
          properties:
            data:
              $ref: "#/components/schemas/RoamingStatus"

    EventRoamingOn:
      description: event structure for roaming on change
      allOf:
        - $ref: "#/components/schemas/CloudEvent"
        - type: object
          properties:
            data:
              $ref: "#/components/schemas/BasicDeviceEventData"

so we have to define the data per child event mandatorily always. The question was whether we should have to define it also in the base schema as optional.

[patrice-conil]

@jlurien, I agree with you, from an OO perspective we don't need to define it in the base schema.
I'm undecided because optional data field can also be considered a placeholder for what to extend in child classes.
But as you are the first that use this template in QoD and don't find it so useful, I will remove it from base class.
@rartych, both versions works with code generators.
I think defining ChildEventWithData add unnecessary layer in model, I just added a comment in CloudEvent description. WDYT about?

[jlurien]

So far, it is kept as optional in the CloudEvent base class. I think this is what get more consensus. If both work well with code generators, they are totally equivalent and the final schema once the composition is applied is the same.

[rartych]

I am fine with both approaches. API Design Guidelines are "normative" for CAMARA, here we provide example that can be used by subprojects and API Consumers, but other definitions can be used if they conform to API Design Guidelines and CloudEvents specification.

[PedroDiez]

Please see: #43 (comment)

[jlurien]

I think this is correct but I think someone complained about lack of support by certain code generators. We could stick to just application/json if it is safer

[rartych]

According to CloudEvents discussion format of "+json" is defined in RFC6839, so it should be supported by code generators.

I propose to follow the CloudEvents requirement for JSON format:

Such a representation MUST use the media type application/cloudevents+json

[rartych]

@jlurien @patrice-conil Can we finalize still open points and go with final review, to be ready for Commonalities 0.2.0 release?

[jlurien]

I see that you finally have removed data from the base CloudEvent object. I thought you were going to keep it as optional, but it is OK to remove it as long as we add also an example about how to model a ChildEvent with required data. Otherwise the artifact is not complete in order to serve as a model.

[patrice-conil]

I see that you finally have removed data from the base CloudEvent object. I thought you were going to keep it as optional, but it is OK to remove it as long as we add also an example about how to model a ChildEvent with required data. Otherwise the artifact is not complete in order to serve as a model.

Hi @jlurien, Commit reverted to keep data as optional field.

[patrice-conil]

Hi @rartych,
I think we haven't any open point (or I missed them) so we can finalize review.

[rartych]

Thanks @patrice-conil .

@jlurien @PedroDiez @RubenBG7 @jordonezlucena Could you approve this PR, so we can be ready for release 0.2.0?

[PedroDiez]

Thanks @patrice-conil .

@jlurien @PedroDiez @RubenBG7 @jordonezlucena Could you approve this PR, so we can be ready for release 0.2.0?

will review today

[PedroDiez]

LGTM

[jordonezlucena]

/LGTM