Skip to content

Add request-side URL/body span attributes; split up RawTask._result - #331

Merged
o-nnerb merged 2 commits into
mainfrom
claude/swift-tracing-impact-analysis-470028
Sep 4, 2026
Merged

o-nnerb merged 2 commits into
mainfrom
claude/swift-tracing-impact-analysis-470028

Conversation

@o-nnerb

@o-nnerb o-nnerb commented Sep 4, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds url.scheme, server.address, server.port, url.path, and http.request.body.size to the request span RawTask starts, on top of the existing http.request.method/url.full. This mirrors what async-http-client's own built-in tracing gained in swift-server/async-http-client#906 (released in 1.36.1) — previously it only set http.request.method. url.query is deliberately left out since query strings can carry tokens/PII that shouldn't land on a span by default.
  • Splits RawTask._result(environment:), which had grown into one ~210-line function doing hook notification, executor/network-path validation, client resolution, and the whole span lifecycle, into focused private methods (notifyDescriptorHooks, validateRequiredExecutor, waitForNetworkPath, resolveClient, runSession, executeTraced, startRequestSpan, setURLAttributes, endRequestSpan).

Why breaking-changes

No public API changed — this is an internal refactor plus additive span attributes. Labeled breaking-changes per explicit request rather than an assessment that anything actually breaks for consumers.

Design notes

  • The refactor preserves task-local propagation timing exactly: the code this file exists for (owning the span lifecycle itself so ServiceContext.current survives, instead of delegating to async-http-client's tracing, which loses it on an EventLoop hop) is unaffected, since the extracted methods are still called synchronously within the same task — no new Task/EventLoop boundary was introduced.
  • server.port falls back to the scheme's default (80/443) when the URL has none, mirroring async-http-client's DeconstructedURL/Scheme.defaultPort.

Test plan

  • swift build — clean build
  • swift test --filter "RequestServiceContextTests|SessionTests|RawTask" — 50/50 passing
  • swift test --filter RequestDLTests — full suite, 1123/1123 passing

🤖 Generated with Claude Code

RawTask's span now sets url.scheme, server.address, server.port,
url.path and http.request.body.size alongside the existing
http.request.method/url.full — matching what async-http-client's own
built-in tracing gained in swift-server/async-http-client#906
(released in 1.36.1). url.query is deliberately left out since query
strings can carry tokens/PII.

Also splits RawTask._result(environment:), which had grown into one
~210-line function covering hook notification, executor/network-path
validation, client resolution, and the whole span lifecycle, into
focused private methods. Behavior and task-local propagation timing
(the ServiceContext/EventLoop-hop workaround this file exists for)
are unchanged; the extracted methods are called synchronously in the
same task, so no propagation boundary moved.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@o-nnerb o-nnerb added the breaking-changes This PR is a new major version release label Sep 4, 2026
@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.61%. Comparing base (600df4e) to head (e2d311b).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #331   +/-   ##
=======================================
  Coverage   98.61%   98.61%           
=======================================
  Files           5        5           
  Lines        1082     1082           
=======================================
  Hits         1067     1067           
  Misses         15       15           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@o-nnerb
o-nnerb merged commit 65cc42e into main Sep 4, 2026
19 checks passed
@o-nnerb
o-nnerb deleted the claude/swift-tracing-impact-analysis-470028 branch September 4, 2026 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-changes This PR is a new major version release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant