The first baggage item cannot be added in the second service

[william-tran]

Consider services A -> B -> C where calls are over HTTP. A does not add baggage. When B calls Span.setBaggageItem(k,v) an exception is thrown:

java.lang.UnsupportedOperationException: null
    at java.base/java.util.AbstractMap.put(AbstractMap.java:209) ~[na:na]
    at datadog.trace.agent.ot.DDSpanContext.setBaggageItem(DDSpanContext.java:260) ~[na:na]
    at datadog.trace.agent.ot.DDSpan.setBaggageItem(DDSpan.java:212) ~[na:na]
    at datadog.trace.agent.ot.DDSpan.setBaggageItem(DDSpan.java:33) ~[na:na]
    at com.acme.myservice.MyService.doSomething(MyService.java:42) ~[classes/:na]

This happens because:

1. DatadogHttpCodec.extract first initializes the baggage local variable as a Collections.emptyMap: https://github.com/DataDog/dd-trace-java/blob/v0.33.0/dd-trace-ot/src/main/java/datadog/opentracing/propagation/DatadogHttpCodec.java#L63,
2. Then only replaces it with a mutable map if the upstream service populated baggage https://github.com/DataDog/dd-trace-java/blob/v0.33.0/dd-trace-ot/src/main/java/datadog/opentracing/propagation/DatadogHttpCodec.java#L86-L90
3. The ExtractedContext is passed that emptyMap and is returned by DatadogHttpCodec.extract. It is then used to create the DDSpanContext in DDTracer.buildSpanContext: https://github.com/DataDog/dd-trace-java/blob/v0.33.0/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java#L643-L649

We could fix this in
a) DatadogHttpCodec.extract by never returning a Collections.emptyMap
b) DDTracer.buildSpanContext by only doing the assignment if ExtractedContext's baggage is not empty
c) DDSpanContext() by doing this.baggageItems = new ConcurrentHashMap<>(baggageItems) instead of the straight assignment

Assuming a CHM is necessary in DDSpanContext, c) would be the best bet as it guarantees thread safety; we've otherwise been allowing both CHM and regular HashMaps from ExtractedContext.

[randomanderson]

Thanks for this investigation. I merged your pull request.

I think the code in DatadogHttpCodec, the other codecs, and DDSpanContext can use some further cleanup and simplification. I'll look into it at a later date.