Merge pull request #255 from solarwinds/NH-65061-change-processed-txn…

…-name-storage-type

NH-65061 Change processed txn name storage type

solarwinds_apm/trace/base_metrics_processor.py

@@ -5,13 +5,14 @@
 # Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
 
 import logging
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Any
 
 from opentelemetry.sdk.trace import SpanProcessor
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.trace import SpanKind, StatusCode
 
 from solarwinds_apm.apm_constants import INTL_SWO_SUPPORT_EMAIL
+from solarwinds_apm.trace.tnames import TransactionNames
 from solarwinds_apm.w3c_transformer import W3CTransformer
 
 if TYPE_CHECKING:
@@ -38,32 +39,29 @@ def __init__(
     ) -> None:
         self.apm_txname_manager = apm_txname_manager
 
-    def get_trans_name_and_url_tran(
+    def get_tnames(
         self,
         span: "ReadableSpan",
-    ):
-        """Return cached trans_name and url_tran for current trace and span ID"""
-        txn_name_tuple = self.apm_txname_manager.get(
+    ) -> Any:
+        """Return cached TransactionNames for current trace and span ID"""
+        tnames = self.apm_txname_manager.get(
             W3CTransformer.trace_and_span_id_from_context(span.context)
         )
-        if not txn_name_tuple:
+        if not tnames:
             logger.error(
                 "Failed to retrieve transaction name for metrics generation. Please contact %s",
                 INTL_SWO_SUPPORT_EMAIL,
             )
-            return None, None
+            return None
 
-        try:
-            trans_name = txn_name_tuple[0]
-            url_tran = txn_name_tuple[1]
-        except IndexError:
+        if not isinstance(tnames, TransactionNames):
             logger.error(
-                "Failed to retrieve transaction and URL names for metrics generation. Please contact %s",
+                "Something went wrong with storing transaction and URL names for metrics generation. Please contact %s",
                 INTL_SWO_SUPPORT_EMAIL,
             )
-            return None, None
+            return None
 
-        return trans_name, url_tran
+        return tnames
 
     def is_span_http(self, span: "ReadableSpan") -> bool:
         """This span from inbound HTTP request if from a SERVER by some http.method"""

solarwinds_apm/trace/inbound_metrics_processor.py

@@ -57,13 +57,18 @@ def on_end(self, span: "ReadableSpan") -> None:
         ):
             return
 
-        trans_name, url_tran = self.get_trans_name_and_url_tran(span)
-        if not trans_name:
+        tnames = self.get_tnames(span)
+        if not tnames:
             logger.error(
-                "Could not get transaction name. Not recording otlp metrics."
+                "Could not get transaction name. Not recording inbound metrics."
             )
             return
 
+        trans_name = tnames.trans_name
+        url_tran = tnames.url_tran
+        if tnames.custom_name:
+            trans_name = tnames.custom_name
+
         is_span_http = self.is_span_http(span)
         span_time = self.calculate_span_time(
             span.start_time,

solarwinds_apm/trace/otlp_metrics_processor.py

@@ -47,13 +47,17 @@ def on_end(self, span: "ReadableSpan") -> None:
         ):
             return
 
-        trans_name, _ = self.get_trans_name_and_url_tran(span)
-        if not trans_name:
+        tnames = self.get_tnames(span)
+        if not tnames:
             logger.error(
                 "Could not get transaction name. Not recording otlp metrics."
             )
             return
 
+        trans_name = tnames.trans_name
+        if tnames.custom_name:
+            trans_name = tnames.custom_name
+
         # TODO add sw.service_name
         # https://swicloud.atlassian.net/browse/NH-67392
         # support ssa and conform to Otel proto common_pb2

solarwinds_apm/trace/tnames.py

@@ -0,0 +1,21 @@
+# © 2023 SolarWinds Worldwide, LLC. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at:http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+
+from typing import Any
+
+
+class TransactionNames:
+    """Data type to store calculated trans_name, url_tran, and custom_name"""
+
+    def __init__(
+        self,
+        trans_name: str,
+        url_tran: str,
+        custom_name: Any = None,
+    ):
+        self.trans_name = trans_name
+        self.url_tran = url_tran
+        self.custom_name = custom_name

solarwinds_apm/trace/txnname_calculator_processor.py

@@ -10,6 +10,7 @@
 from opentelemetry.sdk.trace import SpanProcessor
 from opentelemetry.semconv.trace import SpanAttributes
 
+from solarwinds_apm.trace.tnames import TransactionNames
 from solarwinds_apm.w3c_transformer import W3CTransformer
 
 if TYPE_CHECKING:
