From b9624a1d250e29a4f1164e4d1d68bea5813cbfe0 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 9 Oct 2024 13:13:29 +0200 Subject: [PATCH 1/7] Added test for breadcrumb behavior --- sentry_sdk/integrations/asyncpg.py | 4 +- sentry_sdk/integrations/clickhouse_driver.py | 4 +- sentry_sdk/tracing_utils.py | 14 +- tests/test_breadcrumbs.py | 134 +++++++++++++++++++ 4 files changed, 150 insertions(+), 6 deletions(-) create mode 100644 tests/test_breadcrumbs.py diff --git a/sentry_sdk/integrations/asyncpg.py b/sentry_sdk/integrations/asyncpg.py index b05d5615ba..21cb754c14 100644 --- a/sentry_sdk/integrations/asyncpg.py +++ b/sentry_sdk/integrations/asyncpg.py @@ -181,7 +181,9 @@ async def _inner(*args: Any, **kwargs: Any) -> T: with capture_internal_exceptions(): sentry_sdk.add_breadcrumb( - message="connect", category="query", data=span._data + message="connect", + category="query", + data=span._data, ) res = await f(*args, **kwargs) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index daf4c2257c..c9b245d144 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -119,7 +119,9 @@ def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T: with capture_internal_exceptions(): span.scope.add_breadcrumb( - message=span._data.pop("query"), category="query", data=span._data + message=span._data.pop("query"), + category="query", + data=span._data, ) span.finish() diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 2f4dad738a..ff6a9fd657 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -156,14 +156,20 @@ 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 + message=span.description, + type="redis", + category="redis", + data=span._tags, ) elif span.op == OP.HTTP_CLIENT: - scope.add_breadcrumb(type="http", category="httplib", data=span._data) - elif span.op == "subprocess": + scope.add_breadcrumb( + type="http", + category="httplib", + data=span._data, + ) + elif span.op == OP.SUBPROCESS: scope.add_breadcrumb( type="subprocess", category="subprocess", diff --git a/tests/test_breadcrumbs.py b/tests/test_breadcrumbs.py new file mode 100644 index 0000000000..c60cf1f1f2 --- /dev/null +++ b/tests/test_breadcrumbs.py @@ -0,0 +1,134 @@ +from unittest import mock + +import sentry_sdk +from sentry_sdk.consts import OP + + +def test_breadcrumbs(sentry_init, capture_envelopes): + """ + This test illustrates how breadcrumbs are added to the error event when an error occurs + """ + sentry_init( + traces_sample_rate=1.0, + ) + envelopes = capture_envelopes() + + add_breadcrumbs_kwargs = { + "type": "navigation", + "category": "unit_tests.breadcrumbs", + "level": "fatal", + "origin": "unit-tests", + "data": { + "string": "foobar", + "number": 4.2, + "array": [1, 2, 3], + "dict": {"foo": "bar"}, + }, + } + + with sentry_sdk.start_transaction(name="trx-breadcrumbs"): + sentry_sdk.add_breadcrumb(message="breadcrumb0", **add_breadcrumbs_kwargs) + + with sentry_sdk.start_span(name="span1", op="function"): + sentry_sdk.add_breadcrumb(message="breadcrumb1", **add_breadcrumbs_kwargs) + + with sentry_sdk.start_span(name="span2", op="function"): + sentry_sdk.add_breadcrumb(message="breadcrumb2", **add_breadcrumbs_kwargs) + + # Spans that create breadcrumbs automatically + with sentry_sdk.start_span(name="span3", op=OP.DB_REDIS) as span3: + span3.set_data("span3_data", "data on the redis span") + span3.set_tag("span3_tag", "tag on the redis span") + + with sentry_sdk.start_span(name="span4", op=OP.HTTP_CLIENT) as span4: + span4.set_data("span4_data", "data on the http.client span") + span4.set_tag("span4_tag", "tag on the http.client span") + + with sentry_sdk.start_span(name="span5", op=OP.SUBPROCESS) as span5: + span5.set_data("span5_data", "data on the subprocess span") + span5.set_tag("span5_tag", "tag on the subprocess span") + + with sentry_sdk.start_span(name="span6", op="function") as span6: + # This data on the span is not added to custom breadcrumbs. + # Data from the span is only added to automatic breadcrumbs shown above + span6.set_data("span6_data", "data on span6") + span6.set_tag("span6_tag", "tag on the span6") + sentry_sdk.add_breadcrumb(message="breadcrumb6", **add_breadcrumbs_kwargs) + + try: + 1/0 + except ZeroDivisionError as ex: + sentry_sdk.capture_exception(ex) + + (error_envelope, transaction_envelope) = envelopes + error = error_envelope.get_event() + transaction = transaction_envelope.get_transaction_event() + + breadcrumbs = error["breadcrumbs"]["values"] + assert len(breadcrumbs) == 7 + + # Check for my custom breadcrumbs + for i in range(0, 3): + assert breadcrumbs[i]["message"] == f"breadcrumb{i}" + assert breadcrumbs[i]["type"] == "navigation" + assert breadcrumbs[i]["category"] == "unit_tests.breadcrumbs" + assert breadcrumbs[i]["level"] == "fatal" + assert breadcrumbs[i]["origin"] == "unit-tests" + assert breadcrumbs[i]["data"] == { + "string": "foobar", + "number": 4.2, + "array": [1, 2, 3], + "dict": {"foo": "bar"}, + } + assert breadcrumbs[i]["timestamp"] == mock.ANY + + # Check automatic redis breadcrumbs + assert breadcrumbs[3]["message"] == f"span3" + assert breadcrumbs[3]["type"] == "redis" + assert breadcrumbs[3]["category"] == "redis" + assert "level" not in breadcrumbs[3] + assert "origin" not in breadcrumbs[3] + assert breadcrumbs[3]["data"] == { + "span3_tag": "tag on the redis span", + } + assert breadcrumbs[3]["timestamp"] == mock.ANY + + # Check automatic http.client breadcrumbs + assert "message" not in breadcrumbs[4] + assert breadcrumbs[4]["type"] == "http" + assert breadcrumbs[4]["category"] == "httplib" + assert "level" not in breadcrumbs[4] + assert "origin" not in breadcrumbs[4] + assert breadcrumbs[4]["data"] == { + "thread.id": mock.ANY, + "thread.name": mock.ANY, + "span4_data": "data on the http.client span", + } + assert breadcrumbs[4]["timestamp"] == mock.ANY + + # Check automatic subprocess breadcrumbs + assert breadcrumbs[5]["message"] == f"span5" + assert breadcrumbs[5]["type"] == "subprocess" + assert breadcrumbs[5]["category"] == "subprocess" + assert "level" not in breadcrumbs[5] + assert "origin" not in breadcrumbs[5] + assert breadcrumbs[5]["data"] == { + "thread.id": mock.ANY, + "thread.name": mock.ANY, + "span5_data": "data on the subprocess span", + } + assert breadcrumbs[5]["timestamp"] == mock.ANY + + # Check for custom breadcrumbs on span6 + assert breadcrumbs[6]["message"] == f"breadcrumb6" + assert breadcrumbs[6]["type"] == "navigation" + assert breadcrumbs[6]["category"] == "unit_tests.breadcrumbs" + assert breadcrumbs[6]["level"] == "fatal" + assert breadcrumbs[6]["origin"] == "unit-tests" + assert breadcrumbs[6]["data"] == { + "string": "foobar", + "number": 4.2, + "array": [1, 2, 3], + "dict": {"foo": "bar"}, + } + assert breadcrumbs[6]["timestamp"] == mock.ANY From d2567c93be22ef9307af5ee62bb0bcadea241272 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 9 Oct 2024 13:18:09 +0200 Subject: [PATCH 2/7] debug output --- tests/test_breadcrumbs.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_breadcrumbs.py b/tests/test_breadcrumbs.py index c60cf1f1f2..f5db2cde2c 100644 --- a/tests/test_breadcrumbs.py +++ b/tests/test_breadcrumbs.py @@ -65,6 +65,10 @@ def test_breadcrumbs(sentry_init, capture_envelopes): transaction = transaction_envelope.get_transaction_event() breadcrumbs = error["breadcrumbs"]["values"] + + for crumb in breadcrumbs: + print(crumb) + assert len(breadcrumbs) == 7 # Check for my custom breadcrumbs From 72c1ad87f4db3a9ff003e04e1cb40bed9d7be4f7 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 9 Oct 2024 14:38:18 +0200 Subject: [PATCH 3/7] Fixed breadcrumbs in asyncpg --- sentry_sdk/integrations/asyncpg.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/integrations/asyncpg.py b/sentry_sdk/integrations/asyncpg.py index 21cb754c14..c9c7b1cb60 100644 --- a/sentry_sdk/integrations/asyncpg.py +++ b/sentry_sdk/integrations/asyncpg.py @@ -168,23 +168,29 @@ async def _inner(*args: Any, **kwargs: Any) -> T: name="connect", origin=AsyncPGIntegration.origin, ) as span: - span.set_data(SPANDATA.DB_SYSTEM, "postgresql") + span_data = { + SPANDATA.DB_SYSTEM: "postgresql", + SPANDATA.DB_NAME: database, + SPANDATA.DB_USER: user, + } addr = kwargs.get("addr") if addr: try: - span.set_data(SPANDATA.SERVER_ADDRESS, addr[0]) - span.set_data(SPANDATA.SERVER_PORT, addr[1]) + span_data[SPANDATA.SERVER_ADDRESS] = addr[0] + span_data[SPANDATA.SERVER_PORT] = addr[1] except IndexError: pass - span.set_data(SPANDATA.DB_NAME, database) - span.set_data(SPANDATA.DB_USER, user) + + for k, v in span_data.items(): + span.set_data(k, v) with capture_internal_exceptions(): sentry_sdk.add_breadcrumb( - message="connect", - category="query", - data=span._data, + message="connect", + category="query", + data=span_data, ) + res = await f(*args, **kwargs) return res From 76b5373ace6a3127267e003788b29b64b5bafc17 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 9 Oct 2024 16:03:54 +0200 Subject: [PATCH 4/7] Fix breadcrumb in subprocess --- sentry_sdk/integrations/clickhouse_driver.py | 4 +-- sentry_sdk/integrations/stdlib.py | 19 ++++++++++ sentry_sdk/tracing.py | 1 + sentry_sdk/tracing_utils.py | 17 +++------ tests/integrations/stdlib/test_subprocess.py | 37 +++++++++++++++++++- tests/test_breadcrumbs.py | 14 +++++--- 6 files changed, 72 insertions(+), 20 deletions(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index c9b245d144..e563cbdfc4 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -119,8 +119,8 @@ def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T: with capture_internal_exceptions(): span.scope.add_breadcrumb( - message=span._data.pop("query"), - category="query", + message=span._data.pop("query"), + category="query", data=span._data, ) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 287c8cb272..8e038d6d3b 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -13,6 +13,7 @@ SENSITIVE_DATA_SUBSTITUTE, capture_internal_exceptions, ensure_integration_enabled, + get_current_thread_meta, is_sentry_url, logger, safe_repr, @@ -225,6 +226,24 @@ def sentry_patched_popen_init(self, *a, **kw): rv = old_popen_init(self, *a, **kw) span.set_tag("subprocess.pid", self.pid) + + with capture_internal_exceptions(): + thread_id, thread_name = get_current_thread_meta() + breadcrumb_data = { + "subprocess.pid": self.pid, + "thread.id": thread_id, + "thread.name": thread_name, + } + if cwd: + breadcrumb_data["subprocess.cwd"] = cwd + + sentry_sdk.add_breadcrumb( + type="subprocess", + category="subprocess", + message=description, + data=breadcrumb_data, + ) + return rv subprocess.Popen.__init__ = sentry_patched_popen_init # type: ignore diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 1ca79f6c14..2e560ff3d7 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1586,6 +1586,7 @@ def get_profile_context(self): def set_context(self, key, value): # type: (str, Any) -> None from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute + # TODO-neel-potel we cannot add dicts here self.set_attribute(f"{SentrySpanAttribute.CONTEXT}.{key}", value) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index ff6a9fd657..40588886bf 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -158,22 +158,15 @@ 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", + message=span.description, + type="redis", + category="redis", data=span._tags, ) elif span.op == OP.HTTP_CLIENT: scope.add_breadcrumb( - type="http", - category="httplib", - data=span._data, - ) - elif span.op == OP.SUBPROCESS: - scope.add_breadcrumb( - type="subprocess", - category="subprocess", - message=span.description, + type="http", + category="httplib", data=span._data, ) diff --git a/tests/integrations/stdlib/test_subprocess.py b/tests/integrations/stdlib/test_subprocess.py index 593ef8a0dc..62d5a2aeba 100644 --- a/tests/integrations/stdlib/test_subprocess.py +++ b/tests/integrations/stdlib/test_subprocess.py @@ -3,10 +3,11 @@ import subprocess import sys from collections.abc import Mapping +from unittest import mock import pytest -from sentry_sdk import capture_message, start_transaction +from sentry_sdk import capture_exception, capture_message, start_transaction from sentry_sdk.integrations.stdlib import StdlibIntegration from tests.conftest import ApproxDict @@ -224,3 +225,37 @@ def test_subprocess_span_origin(sentry_init, capture_events): assert event["spans"][2]["op"] == "subprocess.wait" assert event["spans"][2]["origin"] == "auto.subprocess.stdlib.subprocess" + + +def test_subprocess_breadcrumb(sentry_init, capture_events): + sentry_init() + events = capture_events() + + args = [ + sys.executable, + "-c", + "print('hello world')", + ] + popen = subprocess.Popen(args) + popen.communicate() + popen.poll() + + try: + 1 / 0 + except ZeroDivisionError as ex: + capture_exception(ex) + + (event,) = events + breadcrumbs = event["breadcrumbs"]["values"] + assert len(breadcrumbs) == 1 + + (crumb,) = breadcrumbs + assert crumb["type"] == "subprocess" + assert crumb["category"] == "subprocess" + assert crumb["message"] == " ".join(args) + assert crumb["timestamp"] == mock.ANY + assert crumb["data"] == { + "subprocess.pid": popen.pid, + "thread.id": mock.ANY, + "thread.name": mock.ANY, + } diff --git a/tests/test_breadcrumbs.py b/tests/test_breadcrumbs.py index f5db2cde2c..f26506c359 100644 --- a/tests/test_breadcrumbs.py +++ b/tests/test_breadcrumbs.py @@ -33,7 +33,9 @@ def test_breadcrumbs(sentry_init, capture_envelopes): sentry_sdk.add_breadcrumb(message="breadcrumb1", **add_breadcrumbs_kwargs) with sentry_sdk.start_span(name="span2", op="function"): - sentry_sdk.add_breadcrumb(message="breadcrumb2", **add_breadcrumbs_kwargs) + sentry_sdk.add_breadcrumb( + message="breadcrumb2", **add_breadcrumbs_kwargs + ) # Spans that create breadcrumbs automatically with sentry_sdk.start_span(name="span3", op=OP.DB_REDIS) as span3: @@ -49,14 +51,16 @@ def test_breadcrumbs(sentry_init, capture_envelopes): span5.set_tag("span5_tag", "tag on the subprocess span") with sentry_sdk.start_span(name="span6", op="function") as span6: - # This data on the span is not added to custom breadcrumbs. + # This data on the span is not added to custom breadcrumbs. # Data from the span is only added to automatic breadcrumbs shown above span6.set_data("span6_data", "data on span6") span6.set_tag("span6_tag", "tag on the span6") - sentry_sdk.add_breadcrumb(message="breadcrumb6", **add_breadcrumbs_kwargs) + sentry_sdk.add_breadcrumb( + message="breadcrumb6", **add_breadcrumbs_kwargs + ) try: - 1/0 + 1 / 0 except ZeroDivisionError as ex: sentry_sdk.capture_exception(ex) @@ -96,7 +100,7 @@ def test_breadcrumbs(sentry_init, capture_envelopes): "span3_tag": "tag on the redis span", } assert breadcrumbs[3]["timestamp"] == mock.ANY - + # Check automatic http.client breadcrumbs assert "message" not in breadcrumbs[4] assert breadcrumbs[4]["type"] == "http" From 1a7f3fcb0334647efb2045eb25c753551de2e916 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 9 Oct 2024 16:31:56 +0200 Subject: [PATCH 5/7] Cleanup --- tests/test_breadcrumbs.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_breadcrumbs.py b/tests/test_breadcrumbs.py index f26506c359..22d9a576f8 100644 --- a/tests/test_breadcrumbs.py +++ b/tests/test_breadcrumbs.py @@ -4,14 +4,14 @@ from sentry_sdk.consts import OP -def test_breadcrumbs(sentry_init, capture_envelopes): +def test_breadcrumbs(sentry_init, capture_events): """ This test illustrates how breadcrumbs are added to the error event when an error occurs """ sentry_init( traces_sample_rate=1.0, ) - envelopes = capture_envelopes() + events = capture_events() add_breadcrumbs_kwargs = { "type": "navigation", @@ -64,9 +64,7 @@ def test_breadcrumbs(sentry_init, capture_envelopes): except ZeroDivisionError as ex: sentry_sdk.capture_exception(ex) - (error_envelope, transaction_envelope) = envelopes - error = error_envelope.get_event() - transaction = transaction_envelope.get_transaction_event() + (error, ) = events breadcrumbs = error["breadcrumbs"]["values"] From 9deb563583795276c35f3c5415ac821e5f38d141 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 9 Oct 2024 16:35:31 +0200 Subject: [PATCH 6/7] Cleanup --- tests/test_breadcrumbs.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_breadcrumbs.py b/tests/test_breadcrumbs.py index 22d9a576f8..8d37370cdf 100644 --- a/tests/test_breadcrumbs.py +++ b/tests/test_breadcrumbs.py @@ -89,7 +89,7 @@ def test_breadcrumbs(sentry_init, capture_events): assert breadcrumbs[i]["timestamp"] == mock.ANY # Check automatic redis breadcrumbs - assert breadcrumbs[3]["message"] == f"span3" + assert breadcrumbs[3]["message"] == "span3" assert breadcrumbs[3]["type"] == "redis" assert breadcrumbs[3]["category"] == "redis" assert "level" not in breadcrumbs[3] @@ -113,7 +113,7 @@ def test_breadcrumbs(sentry_init, capture_events): assert breadcrumbs[4]["timestamp"] == mock.ANY # Check automatic subprocess breadcrumbs - assert breadcrumbs[5]["message"] == f"span5" + assert breadcrumbs[5]["message"] == "span5" assert breadcrumbs[5]["type"] == "subprocess" assert breadcrumbs[5]["category"] == "subprocess" assert "level" not in breadcrumbs[5] @@ -126,7 +126,7 @@ def test_breadcrumbs(sentry_init, capture_events): assert breadcrumbs[5]["timestamp"] == mock.ANY # Check for custom breadcrumbs on span6 - assert breadcrumbs[6]["message"] == f"breadcrumb6" + assert breadcrumbs[6]["message"] == "breadcrumb6" assert breadcrumbs[6]["type"] == "navigation" assert breadcrumbs[6]["category"] == "unit_tests.breadcrumbs" assert breadcrumbs[6]["level"] == "fatal" From df297bec35a0daa6ea47afd8b3f502ca8114bfa9 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 9 Oct 2024 16:38:22 +0200 Subject: [PATCH 7/7] format --- tests/test_breadcrumbs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_breadcrumbs.py b/tests/test_breadcrumbs.py index 8d37370cdf..988f536fde 100644 --- a/tests/test_breadcrumbs.py +++ b/tests/test_breadcrumbs.py @@ -64,7 +64,7 @@ def test_breadcrumbs(sentry_init, capture_events): except ZeroDivisionError as ex: sentry_sdk.capture_exception(ex) - (error, ) = events + (error,) = events breadcrumbs = error["breadcrumbs"]["values"]