Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 80 additions & 48 deletions sentry_sdk/integrations/aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
TransactionSource,
)
from sentry_sdk.tracing_utils import (
add_http_breadcrumb,
add_http_request_source,
has_span_streaming_enabled,
should_propagate_trace,
Expand Down Expand Up @@ -388,57 +389,64 @@ async def on_request_start(
with capture_internal_exceptions():
parsed_url = parse_url(str(params.url), sanitize=False)

breadcrumb = {}

span_name = "%s %s" % (
method,
parsed_url.url if parsed_url else SENSITIVE_DATA_SUBSTITUTE,
)

span: "Union[Span, StreamedSpan, None]"
span: "Union[Span, StreamedSpan, None]" = None
if has_span_streaming_enabled(client.options):
if sentry_sdk.traces.get_current_span() is None:
span = None

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This had to be moved after the PII redaction, because even if we don't want to create a span, we do want to create a breadcrumb, and we need to apply the same PII redacting logic to breadcrumbs.

else:
attributes: "Attributes" = {
"sentry.op": OP.HTTP_CLIENT,
"sentry.origin": AioHttpIntegration.origin,
"http.request.method": method,
}
if parsed_url is not None:
if has_data_collection_enabled(client.options):
url_full = parsed_url.url
attributes["url.path"] = params.url.path

if parsed_url.query:
filtered_query = (
_apply_data_collection_filtering_to_query_string(
query_string=parsed_url.query,
behaviour=client.options["data_collection"][
"url_query_params"
],
)
attributes: "Attributes" = {
"sentry.op": OP.HTTP_CLIENT,
"sentry.origin": AioHttpIntegration.origin,
"http.request.method": method,
}
if parsed_url is not None:
if has_data_collection_enabled(client.options):
url_full = parsed_url.url
attributes["url.path"] = params.url.path

if parsed_url.query:
filtered_query = (
_apply_data_collection_filtering_to_query_string(
query_string=parsed_url.query,
behaviour=client.options["data_collection"][
"url_query_params"
],
)
if filtered_query:
attributes["url.query"] = filtered_query
url_full += "?" + filtered_query

if parsed_url.fragment:
attributes["url.fragment"] = parsed_url.fragment
url_full += "#" + parsed_url.fragment

attributes["url.full"] = url_full
elif should_send_default_pii():
url_full = parsed_url.url
attributes["url.path"] = params.url.path

if parsed_url.query:
url_full += "?" + parsed_url.query
attributes["url.query"] = parsed_url.query
if parsed_url.fragment:
url_full += "#" + parsed_url.fragment
attributes["url.fragment"] = parsed_url.fragment

attributes["url.full"] = url_full

)
if filtered_query:
attributes["url.query"] = filtered_query
url_full += "?" + filtered_query
breadcrumb[SPANDATA.HTTP_QUERY] = filtered_query

if parsed_url.fragment:
attributes["url.fragment"] = parsed_url.fragment
url_full += "#" + parsed_url.fragment
breadcrumb[SPANDATA.HTTP_FRAGMENT] = parsed_url.fragment

attributes["url.full"] = url_full
breadcrumb["url"] = url_full

elif should_send_default_pii():
url_full = parsed_url.url
attributes["url.path"] = params.url.path

if parsed_url.query:
url_full += "?" + parsed_url.query
attributes["url.query"] = parsed_url.query
breadcrumb[SPANDATA.HTTP_QUERY] = parsed_url.query
if parsed_url.fragment:
url_full += "#" + parsed_url.fragment
attributes["url.fragment"] = parsed_url.fragment
breadcrumb[SPANDATA.HTTP_FRAGMENT] = parsed_url.fragment

attributes["url.full"] = url_full
breadcrumb["url"] = url_full

if sentry_sdk.traces.get_current_span() is not None:
span = sentry_sdk.traces.start_span(
name=span_name, attributes=attributes
)
Expand All @@ -453,6 +461,13 @@ async def on_request_start(
legacy_span.set_data("url", parsed_url.url)
legacy_span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query)
legacy_span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment)
breadcrumb.update(
{
SPANDATA.HTTP_QUERY: parsed_url.query,
SPANDATA.HTTP_FRAGMENT: parsed_url.fragment,
"url": parsed_url.url,
}
)
span = legacy_span

if should_propagate_trace(client, str(params.url)):
Expand All @@ -475,18 +490,35 @@ async def on_request_start(
else:
params.headers[key] = value

trace_config_ctx.span = span
trace_config_ctx._sentry_span = span
trace_config_ctx._sentry_breadcrumb = breadcrumb

async def on_request_end(
session: "ClientSession",
trace_config_ctx: "SimpleNamespace",
params: "TraceRequestEndParams",
) -> None:
if trace_config_ctx.span is None:
status = int(params.response.status)

breadcrumb = trace_config_ctx._sentry_breadcrumb
Comment thread
sentrivana marked this conversation as resolved.

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: Accessing trace_config_ctx._sentry_breadcrumb in on_request_end may raise an AttributeError if it wasn't set in on_request_start due to the integration being disabled.
Severity: MEDIUM

Suggested Fix

In on_request_end, guard the access to trace_config_ctx._sentry_breadcrumb. You can use getattr(trace_config_ctx, "_sentry_breadcrumb", None) or a try...except AttributeError block to prevent the crash when the attribute is missing.

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/aiohttp.py#L503

Potential issue: In the `on_request_end` function, `trace_config_ctx._sentry_breadcrumb`
is accessed without first checking if it has been set. If the `on_request_start`
function returns early (for example, if the AioHTTP integration is disabled after a
`ClientSession` is created), `_sentry_breadcrumb` is never assigned. This leads to an
`AttributeError` when `on_request_end` attempts to access it, which can crash the
request handler. This can occur when a `ClientSession` is created while the integration
is enabled, Sentry is then re-initialized without the integration, and a request is
subsequently made using the original session.

if breadcrumb is not None:
breadcrumb.update(
{
SPANDATA.HTTP_METHOD: params.method.upper(),
SPANDATA.HTTP_STATUS_CODE: status,
"reason": params.response.reason,
}
)

add_http_breadcrumb(
status,
breadcrumb,
)

if trace_config_ctx._sentry_span is None:
return

span = trace_config_ctx.span
status = int(params.response.status)
span = trace_config_ctx._sentry_span

if isinstance(span, StreamedSpan):
span.set_attribute("http.response.status_code", status)
Expand Down
18 changes: 17 additions & 1 deletion sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,26 @@ def record_sql_queries(
yield span


def add_http_breadcrumb(status_code, data):
# type: (Optional[int], dict[str, Any]) -> None
Comment on lines +213 to +214

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.

Is there something preventing this type definition from living inline like the following?

Suggested change
def add_http_breadcrumb(status_code, data):
# type: (Optional[int], dict[str, Any]) -> None
def add_http_breadcrumb(status_code: "Optional[int]", data: "dict[str,Any]") -> "None":

Or is it in a comment as a matter of personal preference?

level = None
if status_code:
if 500 <= status_code <= 599:
level = "error"
elif 400 <= status_code <= 499:
level = "warning"

kwargs: "dict[str, Any]" = {"type": "http", "category": "httplib", "data": data}
if level:
kwargs["level"] = level

sentry_sdk.add_breadcrumb(**kwargs)


def maybe_create_breadcrumbs_from_span(
scope: "sentry_sdk.Scope", span: "sentry_sdk.tracing.Span"
) -> None:
if span.op == OP.HTTP_CLIENT:
if span.op == OP.HTTP_CLIENT and span.origin not in ("auto.http.aiohttp",):

@sentrivana sentrivana Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is just here to make sure we're not creating breadcrumbs the old way in transaction-based tracing anymore. Once all HTTP client integrations have been migrated, the whole function will go away

level = None
status_code = span._data.get(SPANDATA.HTTP_STATUS_CODE)
if status_code:
Expand Down
Loading
Loading