@@ -41,11 +42,11 @@ def __init__(
         self.apm_txname_manager = apm_txname_manager
 
     def on_end(self, span: "ReadableSpan") -> None:
-        """Calculates and stores (trans_name, url_tran) tuple
+        """Calculates and stores automated and custom TransactionNames
         for service entry spans.
 
         If a custom name str was stored by the API, this method
-        overwrites that str with a tuple"""
+        overwrites that str with a new TransactionName object"""
         # Only calculate inbound metrics for service entry spans
         parent_span_context = span.parent
         if (
@@ -56,11 +57,13 @@ def on_end(self, span: "ReadableSpan") -> None:
             return
 
         trans_name, url_tran = self.calculate_transaction_names(span)
+        custom_name = self.calculate_custom_transaction_name(span)
         self.apm_txname_manager[
             W3CTransformer.trace_and_span_id_from_context(span.context)
-        ] = (
+        ] = TransactionNames(
             trans_name,
             url_tran,
+            custom_name,
         )  # type: ignore
 
     # Disable pylint for compatibility with Python3.7 else TypeError
@@ -71,11 +74,8 @@ def calculate_transaction_names(
         url_tran = span.attributes.get(self._HTTP_URL, None)
         http_route = span.attributes.get(self._HTTP_ROUTE, None)
         trans_name = None
-        custom_trans_name = self.calculate_custom_transaction_name(span)
 
-        if custom_trans_name:
-            trans_name = custom_trans_name
-        elif http_route:
+        if http_route:
             trans_name = http_route
         elif span.name:
             trans_name = span.name

tests/unit/test_processors/test_base_metrics_processor.py

@@ -5,6 +5,7 @@
 # Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
 
 from solarwinds_apm.trace.base_metrics_processor import _SwBaseMetricsProcessor
+from solarwinds_apm.trace.tnames import TransactionNames
 
 class TestSwBaseMetricsProcessor:
 
@@ -45,38 +46,39 @@ def patch_get_trans_name(
 
         return mock_txname_manager, mock_span
 
-    def test_get_trans_name_and_url_tran_not_found(self, mocker):
+    def test_get_tnames_not_found(self, mocker):
         mocks = self.patch_get_trans_name(mocker)
         mock_txname_manager = mocks[0]
         mock_span = mocks[1]
         processor = _SwBaseMetricsProcessor(
             mock_txname_manager
         )
-        assert (None, None) == processor.get_trans_name_and_url_tran(mock_span)
+        assert None == processor.get_tnames(mock_span)
 
-    def test_get_trans_name_and_url_tran_indexerror(self, mocker):
+    def test_get_tnames_wrong_type(self, mocker):
         mocks = self.patch_get_trans_name(
             mocker,
-            get_retval=(),
+            get_retval="some-str",
         )
         mock_txname_manager = mocks[0]
         mock_span = mocks[1]
         processor = _SwBaseMetricsProcessor(
             mock_txname_manager
         )
-        assert (None, None) == processor.get_trans_name_and_url_tran(mock_span)
+        assert None == processor.get_tnames(mock_span)
 
-    def test_get_trans_name_and_url_tran_ok(self, mocker):
+    def test_get_tnames_ok(self, mocker):
+        tnames = TransactionNames("foo", "bar", "baz")
         mocks = self.patch_get_trans_name(
             mocker,
-            get_retval=("foo", "bar"),
+            get_retval=tnames,
         )
         mock_txname_manager = mocks[0]
         mock_span = mocks[1]
         processor = _SwBaseMetricsProcessor(
             mock_txname_manager
         )
-        assert ("foo", "bar") == processor.get_trans_name_and_url_tran(mock_span)
+        assert tnames == processor.get_tnames(mock_span)
 
     def test_is_span_http_true(self, mocker):
         mock_spankind = mocker.patch(

tests/unit/test_processors/test_inbound_metrics_processor.py

@@ -7,6 +7,7 @@
 import pytest  # pylint: disable=unused-import
 
 from solarwinds_apm.trace import SolarWindsInboundMetricsSpanProcessor
+from solarwinds_apm.trace.tnames import TransactionNames
 
 
 class TestSolarWindsInboundMetricsSpanProcessor():
@@ -15,7 +16,7 @@ def patch_for_on_end(
         self,
         mocker,
         is_span_http=True,
-        get_retval=("foo", "bar"),
+        get_retval=TransactionNames("foo", "bar"),
     ):
         mock_is_span_http = mocker.patch(
             "solarwinds_apm.trace.SolarWindsInboundMetricsSpanProcessor.is_span_http"
@@ -260,7 +261,7 @@ def test_on_end_missing_txn_name(self, mocker):
         mock_calculate_span_time.assert_not_called()
         mock_has_error.assert_not_called()
 
-    def test_on_end_txn_name_indexerror(self, mocker):
+    def test_on_end_txn_name_wrong_type(self, mocker):
         mock_get_http_status_code, \
             mock_create_http_span, \
             mock_create_span, \
@@ -271,7 +272,7 @@ def test_on_end_txn_name_indexerror(self, mocker):
             mock_calculate_span_time, \
             mock_has_error = self.patch_for_on_end(
                 mocker,
-                get_retval=("only_one_name",),
+                get_retval="some-str",
             )
 
         processor = SolarWindsInboundMetricsSpanProcessor(

tests/unit/test_processors/test_otlp_metrics_processor.py

@@ -5,7 +5,7 @@
 # Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
 
 from solarwinds_apm.trace import SolarWindsOTLPMetricsSpanProcessor
-
+from solarwinds_apm.trace.tnames import TransactionNames
 
 class TestSolarWindsOTLPMetricsSpanProcessor:
 
@@ -14,7 +14,7 @@ def patch_for_on_end(
         mocker,
         has_error=True,
         is_span_http=True,
-        get_retval=("foo", "bar")
+        get_retval=TransactionNames("foo", "bar")
     ):
         mock_random = mocker.patch(
             "solarwinds_apm.trace.otlp_metrics_processor.random"
@@ -259,13 +259,13 @@ def test_on_end_missing_txn_name(self, mocker):
 
         mock_meters.response_time.record.assert_not_called()
 
-    def test_on_end_txn_name_indexerror(self, mocker):
+    def test_on_end_txn_name_wrong_type(self, mocker):
         mock_txname_manager, \
             mock_apm_config, \
             mock_meters, \
             mock_basic_span = self.patch_for_on_end(
                 mocker,
-                get_retval=(),
+                get_retval="some-str",
             )
 
         processor = SolarWindsOTLPMetricsSpanProcessor(
@@ -367,6 +367,7 @@ def test_on_end_not_is_span_http_not_has_error(self, mocker):
                 mocker,
                 has_error=False,
                 is_span_http=False,
+                get_retval=TransactionNames("foo", "bar"),
             )
 
         processor = SolarWindsOTLPMetricsSpanProcessor(

tests/unit/test_processors/test_txnname_calculator_processor.py

@@ -22,8 +22,8 @@ def patch_on_end(self, mocker):
         )
 
         mocker.patch(
-            "solarwinds_apm.trace.TxnNameCalculatorProcessor.calculate_transaction_names",
-            return_value=("foo", "bar")
+            "solarwinds_apm.trace.txnname_calculator_processor.TransactionNames",
+            return_value="foo-tnames",
         )
 
     def test_on_end_valid_local_parent_span(self, mocker):
@@ -70,7 +70,7 @@ def test_on_end_valid_remote_parent_span(self, mocker):
             txn_name_mgr,
         )
         processor.on_end(mock_span)
-        assert txn_name_mgr.get("some-id") == ("foo", "bar")
+        assert txn_name_mgr.get("some-id") == "foo-tnames"
 
     def test_on_end_invalid_remote_parent_span(self, mocker):
         self.patch_on_end(mocker)
@@ -93,7 +93,7 @@ def test_on_end_invalid_remote_parent_span(self, mocker):
             txn_name_mgr,
         )
         processor.on_end(mock_span)
-        assert txn_name_mgr.get("some-id") == ("foo", "bar")
+        assert txn_name_mgr.get("some-id") == "foo-tnames"
 
     def test_on_end_invalid_local_parent_span(self, mocker):
         self.patch_on_end(mocker)
@@ -116,7 +116,7 @@ def test_on_end_invalid_local_parent_span(self, mocker):
             txn_name_mgr,
         )
         processor.on_end(mock_span)
-        assert txn_name_mgr.get("some-id") == ("foo", "bar")
+        assert txn_name_mgr.get("some-id") == "foo-tnames"
 
     def test_on_end_missing_parent(self, mocker):
         self.patch_on_end(mocker)
@@ -132,7 +132,7 @@ def test_on_end_missing_parent(self, mocker):
             txn_name_mgr,
         )
         processor.on_end(mock_span)
-        assert txn_name_mgr.get("some-id") == ("foo", "bar")
+        assert txn_name_mgr.get("some-id") == "foo-tnames"
 
     def test_calculate_transaction_names_span_name_default(self, mocker):
         """Otel Python span.name should always exist"""