Fix parsing trace parent in W3CPropagator

[tarekgh]

Fixes #117500

[Copilot]

Pull Request Overview

This PR fixes the version validation logic in W3CPropagator so that only a version of "ff" is rejected, and adds comprehensive tests in W3CPropagatorTests to verify a variety of valid and invalid trace-parent inputs.

• Updated IsInvalidTraceParent to only reject when both version characters are 'f' instead of either one.
• Added a [Theory] with W3CTraceParentData covering many edge cases for trace-parent parsing.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/libraries/System.Diagnostics.DiagnosticSource/tests/W3CPropagatorTests.cs	Added W3CTraceParentData with cases for valid/invalid trace-parent strings
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/W3CPropagator.cs	Changed version check to traceParent[0] == 'f' && traceParent[1] == 'f'

Comments suppressed due to low confidence (1)

src/libraries/System.Diagnostics.DiagnosticSource/tests/W3CPropagatorTests.cs:343

• The comment indicates that all zeros parent id is valid, but the test expects it to be invalid. Update the comment to 'all zeros parent id is invalid' to match the expected behavior.

            yield return new object[] { "00-0af7651916cd43dd8448eb211c80319c-0000000000000000-01", false }; // all zeros parent id is valid

[tarekgh]

CC @KalleOlaviNiemitalo

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-activity
See info in area-owners.md if you want to be subscribed.

[KalleOlaviNiemitalo]

AFAICT this fixes the bug I reported.

The parsing of traceparent headers with versions higher than "00" still does not seem to comply with https://www.w3.org/TR/2021/REC-trace-context-1-20211123/#versioning-of-traceparent, which allows more than 55 characters (but not fewer). I have not tested that yet, and I think I should file that as a separate issue.

[tarekgh]

@KalleOlaviNiemitalo

The parsing of traceparent headers with versions higher than "00" still does not seem to comply with https://www.w3.org/TR/2021/REC-trace-context-1-20211123/#versioning-of-traceparent, which allows more than 55 characters (but not fewer). I have not tested that yet, and I think I should file that as a separate issue.

Although this is unlikely to happen, I have fixed that with the last commit. Thanks for pointing at that.

[KalleOlaviNiemitalo]

There is a 55-character limit in Activity.IsW3CId(string id) too, and it has been there for years. That is part of why I thought it would be better handled as a separate issue.

[tarekgh]

There is a 55-character limit in Activity.IsW3CId(string id) too, and it has been there for years. That is part of why I thought it would be better handled as a separate issue.

I’d recommend getting @noahfalk’s input on this in case there are any app compatibility implications with making that change. I updated the W3CPropagator since it's a new feature and doesn't raise compatibility concerns.

Honestly, I don’t see this as a high-priority issue—we can always address it later if a new version is introduced in the W3C spec. It’s not worth fixing right now, but filing a bug to track it is perfectly reasonable.

[noahfalk]

👍

[tarekgh]

/ba-g the filature is a timeout in the run.