Skip to content

fix(starlette): Set transaction name on current scope in sync handler - #7201

Draft
mjq wants to merge 1 commit into
masterfrom
mjq/starlette-sync-scope-fix
Draft

fix(starlette): Set transaction name on current scope in sync handler#7201
mjq wants to merge 1 commit into
masterfrom
mjq/starlette-sync-scope-fix

Conversation

@mjq

@mjq mjq commented Aug 13, 2026

Copy link
Copy Markdown
Member

The sync request/response handler passed the isolation scope to _set_transaction_name_and_source, but the transaction/segment span lives on the current scope. As a result the route-resolved name never reached the span for sync endpoints, which were instead named by the raw URL from the ASGI middleware (transaction_info.source of url rather than route). Async handlers already used the current scope and were unaffected.

Pass the current scope (already computed above) so sync and async handlers behave identically:

  • streaming: the segment name / sentry.segment.name.source are route-based
  • static: the transaction event name / source are route-based

For parametrized routes this also removes high-cardinality URL transaction names for sync endpoints.

Found while working on #7183.

The sync request/response handler passed the _isolation_ scope to
`_set_transaction_name_and_source`, but the transaction/segment span lives on
the _current_ scope. As a result the route-resolved name never reached the span
for sync endpoints, which were instead named by the raw URL from the ASGI
middleware (`transaction_info.source` of `url` rather than `route`). Async
handlers already used the current scope and were unaffected.

Pass the current scope (already computed above) so sync and async handlers
behave identically:
- streaming: the segment name / `sentry.segment.name.source` are route-based
- static: the transaction event name / source are route-based

For parametrized routes this also removes high-cardinality URL transaction
names for sync endpoints.
@mjq
mjq force-pushed the mjq/starlette-sync-scope-fix branch from e043a2f to b6107ba Compare August 13, 2026 20:44

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit b6107ba. Configure here.

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Codecov Results 📊

113327 passed | ⏭️ 6769 skipped | Total: 120096 | Pass Rate: 94.36% | Execution Time: 418m 15s

📊 Comparison with Base Branch

Metric Change
Total Tests 📈 +173
Passed Tests 📈 +170
Failed Tests 📉 -1
Skipped Tests 📈 +4

➖ Removed Tests (1)

View removed tests
  • test_span_streaming_quart_auth_user_id[42-True]
    • File: tests.integrations.quart.test_quart

All tests are passing successfully.

✅ Patch coverage is 100.00%. Project has 2490 uncovered lines.
✅ Project coverage is 90.21%. Comparing base (base) to head (head).

Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    90.21%    90.21%        —%
==========================================
  Files          193       193         —
  Lines        25423     25423         —
  Branches      9336      9336         —
==========================================
+ Hits         22933     22933         —
- Misses        2490      2490         —
- Partials      1436      1437        +1

Generated by Codecov Action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant