W3c trace context #188
W3c trace context #188
Conversation
|
I want to call out that my introduction of What's our stance on including classes in the I can always move the class into some internal only module (it will be a useful tool for migrating away from |
…d, and DistributedTracePayload. add test coverage insertDistributedTraceHeaders and acceptDistributedTraceHeaders
|
I've added an additional PR to resurrect an updated version of a test class for the new distributed trace APIs. I've also deprecated the old APIs. |
| acceptDistributedTracePayload(payload); | ||
| } | ||
|
|
||
| void publicApiAcceptDistributedTracePayload(DistributedTracePayload payload) { |
There was a problem hiding this comment.
What the.... did these methods actually serve any purpose? Or am I seeing correctly that all they did was wrap another method call? Weird...
There was a problem hiding this comment.
You're seeing it correctly. I didn't understand either. Went back to the PR where the code was added and didn't find any context that indicated intent.
@jack-berg I think that our approach has generally been that if any critical problem occurs a customer should always be able to quickly remove the agent jar and be confident that all API calls left in their code will simply default to a NoOp version. Because of this approach we can make the guarantee that there is zero impact on the customers environment when the agent jar is removed but API calls remain in their code. While I'm having a hard time thinking of anything that might actually go wrong with the |
|
@jasonjkeller that seems like a good guarantee to make. In this case, I believe the addition of |
|
@tspring @XiXiaPdx Based on the discussion above, can either of you think of any reasons to not include the Here's a practical example of the new APIs being used to propagate DT headers via Kafka record headers: |
A different take on the W3C trace context APIs using a newly introduced
Headersinterface. This is a unifying interface forInboundHeadersandOutboundHeaders, and makes up for a deficiency inInboundHeaderthat has necessitatedExtendedInboundHeaders.If we like this approach, we could deprecate
InboundHeadersandOutboundHeadersand slowly transition all usages toHeaders. BecauseHeadersextends bothInboundHeadersandOutboundHeaders, this transition could be done in small steps.I've also included a default implementation of the new headers interface called
ConcurrentHashMapHeaders, which is thread safe and uses aConcurrentHashMapas the backing data structure for the implementation. It seems necessary to provide users of the new APIs a reasonable default implementation for ease of use.Closes #169.
Closes #170.