From 3367102dceeb3434b37c86058c50706264167f06 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 22 Oct 2024 14:58:30 +0200 Subject: [PATCH 1/9] Fixed breadcrumbs in aiohttp --- sentry_sdk/integrations/aiohttp.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 8edf921303..ddd610595d 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -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() @@ -258,6 +264,7 @@ 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 async def on_request_end(session, trace_config_ctx, params): # type: (ClientSession, SimpleNamespace, TraceRequestEndParams) -> None @@ -269,6 +276,16 @@ async def on_request_end(session, trace_config_ctx, params): span.set_data("reason", params.response.reason) span.finish() + span_data = trace_config_ctx.span_data + 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, + ) + trace_config = TraceConfig() trace_config.on_request_start.append(on_request_start) From ec5a34b7deabf33da17ab2b53b20efc19e8b51a4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 22 Oct 2024 15:05:24 +0200 Subject: [PATCH 2/9] Fixed breadcrumbs in httplib --- sentry_sdk/integrations/stdlib.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 8e038d6d3b..f1309f5d88 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -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) @@ -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 @@ -134,6 +141,16 @@ def getresponse(self, *args, **kwargs): span.set_data("reason", rv.reason) span.finish() + span_data = getattr(self, "_sentrysdk_span_data", None) + 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, + ) + return rv HTTPConnection.putrequest = putrequest # type: ignore[method-assign] From c785b760fc14deff02ae9d5938819ff350049657 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 22 Oct 2024 15:10:36 +0200 Subject: [PATCH 3/9] Fixed breadcrumbs in httpx --- sentry_sdk/integrations/httpx.py | 44 ++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index 6f80b93f4d..c4d8e4b4dc 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -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 ( @@ -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 @@ -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 ( @@ -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 From 6586431c3e49b5ad871d92cf09a03a00d917f486 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 22 Oct 2024 15:35:10 +0200 Subject: [PATCH 4/9] Fix breadcrumbs in boto3 --- sentry_sdk/integrations/boto3.py | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/boto3.py b/sentry_sdk/integrations/boto3.py index 86201d9959..195b532e54 100644 --- a/sentry_sdk/integrations/boto3.py +++ b/sentry_sdk/integrations/boto3.py @@ -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. @@ -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): @@ -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") @@ -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) From ef94296ec1e0f92bf3860c8a68985bceb7c4b2d7 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 22 Oct 2024 15:37:55 +0200 Subject: [PATCH 5/9] Make sure breadcrumb is always added before span is finished --- sentry_sdk/integrations/aiohttp.py | 12 ++++++------ sentry_sdk/integrations/stdlib.py | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index ddd610595d..36c1d807d2 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -271,12 +271,7 @@ async def on_request_end(session, trace_config_ctx, params): if trace_config_ctx.span is None: return - span = trace_config_ctx.span - span.set_http_status(int(params.response.status)) - span.set_data("reason", params.response.reason) - span.finish() - - span_data = trace_config_ctx.span_data + span_data = trace_config_ctx.span_data or {} span_data[SPANDATA.HTTP_STATUS_CODE] = int(params.response.status) span_data["reason"] = params.response.reason @@ -286,6 +281,11 @@ async def on_request_end(session, trace_config_ctx, params): data=span_data, ) + span = trace_config_ctx.span + span.set_http_status(int(params.response.status)) + span.set_data("reason", params.response.reason) + span.finish() + trace_config = TraceConfig() trace_config.on_request_start.append(on_request_start) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index f1309f5d88..8d5ed06386 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -137,11 +137,7 @@ def getresponse(self, *args, **kwargs): rv = real_getresponse(self, *args, **kwargs) - span.set_http_status(int(rv.status)) - span.set_data("reason", rv.reason) - span.finish() - - span_data = getattr(self, "_sentrysdk_span_data", None) + span_data = getattr(self, "_sentrysdk_span_data", {}) span_data[SPANDATA.HTTP_STATUS_CODE] = int(rv.status) span_data["reason"] = rv.reason @@ -151,6 +147,10 @@ def getresponse(self, *args, **kwargs): data=span_data, ) + span.set_http_status(int(rv.status)) + span.set_data("reason", rv.reason) + span.finish() + return rv HTTPConnection.putrequest = putrequest # type: ignore[method-assign] From 47ecc1f7000c5af13a43a63fb6e93047c979411f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 22 Oct 2024 16:38:21 +0200 Subject: [PATCH 6/9] Added test for breadcrumbs in boto3 --- tests/integrations/boto3/test_s3.py | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/integrations/boto3/test_s3.py b/tests/integrations/boto3/test_s3.py index 97a1543b0f..0954437c80 100644 --- a/tests/integrations/boto3/test_s3.py +++ b/tests/integrations/boto3/test_s3.py @@ -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() as 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() From 70117430390d123851f8afdaf78d1a95884599a6 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 22 Oct 2024 16:54:54 +0200 Subject: [PATCH 7/9] Remove breadcrump generation from tracing core code --- sentry_sdk/tracing_utils.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index f063897cb9..3a98914bb9 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -164,12 +164,6 @@ def maybe_create_breadcrumbs_from_span(scope, span): category="redis", data=span._tags, ) - elif span.op == OP.HTTP_CLIENT: - scope.add_breadcrumb( - type="http", - category="httplib", - data=span._data, - ) def _get_frame_module_abs_path(frame): From 0fee982ae3703c4f025b962e114253aea964c919 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 22 Oct 2024 16:59:13 +0200 Subject: [PATCH 8/9] Cleanup --- sentry_sdk/tracing_utils.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 3a98914bb9..b9d4fb9335 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -157,13 +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.DB_REDIS: - scope.add_breadcrumb( - message=span.description, - type="redis", - category="redis", - data=span._tags, - ) + # TODO: can be removed when POtelSpan replaces Span + pass def _get_frame_module_abs_path(frame): From 553acf9ec18bed06829e32db61477aa4498a69ca Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 22 Oct 2024 17:04:06 +0200 Subject: [PATCH 9/9] linting --- sentry_sdk/integrations/redis/_async_common.py | 1 - sentry_sdk/integrations/redis/_sync_common.py | 1 - sentry_sdk/integrations/redis/modules/caches.py | 1 - sentry_sdk/integrations/redis/modules/queries.py | 1 - sentry_sdk/integrations/redis/redis_cluster.py | 1 - tests/integrations/boto3/test_s3.py | 2 +- tests/integrations/redis/test_redis.py | 4 ++-- 7 files changed, 3 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/integrations/redis/_async_common.py b/sentry_sdk/integrations/redis/_async_common.py index 6a136fe29a..f835affbf3 100644 --- a/sentry_sdk/integrations/redis/_async_common.py +++ b/sentry_sdk/integrations/redis/_async_common.py @@ -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 diff --git a/sentry_sdk/integrations/redis/_sync_common.py b/sentry_sdk/integrations/redis/_sync_common.py index f4cb6ee1b8..9c96ad61d1 100644 --- a/sentry_sdk/integrations/redis/_sync_common.py +++ b/sentry_sdk/integrations/redis/_sync_common.py @@ -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 diff --git a/sentry_sdk/integrations/redis/modules/caches.py b/sentry_sdk/integrations/redis/modules/caches.py index d93e729f2b..4ab33d2ea8 100644 --- a/sentry_sdk/integrations/redis/modules/caches.py +++ b/sentry_sdk/integrations/redis/modules/caches.py @@ -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 diff --git a/sentry_sdk/integrations/redis/modules/queries.py b/sentry_sdk/integrations/redis/modules/queries.py index e2189b7f9c..c070893ac8 100644 --- a/sentry_sdk/integrations/redis/modules/queries.py +++ b/sentry_sdk/integrations/redis/modules/queries.py @@ -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 diff --git a/sentry_sdk/integrations/redis/redis_cluster.py b/sentry_sdk/integrations/redis/redis_cluster.py index dbcd20a65d..7975e21083 100644 --- a/sentry_sdk/integrations/redis/redis_cluster.py +++ b/sentry_sdk/integrations/redis/redis_cluster.py @@ -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): diff --git a/tests/integrations/boto3/test_s3.py b/tests/integrations/boto3/test_s3.py index 0954437c80..668e8349b6 100644 --- a/tests/integrations/boto3/test_s3.py +++ b/tests/integrations/boto3/test_s3.py @@ -45,7 +45,7 @@ def test_breadcrumb(sentry_init, capture_events): try: s3 = session.resource("s3") - with sentry_sdk.start_transaction() as transaction, MockResponse( + with sentry_sdk.start_transaction(), MockResponse( s3.meta.client, 200, {}, read_fixture("s3_list.xml") ): bucket = s3.Bucket("bucket") diff --git a/tests/integrations/redis/test_redis.py b/tests/integrations/redis/test_redis.py index f8225bed79..0fec23f273 100644 --- a/tests/integrations/redis/test_redis.py +++ b/tests/integrations/redis/test_redis.py @@ -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 ) @@ -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 )