From 684e9bf51739433ff4a7f52fc9396f86d08dd46b Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 23 Dec 2024 14:50:50 -0800 Subject: [PATCH 1/4] Add _add_info_status for apm-proto span export --- solarwinds_apm/apm_constants.py | 2 ++ solarwinds_apm/exporter.py | 17 ++++++++++++++++- tests/unit/test_exporter.py | 30 +++++++++++++++++++++++++++++- 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/solarwinds_apm/apm_constants.py b/solarwinds_apm/apm_constants.py index 32c53d85..239c7b01 100644 --- a/solarwinds_apm/apm_constants.py +++ b/solarwinds_apm/apm_constants.py @@ -17,6 +17,8 @@ INTL_SWO_LIBOBOE_TXN_NAME_KEY_PREFIX = "oboe" INTL_SWO_OTEL_SCOPE_NAME = "otel.scope.name" INTL_SWO_OTEL_SCOPE_VERSION = "otel.scope.version" +INTL_SWO_OTEL_STATUS_CODE = "otel.status_code" +INTL_SWO_OTEL_STATUS_DESCRIPTION = "otel.status_description" INTL_SWO_TRACESTATE_KEY = "sw" INTL_SWO_TRANSACTION_ATTR_KEY = "sw.transaction" INTL_SWO_TRANSACTION_ATTR_MAX = 255 diff --git a/solarwinds_apm/exporter.py b/solarwinds_apm/exporter.py index f2aeb21f..3f6a2bbe 100644 --- a/solarwinds_apm/exporter.py +++ b/solarwinds_apm/exporter.py @@ -17,13 +17,15 @@ from typing import Any from opentelemetry.sdk.trace.export import SpanExporter -from opentelemetry.trace import SpanKind +from opentelemetry.trace import SpanKind, StatusCode from opentelemetry.util._importlib_metadata import version from solarwinds_apm.apm_constants import ( INTL_SWO_LIBOBOE_TXN_NAME_KEY_PREFIX, INTL_SWO_OTEL_SCOPE_NAME, INTL_SWO_OTEL_SCOPE_VERSION, + INTL_SWO_OTEL_STATUS_CODE, + INTL_SWO_OTEL_STATUS_DESCRIPTION, INTL_SWO_SUPPORT_EMAIL, ) from solarwinds_apm.w3c_transformer import W3CTransformer @@ -102,6 +104,7 @@ def export(self, spans) -> None: evt.addInfo(self._SW_SPAN_KIND, span.kind.name) evt.addInfo("Language", "Python") self._add_info_instrumentation_scope(span, evt) + self._add_info_status(span, evt) self._add_info_instrumented_framework(span, evt) for attr_k, attr_v in span.attributes.items(): attr_v = self._normalize_attribute_value(attr_v) @@ -153,6 +156,18 @@ def _add_info_instrumentation_scope(self, span, evt) -> None: INTL_SWO_OTEL_SCOPE_VERSION, span.instrumentation_scope.version ) + def _add_info_status(self, span, evt) -> None: + """Add info from span status, if exists""" + # "OK" or "ERROR". Not set if "UNSET" + if span.status.status_code in [StatusCode.ERROR, StatusCode.OK]: + evt.addInfo(INTL_SWO_OTEL_STATUS_CODE, span.status.status_code) + + # Only set if has value + if span.status.description: + evt.addInfo( + INTL_SWO_OTEL_STATUS_DESCRIPTION, span.status.description + ) + # pylint: disable=too-many-branches,too-many-statements def _add_info_instrumented_framework(self, span, evt) -> None: """Add info to span for which Python framework has been instrumented diff --git a/tests/unit/test_exporter.py b/tests/unit/test_exporter.py index baeaf428..1b8c2b29 100644 --- a/tests/unit/test_exporter.py +++ b/tests/unit/test_exporter.py @@ -48,6 +48,13 @@ def get_mock_spans(mocker, valid_parent=False): "version": "foo.bar.baz", } ) + mock_status = mocker.Mock() + mock_status.configure_mock( + **{ + "status_code": "foo-code", + "description": "foo-bar-baz", + } + ) mock_span = mocker.Mock() mock_span_context = mocker.Mock() mock_span_context.configure_mock( @@ -69,6 +76,7 @@ def get_mock_spans(mocker, valid_parent=False): mock_exception_event, ], "instrumentation_scope": mock_instrumentation_scope, + "status": mock_status, } mock_parent = None if valid_parent: @@ -365,6 +373,8 @@ def assert_export_add_info_and_report( mocker.call(solarwinds_apm.exporter.SolarWindsSpanExporter._SW_SPAN_NAME, "foo"), mocker.call(solarwinds_apm.exporter.SolarWindsSpanExporter._SW_SPAN_KIND, FooNum.FOO.name), mocker.call("Language", "Python"), + mocker.call("otel.status_code", "foo-code"), + mocker.call("otel.status_description", "foo-bar-baz"), mocker.call("foo", "bar"), mocker.call("Layer", "FOO:foo"), ]) @@ -422,7 +432,16 @@ def test_export_root_span( mock_add_info_instr_fwork, mock_md, mock_spans_root - ): + ): + mock_statuscode = mocker.patch( + "solarwinds_apm.exporter.StatusCode" + ) + mock_statuscode.configure_mock( + **{ + "ERROR": "foo-code", + "OK": "bar-code", + } + ) mock_build_md = mocker.patch( "solarwinds_apm.exporter.SolarWindsSpanExporter._build_metadata", return_value=mock_md @@ -461,6 +480,15 @@ def test_export_parent_valid( mock_md, mock_spans_parent_valid ): + mock_statuscode = mocker.patch( + "solarwinds_apm.exporter.StatusCode" + ) + mock_statuscode.configure_mock( + **{ + "ERROR": "foo-code", + "OK": "bar-code", + } + ) mock_build_md = mocker.patch( "solarwinds_apm.exporter.SolarWindsSpanExporter._build_metadata", return_value=mock_md From c307b496c624ca4e096a2ede75abb3d441de56ae Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 23 Dec 2024 15:04:48 -0800 Subject: [PATCH 2/4] Add tests --- tests/unit/test_exporter.py | 171 ++++++++++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/tests/unit/test_exporter.py b/tests/unit/test_exporter.py index 1b8c2b29..b690ae22 100644 --- a/tests/unit/test_exporter.py +++ b/tests/unit/test_exporter.py @@ -726,6 +726,177 @@ def test__add_info_instrumentation_scope_name_and_version( mocker.call("otel.scope.version", "bar"), ]) + def mock_span_status(self, mocker): + # mock status code + mock_statuscode = mocker.patch( + "solarwinds_apm.exporter.StatusCode" + ) + mock_statuscode.configure_mock( + **{ + "ERROR": "error-code", + "OK": "ok-code", + "UNSET": "uh-oh-unset", + } + ) + + def test___add_info_status_none( + self, + mocker, + exporter, + mock_event, + mock_create_event, + ): + self.mock_span_status(mocker) + + # mock liboboe event + mock_event, mock_add_info, _ \ + = configure_event_mocks( + mocker, + mock_event, + mock_create_event, + True, + ) + # mock otel span with status + mock_status = mocker.Mock() + mock_status.configure_mock( + **{ + "status_code": None, + "description": None, + } + ) + test_span = mocker.Mock() + test_span.configure_mock( + **{ + "status": mock_status, + } + ) + + exporter._add_info_status( + test_span, + mock_event, + ) + mock_add_info.assert_not_called() + + def test___add_info_status_unset_code( + self, + mocker, + exporter, + mock_event, + mock_create_event, + ): + self.mock_span_status(mocker) + + # mock liboboe event + mock_event, mock_add_info, _ \ + = configure_event_mocks( + mocker, + mock_event, + mock_create_event, + True, + ) + # mock otel span with status + mock_status = mocker.Mock() + mock_status.configure_mock( + **{ + "status_code": "uh-oh-unset", + "description": None, + } + ) + test_span = mocker.Mock() + test_span.configure_mock( + **{ + "status": mock_status, + } + ) + + exporter._add_info_status( + test_span, + mock_event, + ) + mock_add_info.assert_not_called() + + def test___add_info_status_ok_code( + self, + mocker, + exporter, + mock_event, + mock_create_event, + ): + self.mock_span_status(mocker) + + # mock liboboe event + mock_event, mock_add_info, _ \ + = configure_event_mocks( + mocker, + mock_event, + mock_create_event, + True, + ) + # mock otel span with status + mock_status = mocker.Mock() + mock_status.configure_mock( + **{ + "status_code": "ok-code", + "description": "blah blah blah", + } + ) + test_span = mocker.Mock() + test_span.configure_mock( + **{ + "status": mock_status, + } + ) + + exporter._add_info_status( + test_span, + mock_event, + ) + mock_add_info.assert_has_calls([ + mocker.call("otel.status_code", "ok-code"), + mocker.call("otel.status_description", "blah blah blah"), + ]) + + def test___add_info_status_error_code( + self, + mocker, + exporter, + mock_event, + mock_create_event, + ): + self.mock_span_status(mocker) + + # mock liboboe event + mock_event, mock_add_info, _ \ + = configure_event_mocks( + mocker, + mock_event, + mock_create_event, + True, + ) + # mock otel span with status + mock_status = mocker.Mock() + mock_status.configure_mock( + **{ + "status_code": "error-code", + "description": "blah blah blah", + } + ) + test_span = mocker.Mock() + test_span.configure_mock( + **{ + "status": mock_status, + } + ) + + exporter._add_info_status( + test_span, + mock_event, + ) + mock_add_info.assert_has_calls([ + mocker.call("otel.status_code", "error-code"), + mocker.call("otel.status_description", "blah blah blah"), + ]) + def mock_and_assert_addinfo_for_instrumented_framework( self, mocker, From 41b43f8fc593520bc22ff9fe22de0d6aa83c38dc Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 23 Dec 2024 16:43:04 -0800 Subject: [PATCH 3/4] Set kv with status_code.name --- solarwinds_apm/exporter.py | 11 +++++--- tests/unit/test_exporter.py | 54 ++++++++++++++++++++----------------- 2 files changed, 37 insertions(+), 28 deletions(-) diff --git a/solarwinds_apm/exporter.py b/solarwinds_apm/exporter.py index 3f6a2bbe..9743e138 100644 --- a/solarwinds_apm/exporter.py +++ b/solarwinds_apm/exporter.py @@ -158,9 +158,14 @@ def _add_info_instrumentation_scope(self, span, evt) -> None: def _add_info_status(self, span, evt) -> None: """Add info from span status, if exists""" - # "OK" or "ERROR". Not set if "UNSET" - if span.status.status_code in [StatusCode.ERROR, StatusCode.OK]: - evt.addInfo(INTL_SWO_OTEL_STATUS_CODE, span.status.status_code) + # Only set if not "UNSET", e.g. "OK", "ERROR" + if ( + span.status.status_code + and span.status.status_code != StatusCode.UNSET + ): + evt.addInfo( + INTL_SWO_OTEL_STATUS_CODE, span.status.status_code.name + ) # Only set if has value if span.status.description: diff --git a/tests/unit/test_exporter.py b/tests/unit/test_exporter.py index b690ae22..caa9c235 100644 --- a/tests/unit/test_exporter.py +++ b/tests/unit/test_exporter.py @@ -12,11 +12,16 @@ import solarwinds_apm.extension.oboe -# Little helper ===================================================== +# Little helpers ===================================================== class FooNum(Enum): FOO = "foo" +class MockStatus(Enum): + UNSET = 0 + OK = 1 + ERROR = 2 + # Mock Fixtures ===================================================== @@ -51,7 +56,7 @@ def get_mock_spans(mocker, valid_parent=False): mock_status = mocker.Mock() mock_status.configure_mock( **{ - "status_code": "foo-code", + "status_code": FooNum.FOO, "description": "foo-bar-baz", } ) @@ -373,7 +378,7 @@ def assert_export_add_info_and_report( mocker.call(solarwinds_apm.exporter.SolarWindsSpanExporter._SW_SPAN_NAME, "foo"), mocker.call(solarwinds_apm.exporter.SolarWindsSpanExporter._SW_SPAN_KIND, FooNum.FOO.name), mocker.call("Language", "Python"), - mocker.call("otel.status_code", "foo-code"), + mocker.call("otel.status_code", "FOO"), mocker.call("otel.status_description", "foo-bar-baz"), mocker.call("foo", "bar"), mocker.call("Layer", "FOO:foo"), @@ -726,19 +731,6 @@ def test__add_info_instrumentation_scope_name_and_version( mocker.call("otel.scope.version", "bar"), ]) - def mock_span_status(self, mocker): - # mock status code - mock_statuscode = mocker.patch( - "solarwinds_apm.exporter.StatusCode" - ) - mock_statuscode.configure_mock( - **{ - "ERROR": "error-code", - "OK": "ok-code", - "UNSET": "uh-oh-unset", - } - ) - def test___add_info_status_none( self, mocker, @@ -746,7 +738,10 @@ def test___add_info_status_none( mock_event, mock_create_event, ): - self.mock_span_status(mocker) + mock_statuscode = mocker.patch( + "solarwinds_apm.exporter.StatusCode", + return_value=MockStatus, + ) # mock liboboe event mock_event, mock_add_info, _ \ @@ -784,7 +779,10 @@ def test___add_info_status_unset_code( mock_event, mock_create_event, ): - self.mock_span_status(mocker) + mock_statuscode = mocker.patch( + "solarwinds_apm.exporter.StatusCode", + return_value=MockStatus, + ) # mock liboboe event mock_event, mock_add_info, _ \ @@ -798,7 +796,7 @@ def test___add_info_status_unset_code( mock_status = mocker.Mock() mock_status.configure_mock( **{ - "status_code": "uh-oh-unset", + "status_code": mock_statuscode.UNSET, "description": None, } ) @@ -822,7 +820,10 @@ def test___add_info_status_ok_code( mock_event, mock_create_event, ): - self.mock_span_status(mocker) + mock_statuscode = mocker.patch( + "solarwinds_apm.exporter.StatusCode", + return_value=MockStatus, + ) # mock liboboe event mock_event, mock_add_info, _ \ @@ -836,7 +837,7 @@ def test___add_info_status_ok_code( mock_status = mocker.Mock() mock_status.configure_mock( **{ - "status_code": "ok-code", + "status_code": MockStatus.OK, "description": "blah blah blah", } ) @@ -852,7 +853,7 @@ def test___add_info_status_ok_code( mock_event, ) mock_add_info.assert_has_calls([ - mocker.call("otel.status_code", "ok-code"), + mocker.call("otel.status_code", "OK"), mocker.call("otel.status_description", "blah blah blah"), ]) @@ -863,7 +864,10 @@ def test___add_info_status_error_code( mock_event, mock_create_event, ): - self.mock_span_status(mocker) + mock_statuscode = mocker.patch( + "solarwinds_apm.exporter.StatusCode", + return_value=MockStatus, + ) # mock liboboe event mock_event, mock_add_info, _ \ @@ -877,7 +881,7 @@ def test___add_info_status_error_code( mock_status = mocker.Mock() mock_status.configure_mock( **{ - "status_code": "error-code", + "status_code": MockStatus.ERROR, "description": "blah blah blah", } ) @@ -893,7 +897,7 @@ def test___add_info_status_error_code( mock_event, ) mock_add_info.assert_has_calls([ - mocker.call("otel.status_code", "error-code"), + mocker.call("otel.status_code", "ERROR"), mocker.call("otel.status_description", "blah blah blah"), ]) From c8bedfcc4f1383e42021708f82c382f152357aa2 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Mon, 23 Dec 2024 16:47:14 -0800 Subject: [PATCH 4/4] Lint --- tests/unit/test_exporter.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_exporter.py b/tests/unit/test_exporter.py index caa9c235..75d03aee 100644 --- a/tests/unit/test_exporter.py +++ b/tests/unit/test_exporter.py @@ -738,7 +738,7 @@ def test___add_info_status_none( mock_event, mock_create_event, ): - mock_statuscode = mocker.patch( + mocker.patch( "solarwinds_apm.exporter.StatusCode", return_value=MockStatus, ) @@ -820,7 +820,7 @@ def test___add_info_status_ok_code( mock_event, mock_create_event, ): - mock_statuscode = mocker.patch( + mocker.patch( "solarwinds_apm.exporter.StatusCode", return_value=MockStatus, ) @@ -864,7 +864,7 @@ def test___add_info_status_error_code( mock_event, mock_create_event, ): - mock_statuscode = mocker.patch( + mocker.patch( "solarwinds_apm.exporter.StatusCode", return_value=MockStatus, )