[WIP] Context propagation

[krnowak]

This is my initial proposal for context propagation, which more or less follows OTEP 42:

• changes in the api/distributedcontext package:
  • replace Map with Correlations and Baggage
    • Correlations only allows to add more stuff to it
    • Baggage gives the user more control of the stuff there.
    • both are kept in the context
• changes in the api/propagation package:
  • have two subpackages, binary and http
  • each subpackage have separate interfaces for span context propagator, correlations propagator and baggage propagator, as well as an interface for propagator that contains all the three (for convenience)
  • for now I didn't delete the old code, just told go to ignore it
  • http subpackage also has the chained propagator
• changes in the propagation package:
  • not much, basically port the existing propagators to use the new interfaces
  • for now I didn't delete the old code, but told go to ignore it
• the rest of the code is adapting to the API changes
• questions:
  • should the types in distributedcontext be plain structs or rather interfaces, which SDK can implement (to optimize the way it stores correlations for example)
  • OTEP mentions functions like getHTTPInjector or getHTTPExtractor for baggage or correlations. I didn't implement those - I think that basically picking an implementation of the propagator can be left to the library the wants to propagate stuff.
• TODO:
  • documentation
  • tests
  • use this API in opentracing bridge
    • this is going to be tricky:
      • OT stores baggage in span context, otel it go context
        • I'm still not sure how to store all the baggage coming from either OT or Otel API in a single place, so both APIs can see the baggage set by the other one
      • OT doesn't seem to have any notion of correlations, otel does have it
        • we probably still want to propagate correlations from the bridge anyway

[ghost]

I'm -1 for sub packages for binary and http. I'd much prefer different interfaces, appropriately named, and put them into a propagation package.

[ghost]

IMO propagation.BinarySpanContext reads better than binary.SpanContextPropagator

Think <topic>.<thing> vs <namespace>.<what+topic>. Example: http.Server not server.HTTP

[krnowak]

propagation.BinarySpanContext is a misleading name - it's a propagator, not a span context. The package names are kinda following the scheme from standard library as in encoding/json or encoding/hex, so we have propagation/binary and propagation/http.

[rghetia]

The above signatures of Inject and Extract do not allow to have generic chainInjectors and chainExtractor described in the otep.
However, if these were to change, for example:

Extract(Supplier) context.Context

then the propagator has to extract the spanContext and save it in context.Context. Now we have two keys, one for spanContext and the other for Span. OR propagators have to start a span using the extracted spanContext which has its own issues.

[krnowak]

I have only written chained propagators for specific stuff and they don't exactly match the behaviour from otep, since chained extractors are called until we get the valid span context, correlation or baggage. I'm not sure what's the use of the generic chainExtractor that calls all the extractors…

And about spancontext in the context - should the users of propagators get the span context from the extractor and pass it themselves to Tracer.Start through some of the relationship-establishing option?

[lizthegrey]

What needs to happen to move this forward?

[rghetia]

What needs to happen to move this forward?

I have not been following context-propagation discussion on gitter or otep#66. It seems like otep#66 is ready for approval. If so, next step would be to align this PR to what is agreed in the otep (or maybe wait until the spec is finalized).

[jmacd]

This branch is sadly full of conflicts, particularly stemming from when we moved propagation into the api/propagators. I've begun looking into this and will see if I can rescue this PR.

[jmacd]

I plan to close this PR in a day or two after building a replacement that's closer to OTEP 66. Thanks @krnowak for the prototype.

[jmacd]

Closing this in favor of #381 which tracks the current state of OTEP66.