+ core: provide span propagation codec for "uber-trace-id", default jaeger format #606
Conversation
There was a problem hiding this comment.
Hey @slavaschmidt sorry for the long wait on this one. There are a few minor comments, hopefully we will be able to merge this thing pretty soon!
| private def encodeSamplingDecision(samplingDecision: SamplingDecision): Byte = samplingDecision match { | ||
| case SamplingDecision.Sample => 1 | ||
| case SamplingDecision.DoNotSample => 0 | ||
| case SamplingDecision.Unknown => 1 // the sampling decision is mandatory in this format |
There was a problem hiding this comment.
Wouldn't it be safer to default to DoNotSample in this case?
There was a problem hiding this comment.
yep, I wasn't sure about that. can you see the change?
| val flags = (sampling + (debug << 1)).toHexString | ||
| val headerValue = Seq(span.trace.id.string, span.id.string, parentContext, flags).mkString(Separator) | ||
|
|
||
| writer.write(HeaderName, urlEncode(headerValue)) |
There was a problem hiding this comment.
Why is the value being url-encoded? I couldn't find anything in the Jaeger docs saying that it should be like that 😕
There was a problem hiding this comment.
The TextMapCodec on jaeger side can be configured to both flavors (urlEncoded or not). The urlEncode reflects the default configuration. To formulate it differently, it didn't worked for us out-of-the box otherwise.
|
Hey @slavaschmidt, I just cherry-picked this PR and merged via 25cc4ff. Thanks a lot for the contribution! |
…as described here: https://www.jaegertracing.io/docs/1.7/client-libraries/#propagation-format