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

Relation between AMQP Format and Transport Binding is unclear #519

Closed
deissnerk opened this issue Oct 2, 2019 · 15 comments · Fixed by #529
Closed

Relation between AMQP Format and Transport Binding is unclear #519

deissnerk opened this issue Oct 2, 2019 · 15 comments · Fixed by #529
Labels

Comments

@deissnerk
Copy link
Contributor

I've been reviewing the AMQP related parts of the spec, and I'm having trouble figuring out how the data section of the AMQP format relates to the AMQP transport binding. I found this:

All implementations of this specification MUST support the JSON event format as well as the AMQP event format for the application-properties section

This restricts the usage of the AMQP event format to the application-properties section, which excludes the handling of data.

In section 2.2 I found:

An application is free to hold the information in any in-memory representation of its choosing, but as the value is transposed into AMQP as defined in this specification, the assumption is that the data value is made available as a sequence of bytes.

Finally section 3.1.2 simply states:

The data byte-sequence is used as the AMQP application-data section.

So the handling of data as described in the AMQP format with its usage of AMQP values would not take place. As I don't think the AMQP format would make sense for the structured encoding, either, this pretty much excludes that this rule from the AMQP format is applied:

For other types (non-binary data without a datacontenttype attribute), the implementation MUST translate the data value into an AMQP type system value and the value MUST be stored in an AMQP value section.

All this really confuses me. In the examples section of the JSON format there is this example:

{
    "specversion" : "1.0-rc1",
    "type" : "com.example.someevent",
    "source" : "/mycontext",
    "id" : "C234-1234-1234",
    "time" : "2018-04-05T17:31:00Z",
    "comexampleextension1" : "value",
    "comexampleothervalue" : 5,
    "datacontenttype" : "application/json",
    "data" : {
        "appinfoA" : "abc",
        "appinfoB" : 123,
        "appinfoC" : true
    }
}

How would an SDK transform this into AMQP format/binary encoding? What would happen, if the datacontenttype was omitted? I suppose, according to the AMQP format some kind of AMQP value would be created. Would it be an AMQP Map?

@alanconway
Copy link
Contributor

Here's my understanding:

Structured format (e.g. JSON):
AMQP { content-type: "application/cloudevents+json"; data-section=}

