Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix breadcrumbs in HTTP clients #3683

Merged
25 changes: 21 additions & 4 deletions sentry_sdk/integrations/aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,17 @@ async def on_request_start(session, trace_config_ctx, params):
% (method, parsed_url.url if parsed_url else SENSITIVE_DATA_SUBSTITUTE),
origin=AioHttpIntegration.origin,
)
span.set_data(SPANDATA.HTTP_METHOD, method)

data = {
SPANDATA.HTTP_METHOD: method,
}
if parsed_url is not None:
span.set_data("url", parsed_url.url)
span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query)
span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment)
data["url"] = parsed_url.url
data[SPANDATA.HTTP_QUERY] = parsed_url.query
data[SPANDATA.HTTP_FRAGMENT] = parsed_url.fragment

for key, value in data.items():
span.set_data(key, value)

client = sentry_sdk.get_client()

Expand All @@ -258,12 +264,23 @@ async def on_request_start(session, trace_config_ctx, params):
params.headers[key] = value

trace_config_ctx.span = span
trace_config_ctx.span_data = data
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: maybe let's call it something like request_data instead of span_data since it's also used for breadcrumbs? (Also in the other integrations)


async def on_request_end(session, trace_config_ctx, params):
# type: (ClientSession, SimpleNamespace, TraceRequestEndParams) -> None
if trace_config_ctx.span is None:
return

span_data = trace_config_ctx.span_data or {}
span_data[SPANDATA.HTTP_STATUS_CODE] = int(params.response.status)
span_data["reason"] = params.response.reason

sentry_sdk.add_breadcrumb(
type="http",
category="httplib",
data=span_data,
)

span = trace_config_ctx.span
span.set_http_status(int(params.response.status))
span.set_data("reason", params.response.reason)
Expand Down
32 changes: 28 additions & 4 deletions sentry_sdk/integrations/boto3.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,20 @@ def _sentry_request_created(service_id, request, operation_name, **kwargs):
origin=Boto3Integration.origin,
)

data = {
SPANDATA.HTTP_METHOD: request.method,
}
with capture_internal_exceptions():
parsed_url = parse_url(request.url, sanitize=False)
span.set_data("aws.request.url", parsed_url.url)
span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query)
span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment)
data["aws.request.url"] = parsed_url.url
data[SPANDATA.HTTP_QUERY] = parsed_url.query
data[SPANDATA.HTTP_FRAGMENT] = parsed_url.fragment

for key, value in data.items():
span.set_data(key, value)

span.set_tag("aws.service_id", service_id)
span.set_tag("aws.operation_name", operation_name)
span.set_data(SPANDATA.HTTP_METHOD, request.method)

# We do it in order for subsequent http calls/retries be
# attached to this span.
Expand All @@ -91,6 +96,7 @@ def _sentry_request_created(service_id, request, operation_name, **kwargs):
# request.context is an open-ended data-structure
# where we can add anything useful in request life cycle.
request.context["_sentrysdk_span"] = span
request.context["_sentrysdk_span_data"] = data


def _sentry_after_call(context, parsed, **kwargs):
Expand All @@ -100,6 +106,15 @@ def _sentry_after_call(context, parsed, **kwargs):
# Span could be absent if the integration is disabled.
if span is None:
return

span_data = context.pop("_sentrysdk_span_data", {})

sentry_sdk.add_breadcrumb(
type="http",
category="httplib",
data=span_data,
)

span.__exit__(None, None, None)

body = parsed.get("Body")
Expand Down Expand Up @@ -143,4 +158,13 @@ def _sentry_after_call_error(context, exception, **kwargs):
# Span could be absent if the integration is disabled.
if span is None:
return

span_data = context.pop("_sentrysdk_span_data", {})

sentry_sdk.add_breadcrumb(
type="http",
category="httplib",
data=span_data,
)

