Skip to content

feat(tornado): Support span streaming#6206

Open
sl0thentr0py wants to merge 4 commits into
masterfrom
neel/span-first/tornado
Open

feat(tornado): Support span streaming#6206
sl0thentr0py wants to merge 4 commits into
masterfrom
neel/span-first/tornado

Conversation

@sl0thentr0py
Copy link
Copy Markdown
Member

Issues

@sl0thentr0py sl0thentr0py requested a review from a team as a code owner May 5, 2026 13:32
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 5, 2026

@sl0thentr0py sl0thentr0py changed the title ref(tornado): migrate to span-first ref(tornado): migrate to span first May 5, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Codecov Results 📊

282 passed | Total: 282 | Pass Rate: 100% | Execution Time: 42.43s

All tests are passing successfully.

❌ Patch coverage is 19.70%. Project has 14825 uncovered lines.

Files with missing lines (2)
File Patch % Lines
tornado.py 11.04% ⚠️ 145 Missing
consts.py 99.50% ⚠️ 2 Missing

Generated by Codecov Action

@sl0thentr0py sl0thentr0py changed the title ref(tornado): migrate to span first feat(tornado): Support span streaming May 5, 2026
Copy link
Copy Markdown
Contributor

@alexander-alderman-webb alexander-alderman-webb left a comment

Choose a reason for hiding this comment

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

Comment not blocking, looks good to me!

Comment thread sentry_sdk/integrations/tornado.py Outdated
Comment thread tests/integrations/tornado/test_tornado.py Outdated
Comment thread tests/integrations/tornado/test_tornado.py Outdated
span.status = "error" if status_int >= 400 else "ok"


def _get_request_attributes(request: "Any") -> "Dict[str, Any]":
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.

Are we still capturing the request body in the streaming path? See https://getsentry.github.io/sentry-conventions/attributes/http/#http-request-body-data

For prior art IIRC @ericapisani did this for another integration, not sure which exactly

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

starlette and aiohttp were the couple that I worked on that had this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

added req body

Comment thread sentry_sdk/integrations/tornado.py Outdated
Comment thread sentry_sdk/integrations/tornado.py Outdated
span.status = "error" if status_int >= 400 else "ok"


def _get_request_attributes(request: "Any") -> "Dict[str, Any]":
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

starlette and aiohttp were the couple that I worked on that had this

Comment thread sentry_sdk/integrations/tornado.py
@sl0thentr0py sl0thentr0py force-pushed the neel/span-first/tornado branch from 82de412 to a58538d Compare May 7, 2026 13:05
Comment thread sentry_sdk/integrations/tornado.py
Comment thread sentry_sdk/integrations/tornado.py Outdated
Comment thread sentry_sdk/integrations/tornado.py
@sl0thentr0py sl0thentr0py force-pushed the neel/span-first/tornado branch from 036b60a to 4f36419 Compare May 15, 2026 12:34
Comment thread sentry_sdk/integrations/tornado.py Outdated
Comment thread sentry_sdk/integrations/tornado.py Outdated
Comment thread sentry_sdk/integrations/tornado.py
@sl0thentr0py sl0thentr0py force-pushed the neel/span-first/tornado branch 3 times, most recently from 4d430e9 to 2106ec2 Compare May 15, 2026 13:06
@sl0thentr0py sl0thentr0py force-pushed the neel/span-first/tornado branch 2 times, most recently from cce1123 to e390c87 Compare May 15, 2026 13:12
sl0thentr0py and others added 3 commits May 15, 2026 15:14
Add span-streaming support to the Tornado integration. When span
streaming is enabled, the request handler emits a StreamedSpan with
HTTP request attributes (method, headers, query, URL, client address)
and sets the response status on completion. The legacy transaction
path is preserved for non-streaming mode.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sl0thentr0py sl0thentr0py force-pushed the neel/span-first/tornado branch from e390c87 to ec6fd01 Compare May 15, 2026 13:14
Comment on lines +197 to +201
if request.query:
attributes[SPANDATA.HTTP_QUERY] = request.query

attributes[SPANDATA.URL_FULL] = "%s://%s%s" % (
request.protocol,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The Tornado integration incorrectly populates url.full without the query string and uses the wrong attribute key (http.query) for the query string itself.
Severity: LOW

Suggested Fix

To fix the url.full attribute, append request.query if it exists, prefixed with a ?. To address the incorrect attribute key, assign request.query to SPANDATA.URL_QUERY (url.query) instead of SPANDATA.HTTP_QUERY. Alternatively, if using http.query is desired, prepend a ? to request.query before assignment.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry_sdk/integrations/tornado.py#L197-L201

Potential issue: In the Tornado integration's span streaming path, the `url.full`
attribute is constructed using `request.path`, which omits the query string, contrary to
OpenTelemetry and Sentry conventions. Additionally, the request's query string, which
lacks a leading `?`, is assigned to the `http.query` attribute (`SPANDATA.HTTP_QUERY`).
Per Sentry's constants, `http.query` should include the `?`, and the query string
without the `?` should be assigned to `url.query` (`SPANDATA.URL_QUERY`). This results
in non-compliant and potentially misleading span data.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1fa438a. Configure here.

Comment thread sentry_sdk/integrations/tornado.py
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.

Migrate tornado to span first

4 participants