Skip to content
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

Dependency on Cloud Events #126

Open
egekorkan opened this issue Nov 10, 2021 · 13 comments
Open

Dependency on Cloud Events #126

egekorkan opened this issue Nov 10, 2021 · 13 comments

Comments

@egekorkan
Copy link
Contributor

@mlagally has mentioned the F2F before TPAC of using cloud events in the profile and there are also other mentions of it in various issues. Something that needs to be documented somewhere is that the CloudEvents specification is driven by the Cloud Native Foundation, which is not a standards organization, thus cannot be simply linked to. This implies that having a dependency requires this profile spec to mirror their specification, as TD does with JSON Schema. The group should decide on whether this work is feasible in the current charter before deciding on adopting this specification.

@mlagally
Copy link
Contributor

@egekorkan
Thanks for flagging the problem.
I think we do not have to reference the entire cloud event specification (which is not large btw.) but just select the parts that are relevant for us. As explained in #100, this is only a very small section, which could be just included in the same way.

@benfrancis
Copy link
Member

Apart from not being from a standards body (whether in part or in full), I don't think a dependency on CloudEvents is needed or desirable for the Core Profile, see #100.

@mlagally
Copy link
Contributor

@benfrancis
I agree, we don't need the full cloud events, however a structured JSON payload with clear properties to be useful. Tying these into specifics of the the protocol binding is not desirable.
I think the event object should be protocol agnostic.

Wrt. reference problem: we can resolve by defining a compatible payload as part of the profile.

@benfrancis
Copy link
Member

benfrancis commented Nov 17, 2021

Tying these into specifics of the the protocol binding is not desirable.
I think the event object should be protocol agnostic.

Again, "tying into specifics of the protocol" is the whole point of a protocol binding. Protocol bindings should not be protocol agnostic because their purpose is to be protocol specific. Other event mechanisms in other protocols may also have their own payload formats and may use a non-JSON serialisation or binary data format, so there's not even any guarantee that the same object wrapper could be re-used.

The current event protocol binding provides a very simple and neat mapping from the WoT information model onto the server-sent events event stream format:

  1. The event name maps onto the event field
  2. The data schema maps onto the data field
  3. A timestamp can optionally be provided in the id field

Example event affordance in a Thing Description:

  "events": {
    "overheated": {
      "data": {
        "type": "number"
      },
      "forms": [{
        "href": "./events/overheated",
        "subprotocol": "sse"
      }]
    }
  },

Example event payload:

event: overheated\n
data: 90\n
id: 2021-11-16T16:53:50.817Z\n\n

Why overcomplicate this by adding in an unnecessary data wrapper which must then be added to the data schema of every single event affordance? Other than the non-goal of protocol agnosticism, what value does it provide to justify the added complexity?

@egekorkan
Copy link
Contributor Author

#235 needs to be revisited since it was merged with no proper reviews (also the PR was merged when it was 1 day old)

@egekorkan
Copy link
Contributor Author

Summary of my points across multiple issues and PRs since this issue is going to be used for discussions:

  1. The PR for CloudEvent was created 21 days ago and merged 22 days ago, meaning only for 1 day it was online. It took over work from WIP: event payload format #198 which had comments that were not resolved. PR CloudEvents as message payload format for WebHooks #235 also got no reviews on GitHub. The minutes show that only Michael McCool, Michael Lagally, Kaz, and Mizushima-san were in the meeting and not the people who actually expressed opinions on this feature (me, Ben and Sebastian).
  2. I think it is fundamentally wrong to propose this as mentioned also in Structured event object #134 (comment). I am not against CloudEvents spec in any way, it is very important work. However, it makes zero sense in a world where TDs exist. CloudEvents simply adds a metadata layer around the actual payload. Most of that metadata is already in the TD as pointed out by @sebastiankb as well. More specifically, the payload format defines the following points, I am putting my notes next to each:
  • specversion : can be put in the TD
  • id: can remain, it makes sense
  • source: base is already in the TD
  • subject: this can be in the href of the TD
  • type: even the text says copy content from TD
  • time: can remain, it makes sense
  • datacontenttype: we already have contentType in the TD
  • data: we have data in the TD. Also, the fact that this is optional, conflicts with :
    • https://w3c.github.io/wot-profile/#example-43 which only has a boolean data schema
    • some discussion in the TD(?) spec saying that we should not have empty payloads. Making this field optional means that we encourage empty payloads here. I am aware that the HTTP message will have a body but the other fields are basically header information put into the body.

Adding this information in a TD makes for longer TDs (maybe not depending on the my comment below) and bigger payloads.

  1. It is not clear what the consumer does with this information, what is the motivation behind it? From my point of view, once this event payload comes to the Consumer, it does not need a TD anymore.
  2. I think seeing some TDs would make my point above even more clear. For example, the TD from Oracle at https://github.com/w3c/wot-testing/blob/main/events/2022.06.Online/Profiles/TD/Oracle/TDs/WoTWebThing.td.jsonld#L214 does not even contain this metadata. This raises an important discussion about whether this payload structure should be represented in the TDs as well or are they just assumed.
  3. As expressed in the meeting minutes and the first comment here, it is a dependency like JSON Schema, which is not properly standardized yet. It is also in the incubation phase at CNCF: https://www.cncf.io/projects/#incubating . What kind of guarantees do we have? Since their spec is copied over here, we will not be compatible with them (a major argument from @mmccool and @mlagally ?) if they do some changes, even without bumping the version.

@mlagally
Copy link
Contributor

@egekorkan
As the description in #235 clearly states #235 is a rebased version of a previous MR that was available for received various review comments and questions: #198
#198 has been available since May 5th. and received review from you, Sebastian and you had raised some questions. There were no change requests.

