Send span.kind to jaeger and overwrite grpc metadata instead of appending #441
Conversation
amrmahdi
requested review from
iredelmeier,
jmacd,
lizthegrey,
paivagustavo,
rghetia and
tedsuo
as code owners
|
I signed it |
| } | ||
|
|
||
| func (s *metadataSupplier) Set(key string, value string) { | ||
| s.metadata.Append(key, value) | ||
| s.metadata.Set(key, value) |
There was a problem hiding this comment.
I feel like this needs an explanation. There's no testing here, which suggests the correct behavior might not be well understood. Can you add some detail?
There was a problem hiding this comment.
This follows the same behavior of the httptrace. What happens is if there is an existing traceparent metadata in the context (due to an incoming RPC to the server side) and on the server the RPC is forwarded (proxied), the traceparent header will be appended, causing it to be invalid. And hence when it is received on another server, the parsing of the traceparent header fails and assumes an empty context, since the traceparent value is a concatenated with a ,.
There was a problem hiding this comment.
I'd say it's a good change, but maybe we should add a text to the propagation.Supplier interface that it acts on single-valued keys, so implementations of the interface should avoid appending operations.
span.kindtag to jaegermetadataSupplierto overwrite the header if existing instead of appending to it. I found that this is problematic for nested requests i.e. client -> server1 -> server1-client -> etc ...