Skip to content

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

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

Merged
merged 3 commits into from
Dec 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down
4 changes: 3 additions & 1 deletion elasticapm/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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}

Expand Down
10 changes: 8 additions & 2 deletions elasticapm/contrib/asyncio/traces.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)
3 changes: 3 additions & 0 deletions elasticapm/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
9 changes: 8 additions & 1 deletion elasticapm/traces.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
72 changes: 65 additions & 7 deletions tests/client/exception_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down