lightstep / lightstep-tracer-go

The Lightstep distributed tracing library for Go
https://lightstep.com
MIT License
98 stars 54 forks source link

Use map instead of string to configure propagators #239

Closed codeboten closed 4 years ago

codeboten commented 4 years ago

This enables users to configure custom propagators.

Signed-off-by: Alex Boten aboten@lightstep.com

codeboten commented 4 years ago

closes #239

codeboten commented 4 years ago

@iredelmeier I think you're right, the plan was to eventually implement a PropagatorStack to support multiple propagators as the java implementation: https://github.com/lightstep/lightstep-tracer-java-common/blob/master/common/src/main/java/com/lightstep/tracer/shared/PropagatorStack.java

codeboten commented 4 years ago

Though re-reading the implementation in csharp and java, the interface for PropagatorStack is the same as a Propagator, so this change works.

iredelmeier commented 4 years ago

@codeboten I think my understanding is the same as yours re: Java and C#? That is, a PropagatorStack is still a Propagator, but it's implemented as an array of n propagators.

The split across different propagator formats makes this a bit more painful... :/

codeboten commented 4 years ago

Right, the difference being with the PropagatorStack, the format has to be specified at creation, where as with this implementation, which is similar to the javascript/ruby implementations, a map of formats can be specified and propagators for each format configured.

I suggest merging this change, and in a later change providing an implementation of the PropagatorStack to support multiple propagators per format.

codeboten commented 4 years ago

Added a PropagatorStack to support multiple formats.

codeboten commented 4 years ago

Good question, we could support something like

     propagators:
        http_headers:
            ls
            b3
        text_map:
            ls
        binary:
            binary

Although that wouldnt account for custom propagators