Skip to content

[API] Refactor slicing code#7446

Merged
martincostello merged 2 commits into
open-telemetry:mainfrom
martincostello:refactor-slicing
Jun 23, 2026
Merged

[API] Refactor slicing code#7446
martincostello merged 2 commits into
open-telemetry:mainfrom
martincostello:refactor-slicing

Conversation

@martincostello

Copy link
Copy Markdown
Member

Noticed while working on #7433.

Changes

Use IsEmpty and consume the slice rather than tracking an index.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

Use `IsEmpty` and consume the slice rather than tracking an index.
@martincostello
martincostello marked this pull request as ready for review June 23, 2026 07:54
@martincostello
martincostello requested a review from a team as a code owner June 23, 2026 07:54
Copilot AI review requested due to automatic review settings June 23, 2026 07:54
@github-actions github-actions Bot added the pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package label Jun 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the tracestate parsing loop in TraceContextPropagator to consume the ReadOnlySpan<char> as it is parsed (using IsEmpty and slicing), instead of tracking a separate index. This is part of the API-layer W3C Trace Context propagation logic.

Changes:

  • Updated the TryExtractMultipleTracestate loop to parse comma-separated members by advancing the span (tracestate = tracestate.Slice(...)) until empty.
  • Simplified slice/index usage by calling IndexOf(',') on the remaining span rather than slicing from an external begin offset.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs Outdated
@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.98%. Comparing base (88fd1f0) to head (780e201).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7446   +/-   ##
=======================================
  Coverage   89.98%   89.98%           
=======================================
  Files         277      277           
  Lines       14624    14623    -1     
=======================================
  Hits        13159    13159           
+ Misses       1465     1464    -1     
Flag Coverage Δ
unittests-Project-Experimental 89.92% <100.00%> (+0.01%) ⬆️
unittests-Project-Stable 89.92% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
....Api/Context/Propagation/TraceContextPropagator.cs 91.25% <100.00%> (+0.26%) ⬆️

... and 2 files with indirect coverage changes

Assign directly to an empty span instead of slicing away the whole span.
@martincostello
martincostello enabled auto-merge June 23, 2026 08:23
@martincostello
martincostello added this pull request to the merge queue Jun 23, 2026
Merged via the queue into open-telemetry:main with commit e4e39ee Jun 23, 2026
74 checks passed
@martincostello
martincostello deleted the refactor-slicing branch June 23, 2026 08:56
@martincostello martincostello added this to the v1.17.0 milestone Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants