Feature Request: implement W3C TraceContext traceparent header extract/inject

[ecourreges-orange]

In order to be future proof, since uber-trace-id is not coded yet, I think issue #22 can be abandoned, and replaced with W3C TraceContext traceparent header implementation.

I am thinking of adding support for extraction of the traceparent header, which doesn't need to be a different plugin:
I would simply code it as "if traceparent found, then use it, if not, revert to B3 headers".

As for the injection propagation, I haven't looked at the code of this plugin yet, but I imagine it is better to be able to configure the output injection format, or worst case "use in output the format you received in input, with a default for traceparent".

My timeline for this is not necessarily very short though, somewhere between end march and june...

[ecourreges-orange]

Actually, sorry, don't count on me for this functionnality, the team that uses Kong in my company is in version 0.12 which doesn't work with this plugin anyway, so I'd rather wait for them to upgrade than try to backport the plugin for 0.12 :-(
Now that you have b3 single header merged in master, the plugin is not far from supporting traceparent which is very close in format to tracestate: b3=...

[subvocal]

Hi @ecourreges-orange - I'm implementing W3C Trace Context support and have it working in my fork. I'm not considering it complete enough for a PR until I figure out what the behavior should be for processing. My thoughts on what you proposed for the implementation behavior:

"if traceparent found, then use it, if not, revert to B3 headers".

This makes sense. I've been considering leaving B3 as the first header since it's more likely to be present until W3C TC is more widely adopted. This makes it easier to support situations where both B3 and W3C TC headers are present. I expect both headers would contain the same traceid, etc but W3C TC should override, if not. (see next section for a case when both would be present)

As for the injection propagation, I haven't looked at the code of this plugin yet, but I imagine it is better to be able to configure the output injection format, or worst case "use in output the format you received in input, with a default for traceparent".

Right now, my fork of the plugin is emitting both the W3C TC and the B3 header. I agree that this should be configurable and also think that the configuration should allow for the option of emitting both. Companies might have a mixed environment where some tracers use B3 and others use W3C TC so emitting just one might break traces. Emitting both headers allows for cross-compatibility in mixed environments. Once people upgrade all tracers to use W3C TC, they can change the config to only emit W3C TC.

What do you think?

[subvocal]

@ecourreges-orange - I submitted a PR that adds W3C Trace Context support. Please feel free to comment there: #69

[seh]

Now that we merged #75, that covers consumption of the W3C "traceparent" header. Is the plan here to also emit that header from Kong to the upstream targets?

[More reading...]

Oh, I missed the set function that propagates the header in various formats. Given that, is this work done?

[subvocal]

I have the same question...
Looks like Kong bundles the Zipkin plugin with the Kong release artifact. It's not clear to me how that bundling works and if there needs to be a Kong release for that to happen.

[seh]

Right now, Kong is taking the latest release before minor version 0.3. We'll need a new release issued here in order for Kong to pick up this new capability.

By the way, I tested the W3C Trace Context support today, and it worked flawlessly.

[seh]

One subtlety that could be clearer in the documentation: By my reading of this line in the source, if there's no inbound trace context and the Zipkin plugin is synthesizing a context to send to the upstream server, if the header_type configuration field is left as "preserve," the plugin sends the trace context in B3 format.

I'd like to be able to preserve an incoming context's format, but send synthesized contexts in W3C Trace Context format. As written today. it looks like the only way to emit the W3C Trace Context format is to receive a "traceparent" header or set header_type to "w3c".

[kikito]

Hi @seh , I think I understand what you want, but I am not sure how to translate that into a plugin configuration. Do you have something in mind on this regard?

[seh]

I think we'd need a second knob there like header_type. Coming up with a name is difficult and controversial, but as a straw man, consider fresh_header_type or synthesized_header_type. It could take on all values accepted by header_type except "preserve." If we receive an appropriate header, the header_type value determines how to send it upstream. If we don't receive an appropriate header, the hypothetical fresh_header_type value would determine how to send a header upstream.

I suppose that fresh_header_type could even accept a sentinel "none" value, indicating that if we don't receive a header, we should not synthesize one to send upstream.

[kikito]

Hi again,

I think I might have misunderstood before (apologies, we're dealing with subtle things here).

I'd like to be able to preserve an incoming context's format, but send synthesized contexts in W3C Trace Context format. As written today. it looks like the only way to emit the W3C Trace Context format is to receive a "traceparent" header or set header_type to "w3c".

You might have missed an important bit: the plugin always preserves the incoming context format.

There's three cases. The first two, I think you are clear enough:

• header_type = 'preserve'. In this case "you get what you put in the request", defaulting to B3.
• header_type = 'w3c' + incoming request has the same header. The plugin passes along the w3c header.

The third one is where I think there is a misunderstanding:

• header_type = 'w3c' + incoming request has a different tracing header.

On this case (or in any other combination where the configured header type and the inbound request header type differ) what happens is: both kinds of headers are added to the outbound request. In other words: if header_type == 'w3c' and received_header == 'b3-single', then the request sent upwards will have both: a w3c format and a b3-single format.

Also, when this mismatch is found, a warning is logged.

This is all to say that I think your needs might be met by just setting header_type to w3c. (And perhaps ignoring some warnings in the logs).

This behavior might be more apparent by looking at the specs:

https://github.com/Kong/kong-plugin-zipkin/blob/master/spec/tracing_headers_spec.lua#L442-L467

After this explanation, do you still think a new config option is needed?

[seh]

Yes, I still think a new option is necessary. I appreciate your thorough explanation, and I was able to glean most of that from reading code.

What you didn't address as a fourth case is what happens when the incoming request has no header. You did mention that for "preserve," the default is B3, which perhaps says as much indirectly. I suppose you're saying that one gets what she puts into the request, but if there was nothing in the request, she gets the default. If that's the proper reading, then the first case is really two separate cases.

By my reading of the code, there's no way to force a synthesized trace context for a request that had no such header to be sent upstream as a W3C Trace Context. It will create fresh trace and span IDs and send them upstream in B3 format. If my upstream server doesn't understand B3 format, it can't participate in this trace.

[kikito]

Understood, thanks for answering. I will add a default_header_type option (defaulting to B3) which will be used when preserve is used and no header is included on the inbound request.

[seh]

That's great to hear. Just to make sure I understand why this new field would only be consulted when used in concert with a header_type value of "preserve", is that because you intend to change the code to honor values like "b3" and "w3c" when sending headers upstream for synthesized trace contexts? Right now, such synthesized contexts always wind up in the branch emitting B3 headers.

That is, if I want my server to always receive W3C Trace Context headers, I could set header_type to "w3c". I might still also receive additional headers in other formats, depending on what clients send, but I'd always receive W3C Trace Context headers, even for synthesized trace contexts when the client didn't send any headers originally. Is that correct?

[seh]

@kikito, have you started in on this change? I'm still wondering about the interaction of header_type, "preserve", and the proposed default_header_type.

[kikito]

Hello, we were busy at Kong releasing 2.1.0 so work on Zipkin had to be postponed (and now summer holidays are comming soon). Implementing this change is next on my priority list.

[kikito]

Hello @seh , if it's at all possible for you, please give #93 a look to validate that it suits your needs.

[kikito]

Hi, I think this issue was implemented by #93, so I am closing down the issue. Please reopen if this is not the case.

[seh]

Though I had seen your message asking for review almost a month ago, I wasn't working with Kong at the time, and I let it slip from my mind. Today I reviewed #93, and it looks like it satisfies the concerns I had expressed here—especially removing what had been line 287 in file tracing_headers.lua. Thank you very much. I look forward to trying this in action soon.