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

Submission of draft Cloud Events DDS Protocol Binding and DDS Event Format #1233

Closed
wants to merge 47 commits into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented Oct 5, 2023

Fixes #

This PR includes the following new files added to the cloudevents/working-drafts folder:

  • dds-protocol-binding.md
  • dds-format.xml
  • dds-format.md

Proposed Changes

  • cloudevents/working-drafts/dds-protocol-binding.md (new contribution)
  • cloudevents/working-drafts/dds-format.xml (new contribution)
  • cloudevents/working-drafts/dds-format.md (new contribution)


### 2.2 OPTIONAL Attributes

CloudEvents Spec defines OPTIONAL attributes. The set of possible Attribute Types and Values
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Spec/spec/

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated and language cleaned up


```

Based on the above, the type definition for optional Attributes is as follows:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like I'm missing something... what in the example below indicates that the attribute being defined is optional vs mandatory? Also is Attribute (in <struct name="Attribute">) the name of the attribute? If so, perhaps call it something less ambiguous, like MyExtension - otherwise it looks like a keyword and not a user defined name.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed "Attribute" -> "DDS-CE-Attribute" and "Attributes" -> "DDS-CE-Attributes". Does that read any better to you?

For other types, the implementation MUST translate the data value into a text or JSON
representation using the union type described for the message body.

## 4 Examples
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be useful to DDS folks to see a full XML CE serialization, in addition to the table below?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The long story: The XML format here is what is used to define what is required/optional in the DDS CE message. This format information is then provided to a DDS code generator that defines the message classes (code in C, C++, Java, Javascript, other supported languages) with the required attributes. The data values are populated in code into the message classes and the data is actually sent over the wire in a CDR (Common Data Representation) format which is then decoded by the DDS message receiver on the other side. So the XML is not the serialization format itself, it is simply the information used to configure the CDR serialization/deserialization process.

This was obviously not clearly written down by me. I could use some advice though on how to best put state this information in the document. Could we have a discussion on this topic? I will see if I can get on the agenda at tomorrow's meeting.

@duglin
Copy link
Collaborator

duglin commented Oct 12, 2023

I'm curious about the relationship between this and the XML stuff we have... should there be a relationship? Should this be a specific usage of the XML format? Meaning, we just make sure that any of our XML format rules are not violated by this PR.

@JemDay @jskeet any thoughts?

@jskeet
Copy link
Contributor

jskeet commented Oct 12, 2023

It feels odd to me to have a format specific to one particular use case. I would prefer it if the XML format could be used by DDS as-is - and would like to know what benefit there is in having a whole separate format vs "DDS uses the XML format, and specifies that attributes X, Y and Z must be present."

@duglin
Copy link
Collaborator

duglin commented Oct 25, 2023

@protimabanerjee any update on this one coming?

@ghost
Copy link
Author

ghost commented Oct 25, 2023 via email

Protima Banerjee added 22 commits October 25, 2023 17:59
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: Protima Banerjee <pbanerjee@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
@ghost
Copy link
Author

ghost commented Nov 6, 2023 via email

[CloudEvents][ce] is a standardized and protocol-agnostic definition of the
structure and metadata description of events. This specification defines how the
elements defined in the CloudEvents specification are are represented using
a protobuf schema.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"protobuf" ? :-) and on line 32 below

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed :-)

datakey:"Somekey",
datacontentencoding: "binary",
data: Buffer.from("Some text string" as string) // send the binary representation of a string
// [ext1Name]: ext1Value,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you turn these extension samples into real code, not comments? I'd like to see (and ensure) that extensions are treated the same as spec-defined attributes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We started digging a bit deeper into optional attributes and extensions and would like to defer support for these until a later revision of the protocol binding.

The issue is that there is some overlap between optional attributes that can be specified at the DDS protocol level using DDS User Data, Topic Data and Group Data Quality of Service (QoS) settings. It is not exactly clear if these QoS (if they are set) should be propagated up to CE Optional Attributes or if CE Optional Attributes should be mapped down to DDS QoS. Similarly, for extension attributes there is some overlap between DDS QoS and CE Extension attributes. The clearest overlap is between the Expiry Time and DDS Lifespan - we would like to consider use cases to determine how best to map from one to the other.

At first glance, it seemed like it would be possible to simply carry the OPTIONAL attributes through, but now we are not so sure that this will make sense for DDS->CloudEvents use cases.

Does this approach seem reasonable?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, the working code (no extension attributes) is here: https://github.com/protimabanerjee/sdk-javascript/tree/main/examples/dds-ex-simple

<member name="value" type="nonBasic" nonBasicTypeName="io::cloudevents::AttributeValue"/>
</struct>

<typedef name="DDS-CE-Attributes" type="nonBasic" nonBasicTypeName="io::cloudevents::DDS-CE-Attribute" sequenceMaxLength="100"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean there can only be 100 CE attributes? Is that a limit per the DDS spec or just a "sounds about right" value? Does there have to be a max specified at all?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A max doesn't have to be specified because DDS does support unbounded strings and sequences. We actually ran some examples to verify the structure and setting a max makes the example code a bit simpler. There are a couple of extra memory management parameters you have to add with unbounded sequences. I just added those and verified that the examples work as expected without the maxlength needing to be specified.

</union>

<struct name="DDS-CE-Attribute">
<member name="key" type="string" stringMaxLength="255"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec says attribute names SHOULD NOT exceed 20 chars.... notice it's "SHOULD NOT", not "MUST NOT", so the 255 is probably ok here, but I wonder if we should add some text to enforce the "SHOULD NOT" aspect... something like: the above definition mandates that attribute names are not longer than 255 characters, however, per the CE spec they SHOULD NOT be longer than 20 characters. Maybe? BTW, where did "255" come from? Is that a DDS thing?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

255 is the default max string length for DDS. I will update max length to be 20 characters for attribute name - I don't see any good reason to extend a DDS attribute name longer than that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I just removed the Attribute name altogether in light of the earlier comment.

are needed to construct the key-value sequence to establish the association
between the IDL types and the CloudEvent type system.

```xml
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this xml normative or just a sample and people can tweak as needed? If normative, we may want to say that DDS encoded messages MUST adhere to this definition.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is the right approach to make the XML normative. I updated the wording to say that DDS encoded messages must adhere to the schema.