span.__exit__(type(exception), exception, None)
44 changes: 36 additions & 8 deletions sentry_sdk/integrations/httpx.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,16 @@ def send(self, request, **kwargs):
),
origin=HttpxIntegration.origin,
) as span:
span.set_data(SPANDATA.HTTP_METHOD, request.method)
data = {
SPANDATA.HTTP_METHOD: request.method,
}
if parsed_url is not None:
span.set_data("url", parsed_url.url)
span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query)
span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment)
data["url"] = parsed_url.url
data[SPANDATA.HTTP_QUERY] = parsed_url.query
data[SPANDATA.HTTP_FRAGMENT] = parsed_url.fragment

for key, value in data.items():
span.set_data(key, value)

if should_propagate_trace(sentry_sdk.get_client(), str(request.url)):
for (
Expand All @@ -89,6 +94,15 @@ def send(self, request, **kwargs):
span.set_http_status(rv.status_code)
span.set_data("reason", rv.reason_phrase)

data[SPANDATA.HTTP_STATUS_CODE] = rv.status_code
data["reason"] = rv.reason_phrase

sentry_sdk.add_breadcrumb(
type="http",
category="httplib",
data=data,
)

return rv

Client.send = send
Expand Down Expand Up @@ -116,11 +130,16 @@ async def send(self, request, **kwargs):
),
origin=HttpxIntegration.origin,
) as span:
span.set_data(SPANDATA.HTTP_METHOD, request.method)
data = {
SPANDATA.HTTP_METHOD: request.method,
}
if parsed_url is not None:
span.set_data("url", parsed_url.url)
span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query)
span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment)
data["url"] = parsed_url.url
data[SPANDATA.HTTP_QUERY] = parsed_url.query
data[SPANDATA.HTTP_FRAGMENT] = parsed_url.fragment

for key, value in data.items():
span.set_data(key, value)

if should_propagate_trace(sentry_sdk.get_client(), str(request.url)):
for (
Expand All @@ -145,6 +164,15 @@ async def send(self, request, **kwargs):
span.set_http_status(rv.status_code)
span.set_data("reason", rv.reason_phrase)

data[SPANDATA.HTTP_STATUS_CODE] = rv.status_code
data["reason"] = rv.reason_phrase

sentry_sdk.add_breadcrumb(
type="http",
category="httplib",
data=data,
)

return rv

AsyncClient.send = send
1 change: 0 additions & 1 deletion sentry_sdk/integrations/redis/_async_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
_get_pipeline_data,
_update_span,
)
from sentry_sdk.tracing import Span
from sentry_sdk.utils import capture_internal_exceptions

from typing import TYPE_CHECKING
Expand Down
1 change: 0 additions & 1 deletion sentry_sdk/integrations/redis/_sync_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
_get_pipeline_data,
_update_span,
)
from sentry_sdk.tracing import Span
from sentry_sdk.utils import capture_internal_exceptions

from typing import TYPE_CHECKING
Expand Down
1 change: 0 additions & 1 deletion sentry_sdk/integrations/redis/modules/caches.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

if TYPE_CHECKING:
from sentry_sdk.integrations.redis import RedisIntegration
from sentry_sdk.tracing import Span
from typing import Any, Optional


Expand Down
1 change: 0 additions & 1 deletion sentry_sdk/integrations/redis/modules/queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
if TYPE_CHECKING:
from redis import Redis
from sentry_sdk.integrations.redis import RedisIntegration
from sentry_sdk.tracing import Span
from typing import Any


Expand Down
1 change: 0 additions & 1 deletion sentry_sdk/integrations/redis/redis_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
RedisCluster as AsyncRedisCluster,
ClusterPipeline as AsyncClusterPipeline,
)
from sentry_sdk.tracing import Span


def _get_async_cluster_db_data(async_redis_cluster_instance):
Expand Down
25 changes: 21 additions & 4 deletions sentry_sdk/integrations/stdlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,17 @@ def putrequest(self, method, url, *args, **kwargs):
% (method, parsed_url.url if parsed_url else SENSITIVE_DATA_SUBSTITUTE),
origin="auto.http.stdlib.httplib",
)
span.set_data(SPANDATA.HTTP_METHOD, method)

