Added OpenTelemetry W3C Trace Context #16168
Conversation
…backward compatibility with Zipkin B3 format. Signed-off-by: Somil Jain <somiljain896@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: SomilJain0112 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Welcome @SomilJain0112! It looks like this is your first PR to knative/serving 🎉 |
knative-prow
bot
added
size/L
|
Hi @SomilJain0112. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
knative-prow
bot
added
ok-to-test
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16168 +/- ##
==========================================
- Coverage 80.08% 80.02% -0.07%
==========================================
Files 214 215 +1
Lines 16940 13290 -3650
==========================================
- Hits 13566 10635 -2931
+ Misses 3015 2295 -720
- Partials 359 360 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pkg/http/trace.go
Outdated
| return b3TraceID | ||
| } | ||
|
|
||
| if b3TraceID := h.Get("X-B3-Traceid"); b3TraceID != "" { |
There was a problem hiding this comment.
Do we need this fallback? I thought Get should look up a canonical version of the key.
| // ExtractTraceID extracts the trace ID from the request headers. | ||
| // It supports both W3C Trace Context (traceparent) and B3 (X-B3-TraceId) formats. | ||
| func ExtractTraceID(h http.Header) string { | ||
| if traceparent := h.Get("traceparent"); traceparent != "" { |
There was a problem hiding this comment.
Can you add //nolint:canonicalheader comment to supress the lint errors here
pkg/http/trace.go
Outdated
| // It supports both W3C Trace Context (traceparent) and B3 (X-B3-TraceId) formats. | ||
| func ExtractTraceID(h http.Header) string { | ||
| if traceparent := h.Get("traceparent"); traceparent != "" { | ||
| parts := strings.Split(traceparent, "-") |
There was a problem hiding this comment.
nit: could use strings.SplitN with N == 3
|
We should also update this test serving/test/e2e/logging_test.go Line 51 in 736b4c4 That was the original reason for the issue |
Signed-off-by: Somil Jain <somiljain896@gmail.com>
|
Hii @dprotaso , |
|
/hold for release we'll revist this after we cut 1.20 today |
knative-prow
bot
added
the
do-not-merge/hold
2 participants
Fixes #
#15990
Proposed Changes
Added OpenTelemetry W3C Trace Context support while maintaining full backward
compatibility with Zipkin B3 format.
Updated all the relevant files.
Release Note
Added support for OpenTelemetry W3C Trace Context (traceparent header) in request logging, while maintaining backward compatibility with Zipkin B3 format.