Move subprocess breadcrumbs from maybe_create_breadcrumbs_from_span to integration.

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

sentry_sdk/tracing_utils.py

@@ -157,18 +157,17 @@ 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="subprocess",
-            category="subprocess",
-            message=span.description,
+            type="http",
+            category="httplib",
             data=span._data,
         )
 

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,
+    }

tests/test_breadcrumbs.py

@@ -0,0 +1,140 @@
+from unittest import mock
+
+import sentry_sdk
+from sentry_sdk.consts import OP
+
+
+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,
+    )
+    events = capture_events()
+
+    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,) = events
+
+    breadcrumbs = error["breadcrumbs"]["values"]
+
+    for crumb in breadcrumbs:
+        print(crumb)
+
+    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"] == "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"] == "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"] == "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