trace: ParseTraceState ignores duplicated tracestate keys#4638
trace: ParseTraceState ignores duplicated tracestate keys#4638pellared wants to merge 5 commits intoopen-telemetry:mainfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4638 +/- ##
=======================================
- Coverage 81.3% 81.3% -0.1%
=======================================
Files 222 222
Lines 17682 17682
=======================================
- Hits 14391 14389 -2
- Misses 2991 2993 +2
Partials 300 300
|
|
|
||
| if _, ok := found[m.Key]; ok { | ||
| return TraceState{}, wrapErr(errDuplicate) | ||
| continue // Duplicate is ignored per https://github.com/w3c/trace-context/blob/551a5b0855171281e98b4c2a814bf9e1f837cd53/test/test.py#L563-L568. |
There was a problem hiding this comment.
This is a link to W3C unit tests. Can you instead link to the part of the W3C specification that describes this behavior?
There was a problem hiding this comment.
I have not found anything better than the test code in the specification that describes it. See: https://github.com/instana/trace-context/blob/master/spec/20-http_request_header_format.md
There was a problem hiding this comment.
Do you have any other suggestion or can this comment be resolved?
There was a problem hiding this comment.
It seems like this behavior is only mentioned in the editor draft of the W3C trace-context as optional. This PR seems premature given that unreleased state.
There was a problem hiding this comment.
It seems like this behavior is only mentioned in the editor draft of the W3C trace-context as optional.
For me even the editor draft does not clearly defines this behavior thus I created w3c/trace-context#548
Inspired by open-telemetry/opentelemetry-dotnet#4937 (kudos to @Kielek)
The change is a follow-up on the changes done in the Trace Context Specification:
I also created for completeness: w3c/trace-context#547