data = {
SPANDATA.HTTP_METHOD: method,
}
if parsed_url is not None:
span.set_data("url", parsed_url.url)
span.set_data(SPANDATA.HTTP_QUERY, parsed_url.query)
span.set_data(SPANDATA.HTTP_FRAGMENT, parsed_url.fragment)
data["url"] = parsed_url.url
data[SPANDATA.HTTP_QUERY] = parsed_url.query
data[SPANDATA.HTTP_FRAGMENT] = parsed_url.fragment

for key, value in data.items():
span.set_data(key, value)

rv = real_putrequest(self, method, url, *args, **kwargs)

Expand All @@ -118,6 +124,7 @@ def putrequest(self, method, url, *args, **kwargs):
self.putheader(key, value)

self._sentrysdk_span = span # type: ignore[attr-defined]
self._sentrysdk_span_data = data # type: ignore[attr-defined]

return rv

Expand All @@ -130,6 +137,16 @@ def getresponse(self, *args, **kwargs):

rv = real_getresponse(self, *args, **kwargs)

span_data = getattr(self, "_sentrysdk_span_data", {})
span_data[SPANDATA.HTTP_STATUS_CODE] = int(rv.status)
span_data["reason"] = rv.reason

sentry_sdk.add_breadcrumb(
type="http",
category="httplib",
data=span_data,
)

span.set_http_status(int(rv.status))
span.set_data("reason", rv.reason)
span.finish()
Expand Down
8 changes: 2 additions & 6 deletions sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,8 @@ def record_sql_queries(

def maybe_create_breadcrumbs_from_span(scope, span):
# type: (sentry_sdk.Scope, sentry_sdk.tracing.Span) -> None
if span.op == OP.HTTP_CLIENT:
scope.add_breadcrumb(
type="http",
category="httplib",
data=span._data,
)
# TODO: can be removed when POtelSpan replaces Span
pass


def _get_frame_module_abs_path(frame):
Expand Down
31 changes: 31 additions & 0 deletions tests/integrations/boto3/test_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,37 @@ def test_basic(sentry_init, capture_events):
assert span["description"] == "aws.s3.ListObjects"


def test_breadcrumb(sentry_init, capture_events):
sentry_init(traces_sample_rate=1.0, integrations=[Boto3Integration()])
events = capture_events()

try:
s3 = session.resource("s3")
with sentry_sdk.start_transaction(), MockResponse(
s3.meta.client, 200, {}, read_fixture("s3_list.xml")
):
bucket = s3.Bucket("bucket")
# read bucket (this makes http request)
[obj for obj in bucket.objects.all()]
1 / 0
except Exception as e:
sentry_sdk.capture_exception(e)

(_, event) = events
crumb = event["breadcrumbs"]["values"][0]
assert crumb == {
"type": "http",
"category": "httplib",
"data": {
"http.method": "GET",
"aws.request.url": "https://bucket.s3.amazonaws.com/",
"http.query": "encoding-type=url",
"http.fragment": "",
},
"timestamp": mock.ANY,
}


def test_streaming(sentry_init, capture_events):
sentry_init(traces_sample_rate=1.0, integrations=[Boto3Integration()])
events = capture_events()
Expand Down
4 changes: 2 additions & 2 deletions tests/integrations/redis/test_redis.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def test_data_truncation(sentry_init, capture_events, render_span_tree):
- op="": description=null
- op="db.redis": description="SET 'somekey1' '{long_string[: 1024 - len("...") - len("SET 'somekey1' '")]}..."
- op="db.redis": description="SET 'somekey2' 'bbbbbbbbbb'"\
"""
""" # noqa: E221
)


Expand All @@ -212,7 +212,7 @@ def test_data_truncation_custom(sentry_init, capture_events, render_span_tree):
- op="": description=null
- op="db.redis": description="SET 'somekey1' '{long_string[: 30 - len("...") - len("SET 'somekey1' '")]}..."
- op="db.redis": description="SET 'somekey2' '{short_string}'"\
"""
""" # noqa: E221
)


Expand Down
Loading