unused bit in trace flag to not fail traceparent parsing

[leonzz]

W3C standard says the interpretation of trace flag should only look for the supported bit, in this case the sampled bit. Therefore the presence of unused bit should not fail the parsing.

ref: https://www.w3.org/TR/trace-context/#trace-flags

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.5%. Comparing base (3f05c91) to head (beb77dc).

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #7398   +/-   ##
=====================================
  Coverage   85.5%   85.5%           
=====================================
  Files        275     275           
  Lines      24614   24609    -5     
=====================================
- Hits       21046   21043    -3     
+ Misses      3189    3187    -2     
  Partials     379     379           

Files with missing lines	Coverage Δ
propagation/trace_context.go	96.4% <ø> (-0.2%)	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dmathieu]

This will need a changelog entry.

Also, while this change looks good, I think I recall a discrepency in the specification. I can't find that anymore though.

[MrAlias]

The current version of this specification (00) only supports a single flag called sampled.

The behavior of other flags, such as (00000100) is not defined and is reserved for future use. Vendors MUST set those to zero.

... [t]he vendor will only parse the trace-flags values supported by this version of this specification and ignore all other values. [...] Vendors will set all unparsed / unknown trace-flags to 0 on outgoing requests.

Please provide evidence that the "W3C standard says the interpretation of trace flag should only look for the supported bit"

[MrAlias]

Possible duplicate of #5075

[leonzz]

The current version of this specification (00) only supports a single flag called sampled.

The behavior of other flags, such as (00000100) is not defined and is reserved for future use. Vendors MUST set those to zero.

... [t]he vendor will only parse the trace-flags values supported by this version of this specification and ignore all other values. [...] Vendors will set all unparsed / unknown trace-flags to 0 on outgoing requests.

Please provide evidence that the "W3C standard says the interpretation of trace flag should only look for the supported bit"

The W3C spec mentions (in the Processing Model section) that The vendor will only parse the trace-flags values supported by this version of this specification and ignore all other values . If parsing fails, the vendor creates a new traceparent header and deletes the tracestate.

I think it's a matter of how do we interpret the above text, should the step of ignore all other values be considered as parsing fails.

IMO it should not be considered as parsing failure because the W3C spec also mentions (in the Trace Flags section) that you cannot interpret flags by decoding the hex value and looking at the resulting number. ... A common mistake in bit fields is forgetting to mask when interpreting flags.

The other part of the spec other flags ... Vendors MUST set those to zero governs the outgoing headers, not the parsing of incoming one. Since we discard the unused bit during parsing, the outgoing one will meet the requirement.

Also, failing the parsing just because of unknown bit would decrease interoperability with potential future versions, for example the level 2 proposal of W3C trace context adds a new bit in the trace flag while keeping the same version.

Possible duplicate of #5075

it's a bit different, IIUC that one was to keep the unknown bit even in the outgoing header, which violates the spec of other flags ... Vendors MUST set those to zero

[leonzz]

This will need a changelog entry.

Also, while this change looks good, I think I recall a discrepency in the specification. I can't find that anymore though.

Thanks! Updated the change log as well.