set the span id as parent id when errors happen in spans (#669)

* set the span id as parent id when errors happen in spans

* update changelog

Author: beniwohli

CHANGELOG.asciidoc

@@ -29,6 +29,7 @@ https://github.com/elastic/apm-agent-python/compare/v5.3.1\...master[Check the d
 ===== New Features
 
  * Added support for W3C `traceparent` and `tracestate` headers {pull}660[#660]
+ * use Span ID as parent ID in errors if an error happens inside a span {pull}669[#669]
 
 
 [[release-notes-5.x]]

elasticapm/base.py

@@ -361,6 +361,7 @@ def _build_msg_for_logging(
         Captures, processes and serializes an event into a dict object
         """
         transaction = execution_context.get_transaction()
+        span = execution_context.get_span()
         if transaction:
             transaction_context = deepcopy(transaction.context)
         else:
@@ -453,7 +454,8 @@ def _build_msg_for_logging(
         if transaction:
             if transaction.trace_parent:
                 event_data["trace_id"] = transaction.trace_parent.trace_id
-            event_data["parent_id"] = transaction.id
+            # parent id might already be set in the handler
+            event_data.setdefault("parent_id", span.id if span else transaction.id)
             event_data["transaction_id"] = transaction.id
             event_data["transaction"] = {"sampled": transaction.is_sampled, "type": transaction.transaction_type}
 

elasticapm/contrib/asyncio/traces.py

@@ -30,7 +30,7 @@
 
 import functools
 
-from elasticapm.traces import capture_span, error_logger, execution_context
+from elasticapm.traces import DroppedSpan, capture_span, error_logger, execution_context
 from elasticapm.utils import get_name_from_func
 
 
@@ -64,6 +64,12 @@ async def __aexit__(self, exc_type, exc_val, exc_tb):
         transaction = execution_context.get_transaction()
         if transaction and transaction.is_sampled:
             try:
-                transaction.end_span(self.skip_frames)
+                span = transaction.end_span(self.skip_frames)
+                if exc_val and not isinstance(span, DroppedSpan):
+                    try:
+                        exc_val._elastic_apm_span_id = span.id
+                    except AttributeError:
+                        # could happen if the exception has __slots__
+                        pass
             except LookupError:
                 error_logger.info("ended non-existing span %s of type %s", self.name, self.type)

elasticapm/events.py

@@ -143,6 +143,9 @@ def capture(client, exc_info=None, **kwargs):
                 "stacktrace": frames,
             },
         }
+        if hasattr(exc_value, "_elastic_apm_span_id"):
+            data["parent_id"] = exc_value._elastic_apm_span_id
+            del exc_value._elastic_apm_span_id
         if compat.PY3:
             depth = kwargs.get("_exc_chain_depth", 0)
             if depth > EXCEPTION_CHAIN_MAX_DEPTH:

elasticapm/traces.py

@@ -611,9 +611,16 @@ def __enter__(self):
 
     def __exit__(self, exc_type, exc_val, exc_tb):
         transaction = execution_context.get_transaction()
+
         if transaction and transaction.is_sampled:
             try:
-                transaction.end_span(self.skip_frames, duration=self.duration)
+                span = transaction.end_span(self.skip_frames, duration=self.duration)
+                if exc_val and not isinstance(span, DroppedSpan):
+                    try:
+                        exc_val._elastic_apm_span_id = span.id
+                    except AttributeError:
+                        # could happen if the exception has __slots__
+                        pass
             except LookupError:
                 logger.info("ended non-existing span %s of type %s", self.name, self.type)
 

tests/client/exception_tests.py

@@ -228,19 +228,77 @@ def test_collect_source_errors(elasticapm_client):
         assert "post_context" not in in_app_frame, in_app_frame_context
 
 
-def test_transaction_data_is_attached_to_errors(elasticapm_client):
+def test_transaction_data_is_attached_to_errors_no_transaction(elasticapm_client):
     elasticapm_client.capture_message("noid")
     elasticapm_client.begin_transaction("test")
-    elasticapm_client.capture_message("id")
-    transaction = elasticapm_client.end_transaction("test", "test")
+    elasticapm_client.end_transaction("test", "test")
     elasticapm_client.capture_message("noid")
 
     errors = elasticapm_client.events[ERROR]
     assert "transaction_id" not in errors[0]
-    assert errors[1]["transaction_id"] == transaction.id
-    assert errors[1]["transaction"]["sampled"]
-    assert errors[1]["transaction"]["type"] == "test"
-    assert "transaction_id" not in errors[2]
+    assert "transaction_id" not in errors[1]
+
+
+def test_transaction_data_is_attached_to_errors_message_outside_span(elasticapm_client):
+    elasticapm_client.begin_transaction("test")
+    elasticapm_client.capture_message("outside_span")
+    transaction = elasticapm_client.end_transaction("test", "test")
+
+    error = elasticapm_client.events[ERROR][0]
+    assert error["transaction_id"] == transaction.id
+    assert error["parent_id"] == transaction.id
+    assert error["transaction"]["sampled"]
+    assert error["transaction"]["type"] == "test"
+
+
+def test_transaction_data_is_attached_to_errors_message_in_span(elasticapm_client):
+    elasticapm_client.begin_transaction("test")
+
+    with elasticapm.capture_span("in_span_handler_test") as span_obj:
+        elasticapm_client.capture_message("in_span")
+
+    transaction = elasticapm_client.end_transaction("test", "test")
+
+    error = elasticapm_client.events[ERROR][0]
+
+    assert error["transaction_id"] == transaction.id
+    assert error["parent_id"] == span_obj.id
+    assert error["transaction"]["sampled"]
+    assert error["transaction"]["type"] == "test"
+
+
+def test_transaction_data_is_attached_to_errors_exc_handled_in_span(elasticapm_client):
+    elasticapm_client.begin_transaction("test")
+    with elasticapm.capture_span("in_span_handler_test") as span_obj:
+        try:
+            assert False
+        except AssertionError:
+            elasticapm_client.capture_exception()
+    transaction = elasticapm_client.end_transaction("test", "test")
+
+    error = elasticapm_client.events[ERROR][0]
+
+    assert error["transaction_id"] == transaction.id
+    assert error["parent_id"] == span_obj.id
+    assert error["transaction"]["sampled"]
+    assert error["transaction"]["type"] == "test"
+
+
+def test_transaction_data_is_attached_to_errors_exc_handled_outside_span(elasticapm_client):
+    elasticapm_client.begin_transaction("test")
+    try:
+        with elasticapm.capture_span("out_of_span_handler_test") as span_obj:
+            assert False
+    except AssertionError:
+        elasticapm_client.capture_exception()
+    transaction = elasticapm_client.end_transaction("test", "test")
+
+    error = elasticapm_client.events[ERROR][0]
+
+    assert error["transaction_id"] == transaction.id
+    assert error["parent_id"] == span_obj.id
+    assert error["transaction"]["sampled"]
+    assert error["transaction"]["type"] == "test"
 
 
 def test_transaction_context_is_used_in_errors(elasticapm_client):