All questions have been answered in
#198.

The content of the MR has not changed, it has been just rebased to resolve merge conflicts,
so it is not unreviewed content, but just a rebased version of #198, which was available for almost two months.

@mlagally
Copy link
Contributor

mlagally commented Jul 20, 2022

On the technical side:
We are talking about green field devices, but not about green field cloud systems.

Interop with existing cloud environments is helping adoption of WoT by enabling interop with what is in the world right now.

It is unlikely that the following implementations will change to use an event format that is exclusively defined in a TD:
https://developer.adobe.com/events/docs/guides/using/custom_events/
https://docs.microsoft.com/en-us/azure/event-grid/cloudevents-schema

@egekorkan
Copy link
Contributor Author

I think my core comment is not addressed anywhere: #198 (review)

If we are not aiming for others to change/adapt to our views of WoT, the whole profile specification makes no sense. We want people to do a specific thing when doing a new WoT implementation. If not, we have the entire binding templates documents that are all about explaining how you can use existing technology or standard in a WoT way. Maybe this is just my mistinterpretation.

We are talking about green field devices, but not about green field cloud systems.

We are talking about devices and sometimes about consumers. We do not have a clear statement about the cloud. Also, if we are talking about cloud like this, we are becoming a cloud-specific HTTP profile which means we need a renaming.

@mlagally
Copy link
Contributor

mlagally commented Jul 20, 2022

@egekorkan :

I find a fundamental problem with using cloud events in a profile that is meant for greenfield applications. It adds too much metadata to payload and does not make sense in a WoT specification.

I believe this is your main comment, please clarify if you mean something else. "too much" and "does not make sense" are opinions, but what "too much"? What means "does not make sense"?

Do you have a counterproposal about a scalable message format that will be easily integrated into changing existing event mechanisms of cloud systems?

@egekorkan
Copy link
Contributor Author

I believe this is your main comment, please clarify if you mean something else. "too much" and "does not make sense" are opinions, but what "too much"? What means "does not make sense"?

Everything that is in the TD already or can be put into the TD, should not be in the payload. Copy pasting the relevant part of my comment from above:

  • specversion : can be put in the TD
  • id: can remain, it makes sense
  • source: base is already in the TD
  • subject: this can be in the href of the TD
  • type: even the text says copy content from TD
  • time: can remain, it makes sense
  • datacontenttype: we already have contentType in the TD

Only the bold parts should stay and are the ones that make sense.

Do you have a counterproposal about a scalable message format that will be easily integrated into changing existing event mechanisms of cloud systems?

I do not know where this motivation comes from. Also, how does adding a wrapper around the data improves scalability?

@benfrancis
Copy link
Member

benfrancis commented Jul 20, 2022

In #100 (comment) I explained why I don't think CloudEvents is a good fit for the HTTP SSE Profile, which already has its own payload format for events. All of the metadata in the CloudEvent format can be provided in a Thing Description and by existing standardised fields in the Server-Sent Events standard.

From my implementation experience for the HTTP SSE Profile I found that the only data actually needed inside an event payload were:

  • Event name
  • Data
  • Timestamp/ID
  • (URL of the event source, which is implicit in the case of SSE since its the URL of the event stream)

WebHooks is a bit different for a couple of reasons:

  1. WebHooks is not a standard, only a design pattern, and as I understand it there is no standard payload format for events as there is for SSE (Most WebHook implementations do not use CloudEvents)
  2. A consumer doesn't implicitly know the source of an event when it arrives, because each event is sent in a separate HTTP request from the Producer to the Consumer and could come from multiple sources (as opposed to over a persistent connection between a Consumer and a Producer's event stream URL)

As @egekorkan has already said, most of the metadata in CloudEvents is redundant in the context of WoT because it's already provided by the Thing Description. Putting those metadata in the payload of every WebHook event request has no obvious benefit for greenfield implementations, and is therefore wasteful.

From what I can tell, the useful fields from CloudEvents for WoT use cases are:

  • source (could be the URL of the TD, base URL, the URL of the event affordance, or the URL of the events meta interaction affordance)
  • subject (event name, if not provided as part of source)
  • time
  • id (if an identifier separate from the timestamp is desired)
  • data

A minimal solution would be a payload format like the following:

{
  "source": "http://192.168.0.124:8080/events/fireAlarm",
  "time": "2021-11-10T11:43:25.135Z",
  "data": true
}

A more verbose option would be:

{
  "source": "http://192.168.0.124:8080",
  "subject": "fireAlarm",
  "time": "2021-11-10T11:43:25.135Z",
  "id": "9c18e98e-083b-11ed-861d-0242ac120002"
  "data": true
}

Or if we wanted to be consistent with the HTTP SSE Profile:

{
  "source": "http://192.168.0.124:8080/events",
  "event": "fireAlarm",
  "id": "2021-11-10T11:43:25.135Z",
  "data": true
}

I don't see much benefit of including all the metadata from the CloudEvents format for greenfield WoT implementations (for which the Profile specification is designed), but big downsides in terms of inefficiency.

It might make sense to create a CloudEvents protocol binding template, since binding templates are designed for describing brownfield devices.

Since @egekorkan and I disagree with @mlagally on whether to use CloudEvents for the HTTP Webhooks Profile, do any of the other editor's of the specification have an opinion? @mmccool, @mryuichi, @sebastiankb, @k-toumura?

Other than Oracle, is anyone else interested in providing an implementation which uses CloudEvents?

@mlagally
Copy link
Contributor

mlagally commented Sep 7, 2022

Payload formats for WebHooks are deferred to Profile 1.1. / Profile 2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants