B3 clarification

[mwear]

Fixes #960

Changes

This PR clarifies expectations around B3 context propagation. Specifically, B3 propagators:

• MUST support extracting context from both single and multi-header encodings
• MUST default to injecting context using the multi-header encoding, but
  MAY provide configuration to change the default

Additionally, it relaxes the requirement that b3 and jaeger propagators need to distributed as extension packages, making it possible to distribute them as part of the SDK if a language chooses.

Related issues #

Related oteps #

[codeboten]

👍

[carlosalberto]

Hey @mwear thanks for the PR - to be clear, you are proposing having a single B3 Propagator that extracts both the single/multiple format and injects either the multiple or single format?

Also, it seems the single format is the (new) preferred format between the two, as mentioned in openzipkin/b3-propagation#21 (not an expert myself though).

[mwear]

Hey @mwear thanks for the PR - to be clear, you are proposing having a single B3 Propagator that extracts both the single/multiple format and injects either the multiple or single format?

I'm proposing a single propagator that will extract either format and injects using a single format (defaulting to multi-header). This is how the Python and Golang implementations already work.

Also, it seems the single format is the (new) preferred format between the two, as mentioned in openzipkin/b3-propagation#21 (not an expert myself though).

I think it's true the single header B3 is the new, preferred way, but multi-header will be more backwards compatible. There is a comment in the golang implementation saying similar. Pasted below:

// InjectEncoding are the B3 encodings used when injecting trace
// information. If no encoding is specified (i.e. B3Unspecified)
// B3MultipleHeader will be used as the default as it is the most
// backwards compatible.

[bogdandrutu]

@adriancole do you mind telling us what is the right default (single or multi header)?

[codefromthecrypt]

Sure.

On read, always read both, but prioritize "b3". Even though this is mentioned in HTTP, it is simplest and most coherent to do this any time B3 is used as sometimes HTTP headers end up read by messaging: "The single-header variant takes precedence over the multiple header one when extracting fields."

On write, here's the suggestion. We don't differentiate between RPC and HTTP when discussing CLIENT.

• span.kind=CLIENT -> B3_MULTI
  • Multi includes serializing the parent ID of the client span unless supportsJoin=false
  • SINGLE can be used alternatively and determined per-request if known downstream supports it.
• span.kind=PRODUCER -> SINGLE_NO_PARENT
  • no parent for messaging spans as it is invalid to share span IDs except RPC
• span.kind=CONSUMER -> SINGLE_NO_PARENT
  • we only serialize back onto the message if the processor framework isn't known. Otherwise, we clear the header.
    SINGLE_NO_PARENT
• not remote -> SINGLE_NO_PARENT
  • ex. same process messaging
  • some formerly used b3 multi as couldn't tell remote or not

Aside: there are a couple binary formats based on B3, notably Finagle and most recently RSocket. We haven't formalized either, but it is the more likely to be standardized, should it work out for them.

[carlosalberto]

Based on the suggestion, and for simplicity purposes and given the time constrains, I suggest we require that:

1. Extraction attempts SINGLE (higher priority) and then MULTI.
2. Injection defaults to SINGLE, but can be configured to inject MULTI instead.

And have a suggestion for SIGs to do the extended logic as described above. @bogdandrutu feels like a good compromise?

[codefromthecrypt]

if you default B3 inject to single for HTTP/RPC, the risk is breaking downstream users who are using outdated or unmaintained instrumentation.

For example, I know some are still using census, and there are some parts that haven't been maintained to support b3 single. For example, https://github.com/envoyproxy/envoy/blob/c5f4302325223765b660f0f366ce25bf8570e7a5/source/extensions/tracers/zipkin/span_context_extractor.cc does, but https://github.com/envoyproxy/envoy/blob/9ad7d4ce5923fb053506cb76d47f93049a509a4c/source/extensions/tracers/opencensus/opencensus_tracer_impl.cc wasn't ported to do the same.

[carlosalberto]

if you default B3 inject to single for HTTP/RPC, the risk is breaking downstream users who are using outdated or unmaintained instrumentation.

Oh, fair enough. So, in that case, as a GA-reachable goal, we either go with:

1. Original @mwear's suggestion (MULTI has higher priority, to increase compatibility)
2. B3 Propagator that injects/extracts BOTH formats, with SINGLE having higher priority.

And as an after-GA goal, implement the logic described above ;) @bogdandrutu Opinion on this?

[bogdandrutu]

@carlosalberto I see that we actually need to support both, and have a default for inject. So personally I am fine with your proposal:

Extraction attempts SINGLE (higher priority) and then MULTI.
Injection defaults to SINGLE, but can be configured to inject MULTI instead.

As long as this is a config, user can tune this (indeed may require some work from the user) but it is fine as the starting point.

[yurishkuro]

@carlosalberto please approve if you agree