Add baggage to B3 codec #438
Conversation
ghost
assigned 
Codecov Report
@@ Coverage Diff @@
## master #438 +/- ##
============================================
+ Coverage 87.41% 87.55% +0.13%
- Complexity 511 517 +6
============================================
Files 65 65
Lines 1979 2000 +21
Branches 259 263 +4
============================================
+ Hits 1730 1751 +21
Misses 160 160
Partials 89 89
Continue to review full report at Codecov.
|
|
we should decide whether baggage will be enabled by default or not. However our baggage management is different and separate from the codec. Which makes me think that we can remove Brave/Sleuth has to be configured explicitly which keys are whitelisted for propagation. |
|
+1 to integrate with baggage manager. |
|
Done in the last commit |
|
@yurishkuro could you please review? |
* it's enabled by default * prefix is configurable Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
| */ | ||
| public class B3TextMapCodec implements Codec<TextMap> { | ||
| protected static final String TRACE_ID_NAME = "X-B3-TraceId"; | ||
| protected static final String SPAN_ID_NAME = "X-B3-SpanId"; | ||
| protected static final String PARENT_SPAN_ID_NAME = "X-B3-ParentSpanId"; | ||
| protected static final String SAMPLED_NAME = "X-B3-Sampled"; | ||
| protected static final String FLAGS_NAME = "X-B3-Flags"; | ||
| protected static final String BAGGAGE_NAME = "baggage-"; |
| // NOTE: uber's flags aren't the same as B3/Finagle ones | ||
| protected static final byte SAMPLED_FLAG = 1; | ||
| protected static final byte DEBUG_FLAG = 2; | ||
|
|
||
| private static final PrefixedKeys keys = new PrefixedKeys(); | ||
| private final String baggageName; |
There was a problem hiding this comment.
ditto call it prefix instead?
| @@ -80,12 +104,37 @@ public SpanContext extract(TextMap carrier) { | |||
| if (entry.getValue().equals("1")) { | |||
| flags |= DEBUG_FLAG; | |||
| } | |||
| } else if (entry.getKey().startsWith(BAGGAGE_NAME)) { | |||
There was a problem hiding this comment.
should this be this.baggageName?
| if (baggage == null) { | ||
| baggage = new HashMap<String, String>(); | ||
| } | ||
| baggage.put(keys.unprefixedKey(entry.getKey(), BAGGAGE_NAME), entry.getValue()); |
Signed-off-by: Pavol Loffay <[email protected]>
|
@black-adder PR updated. |
| return new SpanContext(traceId, spanId, parentId, flags); | ||
| SpanContext spanContext = new SpanContext(traceId, spanId, parentId, flags); | ||
| if (baggage != null) { | ||
| spanContext = spanContext.withBaggage(baggage); |
There was a problem hiding this comment.
this is an interesting use case, what if only baggage is propagated without the rest of the trace context?
There was a problem hiding this comment.
At the moment it's an invalid span context - null
Related to jaegertracing/jaeger#755 (comment)
Signed-off-by: Pavol Loffay [email protected]