Binary format where event has non-empty datacontenttype, e.g. text/XML
AMQP{ content-type="text/XML"; data-section=

Binary format where event has missing/empty datacontenttype: in this case the event data MUST be a CloudEvents type; this is where the AMQP format rules come in.
AMQP{ value-section=}

I'm working on an AMQP binding for cloudevents/sdk-go right now along these lines.
The structured and non-empty content-type cases are straightforward - just set the AMQP content-type and a single data section.

In the empty datacontentype case: the Go SDK holds the event Data as a Go value corresponding to one of the CE types (int32, string, []byte, time.Time) etc. The AMQP binding looks at this and converts to a corresponding AMQP type for the AMQP value section (amqp long, timestamp, binary etc.)

@deissnerk
Copy link
Contributor Author

@alanconway That totally makes sense, but I could not find this in the spec for binary.
As I wrote in cloudevents/sdk-go#217 (comment), my understanding in case of a missing datacontenttype is that the spec allows arbitrary types:

When this attribute [datacontenttype] is omitted, data simply follows the event format's encoding rules. For the JSON event format, the data value can therefore be a JSON object, array, or value.

@alanconway
Copy link
Contributor

@deissnerk I agree that needs clarity. The way I read it "the event format encoding rules" only cover CE types - the formatting rules are explicitly about CE types. Therefore it's implied (unclearly) that if there's no datacontentype (or no AMQP content-type) then we must be dealing with a CE type. Otherwise there's no way to know what to do. The spec does need clearer language.

@deissnerk
Copy link
Contributor Author

@alanconway So, if I remove the datacontenttype from the example above, it would not be a valid CloudEvent?

{
    "specversion" : "1.0-rc1",
    "type" : "com.example.someevent",
    "source" : "/mycontext",
    "id" : "C234-1234-1234",
    "time" : "2018-04-05T17:31:00Z",
    "comexampleextension1" : "value",
    "comexampleothervalue" : 5,
    "data" : {
        "appinfoA" : "abc",
        "appinfoB" : 123,
        "appinfoC" : true
    }
}

@alanconway
Copy link
Contributor

alanconway commented Oct 2, 2019 via email

@deissnerk
Copy link
Contributor Author

I am not sure, if your understanding regarding data and the CE type system matches with what the others (@clemensv?) intended in #484. In this PR the notion of data as an attribute is removed. The idea seems to be that people do not want application/json as datacontenttype. If, as you implemented it, an intermediary can set the datacontenttype to application/json when forwarding the event using a different format, this can work for JSON and becomes just a convenience feature. For the AMQP format it is worse. As it does not define an envelope, it would always require the transformation of an object graph into something else.

So having the support of the AMQP format mandatory for a compliant implementation is problematic.

It is confusing that the semantics of datacontenttype not being present differs depending on transport bindings and event formats:

  • For HTTP binary you can only treat the payload as an undefined binary.
  • In the JSON format it is implicitly assumed to be application/json, if datais used. It should then also be set to application/json when forwarding the event in a different format. Even that is not specified. So someone emitting and event in JSON format leaving out the datacontenttype for convenience, must fear that the JSON object in data will be transcoded several times in a lossy way.
  • If in JSON data_base64 is used, datacontenttype again is assumed to be unspecified.

@markpeek markpeek added the v1.0 label Oct 3, 2019
@alanconway
Copy link
Contributor

alanconway commented Oct 3, 2019 via email

@deissnerk
Copy link
Contributor Author

I like the proposal. As a consequence we could simply remove the section Data from the AMQP format.

@duglin
Copy link
Collaborator

duglin commented Oct 7, 2019

late to the party...

Can be legally decoded in two ways:

  1. Event{ Data: int32(123); DataContentType: nil }
  2. Event{ Data: "123", DataContentType: "application/json" }

I don't think that's right. I think the only interpretation of it is:

  1. Event { Data: number(123); DataContentType: "application/json" }

I say this because:

  • w/o a datacontenttype we implicitly get app/json from the event itself
  • 123 isn't wrapped in quotes so in JSON its a number, not a string

@alanconway
Copy link
Contributor

late to the party...

Can be legally decoded in two ways:

  1. Event{ Data: int32(123); DataContentType: nil }
  2. Event{ Data: "123", DataContentType: "application/json" }

I don't think that's right. I think the only interpretation of it is:

  1. Event { Data: number(123); DataContentType: "application/json" }

I say this because:

  • w/o a datacontenttype we implicitly get app/json from the event itself
  • 123 isn't wrapped in quotes so in JSON its a number, not a string

This is confusion between my initial daft idea about using the CE type system when datacontentype is absent and the more sensible idea expressed in #521. I think if we can hammer out #521 then we're set on this one also.

The Go SDK has to deal with multiple representations of Data, since the JSON-encoded "number" 123 is actually the byte sequence {49,50,51}, alias the string "123", also represented in Go as the 32-bit binary integer int32(123). Plus there are a bunch of convertible numeric types. So probably my use of Go notation above is not helping to clarify :)

@deissnerk
Copy link
Contributor Author

deissnerk commented Oct 8, 2019

I really like #521, but even with this PR being accepted the original question of this issue remains unanswered. So far the AMQP protocol binding references the AMQP format only to clarify how CE types for CE attributes map to AMQP types. When it comes to data, the protocol binding describes structured and binary mode. As the AMQP format does not specify a default datacontenttype, the attribute becomes mandatory in AMQP. So section 3 becomes obsolete. But if we remove section 3, it will not be a format any more, as it does not "specify how to serialize a CloudEvent as a sequence of bytes". A logical consequence would be to get rid of the AMQP format and move the type mapping for CE attributes over to the protocol binding.
@clemensv Any thoughts on that?

@clemensv
Copy link
Contributor

clemensv commented Oct 8, 2019

The AMQP format was really only meant to provide a proper type mapping such that attributes can be mapped to AMQP properties.

It wasn't meant to fully express events in the AMQP type system because the AMQP type system is not really being used as a general purpose data serialization format.

The idea was that AMQP messages generally carry events that are using an encoding that's useful for end-to-end payloads.

@deissnerk
Copy link
Contributor Author

Yes, I understand that. I think the confusion was caused by some changes we waved through in August. With data not being an attribute anymore, the AMQP format got a section describing how data would be encoded. That was different from what the transport binding specified, hence I created this issue and the whole discussion started.
With @alanconway's #521 we get a definition of event format as something that "specifies how to serialize a CloudEvent as a sequence of bytes". The AMQP format as it was originally intended does not match to this definition. This could be resolved by moving the relevant parts of the AMQP format into the transport/protocol binding spec.

@alanconway
Copy link
Contributor

@deissnerk +1 - I'd prefer to reserve the term "format" for encodings that have a "application/cloudevents+" MIME type and can be used to format structured events over any protocol. The JSON format fits that bill, but the AMQP format doesn't.
(It could if the AMQP value encoding had a MIME type and there was interest in using it outside of AMQP protocol connections, but I don't think that is likely)

@deissnerk
Copy link
Contributor Author

@alanconway Please have a look at #529.

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

Successfully merging a pull request may close this issue.

5 participants