<discriminator type="nonBasic" nonBasicTypeName="io::cloudevents::DataKind"/>
<case>
<caseDiscriminator value="(io::cloudevents::BINARY)"/>
<member name="binary_data" type="byte" sequenceMaxLength="100"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean DDS doesn't support binary data longer than 100 bytes?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it supports binary data of any length (unbounded). (It just made my example code simpler when I was doing a test run - but there is no need for a max here.)

The DDS protocol binding does not currently support the _batch_ content mode.

In the _structured_ content mode, event metadata attributes and event data are
placed into the DDS message using an [event format](#14-event-formats).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, the next paragraph mentions the data section but this paragraph does not, is it correct to assume that in structured mode the entire CE goes into the data section?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct. In structured mode, the entire CE message would go into the data section.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the wording a bit between the two sections to make the difference between structured and binary modes a bit more clear.

- `application/cloudevent+dds` for binary and plain text data types

If the `datacontenttype` attribute is set to `application/cloudevent+dds`, the
`datacontentencoding` attribute defines the encoding of the message body.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/defines/MUST be present and MUST define/
I assume? Or is it s/defines/, if present, MUST define/ ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is the latter - if present, MUST define
(thank you!)

If the header is present and its value is `application/cloudevent+dds`, the
receiver decodes the message into the DDS message format, using the
`datacontentencoding` attribute to determine the encoding of the message body.
There are three valid values of the `datacontentencoding` attribute
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 or 2?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is 2! Another good catch - at first, I was thinking the JSON structured mode should go here too - but that is defined by its own format.


### 3.1. Keys

The `datakey` of the DDS message MAY be populated. A _key field_ in DDS is way
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/is way/is a way/ ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed - thank you!

The `datakey` of the DDS message MAY be populated. A _key field_ in DDS is way
to uniquely identify individual instances of data being published to a topic. For
example, if you are publishing data to a Temperature Topic dealing with
temperature readings from different sensors, the sensor ID could be a key field.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example feels odd since "sensor ID" won't be unique per "instance of data", right?

Copy link
Author

@ghost ghost Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In DDS terminology, an instance is a set of samples that all have the same key value. A data sample is an individual message. I changed the wording to:

The datakey of the DDS message MAY be populated. A key field in DDS is a way
to uniquely identify types or categories of data being published to a topic. For
example, if you are publishing data to a Temperature Topic dealing with
temperature readings from different sensors, the Sensor ID could be a key field.

"Instance" and "sample" are not intuitive terms in DDS. Does the above wording make the example any clearer?

the DDS application [data](#21-data) section.

Similar to binary mode, the DDS representation of a structured CloudEvent with
no `data` is a DDS message with no body, and transmission of such a message
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is a DDS message with no body... is this true? Wouldn't a structured CE still have a DDS message+body(data) but just missing the data attribute of the CE structure? Meaning, the DDS message's data would have the CE JSON but in that JSON the data attribute would be empty/missing. Or am I reading this incorrectly?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you are reading it correctly, my wording is incorrect. I changed the wording to the following:

The DDS representation of a structured CloudEvent with no data is a message
that contains all of the CE attributes but has an empty body. Transmission of
such a message is allowable.

Does this seem reasonable?


#### 3.3.3. Metadata Headers

Implementations include the same DDS headers as defined for the
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm reading this as the CE attributes are to appear as DDS Headers AND in the CE structure too. Is that the intent?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we're really only supporting (reading) the content-type header for both binary and structured data, so I updated the text to just say this.

Copy link
Contributor

@JemDay JemDay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still a little confused about whether I can use 'dds format' indepentantly of the 'dds protocol'.

Eg .. Can I format an event using DDS, send it over HTTP, and de-serialize it back into a CE

</case>
<case>
<caseDiscriminator value="(io::cloudevents::BYTES)"/>
<member name="ce_bytes" type="byte" sequenceMaxLength="100"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The CloudEvent type system calls this 'binary' not 'bytes'.

Additionally I don't believe there's any mention of max-length in the spec, rightly or wrongly. So I'm not sure what to suggest as a length constraint if that is required.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We thought about it and at this time we're not ready to support optional and extension attributes yet, since there might be potential conflicts with what is defined at the DDS protocol level in QoS settings. So this section is going away.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Responding to: Can I format an event using DDS, send it over HTTP, and de-serialize it back into a CE

The DDS format is tightly coupled to the DDS protocol. So when you send a message using the DDS protocol, you have to define the message format in DDS IDL, run the IDL through a code generator, and then get a set of classes that serialize/deserialize to the DDS wire protocol.

I don't think there's really a way to use that serialization/deserialization code outside of DDS, the way you can with protobuf. So the end result is that the DDS format works only for the DDS protocol. I agree with you - this is an important point and I will add it to the doc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the slow response...

If the formats & transports are tightly coupled I'm concerned it might not be appropriate to make a part of the CE specification.

If I'm not able to plug-and-play formats (wire-encoding) and different transport protocols this may be better presented as a separate document that describes how CE's fit, or are used, in the context of DDS.

Ideally we should be able to realize a flow such as this (where Int == 'Intermediary'):

Source --[HTTP]--> Intermediary --[DDS]--> Intermediary --[AMQP]--> Intermediary --[KAFKA]--> SInk

Where the CE received by the Sink is equivalent to the CE emitted by the source, and the 'data' is exactly what the source put into the original CE.

If we're not able to slot DDS into that type of pipeline I'm not sure this is the right place.

Apologies if I'm still not joining the (mental) dots.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a valid concern and, honestly, I was having this same doubt after watching Doug's demo showing how CloudEvents is able to pass messages across protocols. If a message goes to DDS that it is not able to deserialize, it does not look at any other information in the message to determine who to forward to. It simply drops the message. Now, there could be updates made at the protocol level to cause this kind of forwarding behavior but it does not exist in the standard today. There is a DDS Web Integration standard that defines how DDS traffic can be tunneled over HTTP, but nothing that does the reverse type of action.

So this might be the blocking point. :-(

<enumerator name="TIMESTAMP"/>
</enum>

<union name="AttributeValue" extensibility="final">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to add a to represent a Timestamp type ???

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above. (But this was actually a copy/paste error from my sample code. I forgot the timestamp for some reason.)

</case>
<case>
<caseDiscriminator value="(io::cloudevents::TEXT)"/>
<member name="text_data" type="string" stringMaxLength="255"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to @duglin comment above ... I'm not sure we place any constraints (rightly or wrongly) on the size of event data.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed all bounds on length/sizes, so we should be good here.

id: "b46cf653-d48a-4b90-8dfa-355c01061364",
type,
source,
datacontenttype: "application/cloudevent+dds",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to align, the data added in the code below is binary content (such as image/jpg)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DDS serialization format (which is called the Common Data Representation or CDR) is actually the format that is used to send the data "over the wire". The receiver on the other side would need to know that binary data was being sent over the DDS wire protocol to know how to decode it.

Does this make sense?

Copy link
Contributor

@JemDay JemDay Dec 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my previous note ...

Isn't the the format "on the wire" the CloudEvent?, the 'datacontenttype' attribute tells us what is 'inside the CE' ... the 'application/cloudevent+dds' construct would be used when sending a 'DDS formatted CE' over a transport such as HTTP.

Your example goes on to add textual data, so the datacontenttype might be 'application/text'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is all part of the same blocker, unfortunately. :-(

</union>

<struct name="Event" extensibility="mutable">
<member name="headers" type="nonBasic" nonBasicTypeName="io::cloudevents::Headers"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this represent ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DDS protocol doesn't have any mechanism for sending headers as some other protocols do so we have to create a custom header structure within the DDS message for the users to add their headers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the struct named 'Event' is the message-definition (for want of a better word) for a CloudEvent serialized using DDS?

Copy link
Contributor

@JemDay JemDay Dec 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is 'headers' the construct used to carry the CE 'extension' attributes ?

Apologies for my lack of understanding of DDS but can you apply a similar model that you used for 'Data' ... ie an extension attribute could be represented a tuple of (name, value) where 'value' must be one of the declared (typedef'd) datatypes.

I guess what I don't see is how I can represent a CE with an custom extension attribute named "expiresat" with a value of a timestamp, or one named "islucky" with a boolean value, for example.

It's true that some of our existing formats don't support the propagation of type information, but if there's a means to do that it really improves interop.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if @JemDay is correct and "headers" are used for extensions, then we may need to rethink this because CE extensions are meant to be serialized just like spec-defined attributes, not within a separate "bucket"

protimabanerjee added 4 commits November 20, 2023 16:04
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
Signed-off-by: protimabanerjee <protima@rti.com>
@duglin
Copy link
Collaborator

duglin commented Nov 30, 2023

@protimabanerjee, from your perspective is this ready to merge?

All - if 'yes', I'd like to see if we can vote on this before the holidays, perhaps even on next week's call so please review it and add comments/concerns.

@ghost
Copy link
Author

ghost commented Nov 30, 2023 via email


The DDS protocol binding does not currently support the _batch_ content mode.

In the _structured_ content mode, event metadata attributes and event data are
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/metadata attributes/context attributes/ I think

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or just "event attributes" w/o "metadata"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my response to Clemens below. Unfortunately, I think I am withdrawing this submission as DDS is not a fit for a CloudEvents protocol or format due to a number of issues, but the primary one being interoperability with other protocols.

@clemensv
Copy link
Contributor

There are some substantial technical aspects that need to be addressed and that require changes:

  • The "datacontentencoding" attribute once existed but was dropped prior to Cloud Events 1.0 in 2019. That attribute can no longer be referenced or used in this fashion
  • A format must not disallow optional/extension attributes as the DDS Event Format does. The DDS Event Format is not conformant with the requirements of CloudEvents

I am not convinced this spec adding true interoperability value because of DDS's tightly coupled nature. It appears trivial for a DDS user to express a CloudEvent, including one that has been routed to a DDS system, by creating a data structure definition that contains the CloudEvent attributes plus the data, very similar to how the DDS Event Format does it, but including extensions and with a concrete schema for the data portion.

I am also somewhat skeptical of the interoperability value of a specification residing in this repository rather than in, say, OpenDDS or in an OMG repository, because of the small number of DDS implementations. The handful of existing commercial DDS vendors have a history of "marketing through standards organizations" and I would therefore be interested in seeing a concrete use-case scenario where CloudEvents is used with DDS today and how this specification helps with interoperability issues arising from that use. I am not sympathetic to a scenario where a vendor can check the CloudEvents box on their product without that being practically meaningful.

In our project primer, we have defined that it is a requirement that the project group agrees that the "specification will be of sustained practical benefit for any party that is unrelated to the product or project from which the protocol or encoding emerged. A base requirement for this is that the protocol or encoding is defined in a fashion that allows alternate implementations independent of the product or project's code."

I would also like to note that I am concerned about RTI's DDS related patent and how that might affect users of this specification
in the above spirit of enabling alternate implementations and strongly suggest that ask for CNCF counsel advice before we accept this submission.


## 2.4 OPTIONAL Attributes & Extensions

At this time OPTIONAL and extension attributes are not supported in the DDS format.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That clause is in conflict with the base spec.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clemensv as always, thank you for this thoughtful review.

@JemDay had also remarked earlier about the interoperability concerns with DDS.

I agree with both of you. I actually ran a few simple experiments to see what would happen if a message that DDS did not know how to interpret was sent to a DDS subscriber. The DDS subscriber simply drops the message with a serialization error. And I believe that this is dictated by the DDS specification. So I believe this submission is at its termination point.

I was not aware that RTI had a DDS patent though. We always have to ensure that DDS is marked as an OMG specification for any public distribution. @clemensv would you send me a link to the patent if you have it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see response above

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@protimabanerjee -

would happen if a message that DDS did not know how to interpret was sent to a DDS subscriber

I'm a little confused by this as I would expect from a DDS perspective you're exchanging CE's, so the applications would produce, and consume CE's as defined in your format specification.

by the `datacontenttype` attribute.

There are two values of `datacontenttype` currently supported:
- `cloudevent/json` for JSON data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not a valid content type for CloudEvents

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see response above

- `application/cloudevent+dds` for binary and plain text data types

If the `datacontenttype` attribute is set to `application/cloudevent+dds`, the
`datacontentencoding` attribute, if present, MUST define the encoding of the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

datacontentencoding only existed in a draft version of the spec and is not part of CloudEvents 1.0 and can therefore no longer be referred to and used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see response above

@duglin
Copy link
Collaborator

duglin commented Dec 14, 2023

@protimabanerjee to be sure... you want to close this PR right?

@ghost
Copy link
Author

ghost commented Dec 14, 2023 via email

@duglin duglin closed this Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants