From f0b8eb6cc255adb1e25ae08dae1c77d4ab2a92ea Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 17 Dec 2024 09:53:07 +0100 Subject: [PATCH 1/6] Fix arq tests --- sentry_sdk/integrations/arq.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/arq.py b/sentry_sdk/integrations/arq.py index 5aa0ba7302..a77f775c73 100644 --- a/sentry_sdk/integrations/arq.py +++ b/sentry_sdk/integrations/arq.py @@ -1,5 +1,7 @@ import sys +from opentelemetry.trace.status import StatusCode + import sentry_sdk from sentry_sdk.consts import OP, SPANSTATUS from sentry_sdk.integrations import DidNotEnable, Integration @@ -116,7 +118,14 @@ async def _sentry_run_job(self, job_id, score): origin=ArqIntegration.origin, ) as span: return_value = await old_run_job(self, job_id, score) - span.set_status(SPANSTATUS.OK) + + status_unset = ( + hasattr(span._otel_span, "status") + and span._otel_span.status.status_code == StatusCode.UNSET + ) + if status_unset: + span.set_status(SPANSTATUS.OK) + return return_value Worker.run_job = _sentry_run_job From dc9e4a348f9955e7db7ea1f5f0608c15ce45d08d Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 17 Dec 2024 13:52:50 +0100 Subject: [PATCH 2/6] Do not access lower level otel_span from integration --- sentry_sdk/integrations/arq.py | 6 +----- sentry_sdk/tracing.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/arq.py b/sentry_sdk/integrations/arq.py index a77f775c73..c26db4520c 100644 --- a/sentry_sdk/integrations/arq.py +++ b/sentry_sdk/integrations/arq.py @@ -119,11 +119,7 @@ async def _sentry_run_job(self, job_id, score): ) as span: return_value = await old_run_job(self, job_id, score) - status_unset = ( - hasattr(span._otel_span, "status") - and span._otel_span.status.status_code == StatusCode.UNSET - ) - if status_unset: + if span.status is None: span.set_status(SPANSTATUS.OK) return return_value diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 7686dcf052..015317b87f 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1583,6 +1583,22 @@ def set_attribute(self, key, value): self._otel_span.set_attribute(key, _serialize_span_attribute(value)) + @property + def status(self): + # type: () -> Optional[str] + if not hasattr(self._otel_span, "status"): + return None + + if self._otel_span.status.status_code == StatusCode.OK: + return SPANSTATUS.OK + else: + return SPANSTATUS.UNKNOWN_ERROR + + @status.setter + def status(self, value): + # type: (Optional[Any]) -> None + raise NotImplementedError("Use set_status instead") + def set_status(self, status): # type: (str) -> None if status == SPANSTATUS.OK: From 77338bbcbfbe3d62ad178990c8a7a4a54affdd06 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 17 Dec 2024 13:58:52 +0100 Subject: [PATCH 3/6] Some explaination --- sentry_sdk/tracing.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 015317b87f..79220592c5 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1586,6 +1586,12 @@ def set_attribute(self, key, value): @property def status(self): # type: () -> Optional[str] + """ + Return the Sentry `SPANSTATUS` corresponding to the underlying OTel status. + Because differences in possible values in OTel `StatusCode` and + Sentry `SPANSTATUS` it can not be guaranteed that the status + set in `set_status()` will be the same as the one returned here. + """ if not hasattr(self._otel_span, "status"): return None From ad035d1b6f005dcf446e7bb2b4efcb08453b3815 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 17 Dec 2024 14:04:27 +0100 Subject: [PATCH 4/6] Forgot unset --- sentry_sdk/tracing.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 79220592c5..87868eb95f 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1595,7 +1595,9 @@ def status(self): if not hasattr(self._otel_span, "status"): return None - if self._otel_span.status.status_code == StatusCode.OK: + if self._otel_span.status.status_code == StatusCode.UNSET: + return None + elif self._otel_span.status.status_code == StatusCode.OK: return SPANSTATUS.OK else: return SPANSTATUS.UNKNOWN_ERROR From 09faed0724f44857a0109432cce37adb118573cb Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 17 Dec 2024 15:16:49 +0100 Subject: [PATCH 5/6] Added tests for status property --- .../integrations/opentelemetry/test_potel.py | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/integrations/opentelemetry/test_potel.py b/tests/integrations/opentelemetry/test_potel.py index 39c48f8cc8..2d1d66c6d0 100644 --- a/tests/integrations/opentelemetry/test_potel.py +++ b/tests/integrations/opentelemetry/test_potel.py @@ -2,6 +2,7 @@ from opentelemetry import trace import sentry_sdk +from sentry_sdk.consts import SPANSTATUS from tests.conftest import ApproxDict @@ -331,3 +332,35 @@ def test_potel_span_root_span_references(): with sentry_sdk.start_span(description="http") as http_span: assert not http_span.is_root_span assert http_span.root_span == request_span + + +@pytest.mark.parametrize( + "status_in,status_out", + [ + (None, None), + ("", SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.OK, SPANSTATUS.OK), + (SPANSTATUS.ABORTED, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.ALREADY_EXISTS, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.CANCELLED, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.DATA_LOSS, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.DEADLINE_EXCEEDED, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.FAILED_PRECONDITION, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.INTERNAL_ERROR, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.INVALID_ARGUMENT, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.NOT_FOUND, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.OUT_OF_RANGE, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.PERMISSION_DENIED, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.RESOURCE_EXHAUSTED, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.UNAUTHENTICATED, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.UNAVAILABLE, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.UNIMPLEMENTED, SPANSTATUS.UNKNOWN_ERROR), + (SPANSTATUS.UNKNOWN_ERROR, SPANSTATUS.UNKNOWN_ERROR), + ], +) +def test_potel_span_status(status_in, status_out): + span = sentry_sdk.start_span(name="test") + if status_in is not None: + span.set_status(status_in) + + assert span.status == status_out From a981e0bc9aac69d0b56c09427907f64e45a2d546 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 18 Dec 2024 09:28:44 +0100 Subject: [PATCH 6/6] Remove status setter warning --- sentry_sdk/tracing.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 87868eb95f..a0b9439dc8 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1602,11 +1602,6 @@ def status(self): else: return SPANSTATUS.UNKNOWN_ERROR - @status.setter - def status(self, value): - # type: (Optional[Any]) -> None - raise NotImplementedError("Use set_status instead") - def set_status(self, status): # type: (str) -> None if status == SPANSTATUS.OK: