Hono: Fix encoding of OpenTracing span in AMQP messages

[ctron]

My understanding is that the current encoding of OpenTracing information in AMQP messages could cause some issues it is not compliant with the AMQP spec.

About annotations the spec says (http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-annotations):

The annotations type is a map where the keys are restricted to be of type symbol or of type ulong. All ulong keys, and all symbolic keys except those beginning with "x-" are reserved. Keys beginning with "x-opt-" MUST be ignored if not understood. On receiving an annotation key which is not understood, and which does not begin with "x-opt", the receiving AMQP container MUST detach the link with a not-implemented error.

What seems to happen right now is that the MessageAnnotationsInjectAdapter is being used in combination with the TEXT_MAP format, which would directly inject uber-trace-id and uberctx-* into the message annotation map (see: io.jaegertracing.internal.propagation.TextMapCodec):

So it might happen, that a receiver simply closes this link when the annotation is present in the map.

The proposal would be to use something like x-opt-opentracing-trace-id and x-opt-opentracing-ctx- instead.

[ctron]

Looking into the code, it actually might be tricky to get the proposal implemented. Maybe a different way to do it would be to keep the current keys of uber-* but put them into the annotations under a single key, using an AMQP map.

[sophokles73]

The MessageAnnotations*Adapter actually injects to/extracts from the AMQP message's delivery annotations at the moment. The naming of the class does not reflect this properly (bad :-1: ). However, this does not change the overall problem here.

[calohmn]

In general I think it's a good idea to keep an eye on the AMQP related work in https://github.com/w3c/trace-context/. There's a draft in https://github.com/w3c/trace-context/pull/217 there.
There it is proposed to use a binary format. As this doesn't seem to be supported by Jaeger yet, I guess it's premature to adopt it here.
In any case I think it would be good to also address https://github.com/eclipse/hono/issues/1012 when doing changes on the Adapter classes here.

[calohmn]

When following the approach suggested in #1012 (also trying to read the trace-context from the _application-properties_ of the bare message), it has to be noted that an AMQP map is not supported as value type there. I would therefore suggest using the plain property names (e.g. _uber-trace-id_) in the _application-properties_ case.

[calohmn]

Created PR #1054.
I've opted for _not_ also trying to read the trace-context from the _application-properties_ of the bare message.
We can add such a feature later on when the w3c trace-context spec is finalized (and when jaeger has support for the binary format if that is used in the spec).

[sophokles73]

@ctron can we (i.e. you) close this issue?