Clarify interoperability of extensions (#508)

[alanconway]

Fixes #508

Signed-off-by: Alan Conway aconway@redhat.com

[duglin]

@alanconway rebase needed

[alanconway]

Dropping this PR, @duglin beat me to it by a day - this change is sufficient for me:

f41061e 2019-09-19 make extensions follow the same pattern as normal attrs (#505)

[alanconway]

Had another think, here's my new take. Less is more! .

[duglin]

rebase needed

[duglin]

rebase needed

[alanconway]

This bit! I need to implement it in skd-go/pkg/binding.

It sounds to me like you want an "unofficial" bag of attributes scraped off the protocol:

Event.ProtocolAttributes() map[string]interface{}

I'm thinking this bag would be composed of all protocol properties/headers from an incoming message with CE-type-system compatible values, minus any that were mapped to proper CE attributes (standard or extension) with their unmodified property/header names. On the outgoing message this bag would be unceremoniously dumped into the outbound message.

There are potential name clashes, and gathering of useless data as we go between bindings but it would probably work well enough to support some scenarios. It would have to be optional as it would probably hopelessly break other scenarios to dump one protocols metadata on another. I did a POC AMQP-HTTP bridge before I joined the Cloud and this is pretty much what it did (although it did have some pairwise AMQP-HTTP smarts that would be lacking in this generic version)

Is that what you're getting at?

[duglin]

Basically yes and I think it was driven by those duplicate serialized extensions. So, in the tracing case we were thinking that it would appear as ce-traceparent and HTTP header traceparent, and if they had different values we wanted to encourage implementations to expose both to the receiving app so they could decide what to do about the diff. If they only ever saw the ce- one (and couldn't even get to the other one) then we're kind of setting people up for a situation where they're forced to only deal with one of the two.

[duglin]

This focuses on a "binding" defining the duality when I tend to think of it as the spec that defines the extension. We don't define any of these attributes in our specs (core or protocol) so it feels misleading to hint that a protocol spec would know anything about these special attributes.

Would it be bad to say: An extension attribute specification MAY define an additional location to which it would be serialized (in additional to the REQUIRED location defined by the protocol binding specifications). ... Such specifications MUST specify how....

[alanconway]

Aha, we reach the heart of our misunderstanding. We are talking about different things with the same words :)

• extension attributes are name-value pairs defined by the core and serialized by a binding.
• extension spec describes an arbitrary external system that defines and uses named extension attributes to accomplish some task.

I tripped over "extension MAY serialize ..." - it made me think the extension spec was somehow changing the serialization rules of the binding. Now I see that's not the case: e.g. the trace extension is using extension attributes to annotate a HTTP message for its own purposes. Those annotations are not part of the binding's serialization of the event and will be ignored by the receiving binding (unless it also implements the extension.)

Of course the extension spec must say how to deal with those annotations, which is what you were trying to advise about from the beginning. It's not part of the core spec, but I'm ok with including it as advisory if we clarify the distinction. Maybe say "annotate" rather than "serialize to an alternate location" or some such. I'm specced out for today, do you want to have a go?

[alanconway]

OK, not specced out - check the latest. Moved back towards your original text, but with more clarity about what is binding spec and what is extension spec. I think it makes sense now.

[duglin]

extension attributes are name-value pairs defined by the core and serialized by a binding.
Well, name-value pairs yes, but not defined by the core. They're defined by someone who isn't us (ignoring the fact that we provide a convenient hosting location for people who don't want to do it themselves).

Yes to your 2nd para.

Not sure what your 3rd (last) para is saying though. An extension spec will, at a minimum, define a new extension attribute. If it does nothing else then that attribute will be serialized like all other CE attributes for whatever format/binding is used (ignore any special cases like 'content-type'). That extension spec MAY also define alternate locations for the data to appear but the data MUST also be serialized using those default serialization rules. And, if there is an alternate location then the spec MUST say what to do if those multiple locations have different values when it is received.

Does that help?

[alanconway]

extension attributes are name-value pairs defined by the core and serialized by a binding.
Well, name-value pairs yes, but not defined by the core.

I want to make it clear that there are 3 distinct layers:

1. core spec defines event to include open set of named attributes
2. binding spec defines how to serialize attributes
3. extension spec choses attribute names and says what to use them for.

I did a minor re-order to make this clearer.

Yes to your 2nd para.

Not sure what your 3rd (last) para is saying though.

Me neither. Maybe you can have another go at it. Here's where I'm having trouble:

An extension spec will, at a minimum, define a new extension attribute. If it does nothing else then that attribute will be serialized like all other CE attributes for whatever format/binding is used (ignore any special cases like 'content-type').

IMO that goes without saying and saying it is confusing. The extension spec has nothing to do with how the attribute is serialized - that's the binding spec. The extension spec is simply picking a name and using the facilities provided by core and binding.

That extension spec MAY also define alternate locations for the data to appear

I'm struggling with this: it's not an "alternate location" for the attribute, it's just one thing an extension might do with attribute values - like storing them in a database, logging them or making routing decisions. I accept this is an important case you want to highlight, but I feel this language sounds like you're mandating something about how events are serialized.

but the data MUST also be serialized using those default serialization rules.

This sounds wrong - the only reason to copy data to different parts of the message is to communicate with some external system that doesn't know about the CE spec. Our encodings might fit, or might not. If an extension is setting timestamps to be read by a system that expects milliseconds-since-epoch, there's no point using RFC3999 strings.

I'll let you take another shot at what you're trying to say there. I think I agree with the substance but I found the language confusing so others might too. Hopefully you see what was confusing me...

[duglin]

An extension spec will, at a minimum, define a new extension attribute. If it does nothing else then that attribute will be serialized like all other CE attributes for whatever format/binding is used (ignore any special cases like 'content-type').

IMO that goes without saying and saying it is confusing. The extension spec has nothing to do with how the attribute is serialized - that's the binding spec. The extension spec is simply picking a name and using the facilities provided by core and binding.

The reason I think it needs to be said is because of all of the discussions we've had in the past around how extensions are serialized. E.g. are they just like normal CE attributes or do they get special "extensions" bucket? You missed a lot of the fun, but it's been a challenge to get everyone on the same page here. Some folks really wanted extensions to be serialized different from spec-defined attributes and as such I think explicitly saying that they're not special helps. At worst, you can say it's redundant and I can live with that if it stops people from making poor choices and breaking our extension model/plans.

That extension spec MAY also define alternate locations for the data to appear

I'm struggling with this: it's not an "alternate location" for the attribute, it's just one thing an extension might do with attribute values - like storing them in a database, logging them or making routing decisions. I accept this is an important case you want to highlight, but I feel this language sounds like you're mandating something about how events are serialized.

What do you think it's mandating? I'm not sure I would put this into the same category as "one thing an extension might do with an attribute value - like storing them in a DB" because those actions are not visible on the wire. I suspect most people will (rightfully) assume that any attribute will only appear once in a CE message because for the majority of the cases that's true (ignoring the fact that an attribute value might also appear in the data section). So, explicitly saying that it's ok for these edge cases (where a value can appear in more than one location in a message) is something I think is valuable to say. Or put another way, I think someone could claim that putting a value in 2 locations is violating the spec because one of those locations isn't prefixed with ce- when clearly the rules for that protocol says it should be. And I think it's important to make it clear that it is ok to do so, as long as the "normal" serialization version is there.

but the data MUST also be serialized using those default serialization rules.

This sounds wrong - the only reason to copy data to different parts of the message is to communicate with some external system that doesn't know about the CE spec. Our encodings might fit, or might not. If an extension is setting timestamps to be read by a system that expects milliseconds-since-epoch, there's no point using RFC3999 strings.

Yes, the reason for the 2nd serialization is to work with another system that doesn't know about the ce- version. Why is that wrong? What needs to happen though is that the extension spec needs to say what a CE receiver should do about conflicts between the two locations.

I'll let you take another shot at what you're trying to say there. I think I agree with the substance but I found the language confusing so others might too. Hopefully you see what was confusing me...

I'd like to turn this around :-) and ask you what's wrong with the original text that's in the spec given I think your understanding of the reasons behind it may have shifted?

[duglin]

LGTM

[duglin]

Approved on the 10/17 call