NH-125600 Add ServiceEntrySpanProcessor._on_ending implementation to reduce overall name pool registrations

solarwinds_apm/api/__init__.py

@@ -14,11 +14,10 @@
 from solarwinds_apm.apm_constants import (
     INTL_SWO_OTEL_CONTEXT_ENTRY_SPAN,
     INTL_SWO_TRANSACTION_ATTR_KEY,
+    INTL_SWO_TRANSACTION_ATTR_MAX,
 )
-from solarwinds_apm.oboe import get_transaction_name_pool
 from solarwinds_apm.oboe.http_sampler import HttpSampler
 from solarwinds_apm.oboe.json_sampler import JsonSampler
-from solarwinds_apm.oboe.transaction_name_pool import TRANSACTION_NAME_DEFAULT
 from solarwinds_apm.sampler import ParentBasedSwSampler
 from solarwinds_apm.tracer_provider import SolarwindsTracerProvider
 from solarwinds_apm.w3c_transformer import W3CTransformer
@@ -35,7 +34,7 @@ def set_transaction_name(custom_name: str) -> bool:
     Takes precedence over transaction_name set in environment variable or config file.
 
     Any uppercase to lowercase conversions or special character replacements
-    are done by the platform. Name length is limited to 256 characters;
+    are done by the platform. Name length is limited to 255 characters;
     anything longer is truncated by APM library.
 
     Parameters:
@@ -82,19 +81,11 @@ def set_transaction_name(custom_name: str) -> bool:
         custom_name,
     )
 
-    # check limit pool; set as "other" if reached and log debug/warning
-    pool = get_transaction_name_pool()
-    registered_name = pool.registered(custom_name)
-    if registered_name == TRANSACTION_NAME_DEFAULT:
-        logger.warning(
-            "Transaction name pool is full; set as %s for span %s",
-            TRANSACTION_NAME_DEFAULT,
-            W3CTransformer.trace_and_span_id_from_context(
-                current_trace_entry_span.context
-            ),
-        )
+    # Set transaction name attribute (will be finalized via pool in _on_ending)
+    # Truncate to max length
     current_trace_entry_span.set_attribute(
-        INTL_SWO_TRANSACTION_ATTR_KEY, registered_name
+        INTL_SWO_TRANSACTION_ATTR_KEY,
+        custom_name[:INTL_SWO_TRANSACTION_ATTR_MAX],
     )
     return True
 

solarwinds_apm/trace/serviceentry_processor.py

@@ -33,8 +33,6 @@
 if TYPE_CHECKING:
     from opentelemetry.sdk.trace import ReadableSpan
 
-    from solarwinds_apm.oboe.transaction_name_pool import TransactionNamePool
-
 logger = logging.getLogger(__name__)
 
 
@@ -55,30 +53,27 @@ def __init__(self) -> None:
     def set_default_transaction_name(
         self,
         span: Span,
-        pool: TransactionNamePool,
         attribute_value: str,
         resolve: bool = False,
     ) -> None:
         """
-        Register transaction name in pool and set as span attribute.
+        Set initial transaction name as span attribute.
+
+        Pool registration is deferred to _on_ending() for all transaction names.
 
         Parameters:
         span (Span): The span to set the transaction name on.
-        pool (TransactionNamePool): The transaction name pool for registration.
-        attribute_value (str): The transaction name value to register.
+        attribute_value (str): The transaction name value.
         resolve (bool): Whether to resolve the transaction name (e.g., for URL paths). Defaults to False.
         """
         transaction_name = attribute_value
         if resolve:
             transaction_name = resolve_transaction_name(attribute_value)
-        registered_name = pool.registered(transaction_name)
-        if registered_name == TRANSACTION_NAME_DEFAULT:
-            logger.warning(
-                "Transaction name pool is full; set as %s for span %s",
-                TRANSACTION_NAME_DEFAULT,
-                W3CTransformer.trace_and_span_id_from_context(span.context),
-            )
-        span.set_attribute(INTL_SWO_TRANSACTION_ATTR_KEY, registered_name)
+        # Set attribute without pool registration (finalized in _on_ending)
+        span.set_attribute(
+            INTL_SWO_TRANSACTION_ATTR_KEY,
+            transaction_name[:INTL_SWO_TRANSACTION_ATTR_MAX],
+        )
 
     def on_start(
         self,
@@ -95,7 +90,7 @@ def on_start(
         3. AWS_LAMBDA_FUNCTION_NAME
         4. Any instrumentor-set span attributes for HTTP
         5. Span name (default)
-        6. "other" (when the transaction name pool limit reached)
+        6. "other" (when the transaction name pool limit reached at span _on_ending)
 
         If entry span, caches it in context for custom transaction naming.
 
@@ -114,31 +109,26 @@ def on_start(
 
         # Calculate non-custom txn name for entry span if we can retrieve the URL
         # or serverless name. Otherwise, use the span's name
-        pool = get_transaction_name_pool()
-
         sw_apm_txn_name = os.environ.get("SW_APM_TRANSACTION_NAME", None)
         faas_name = span.attributes.get(ResourceAttributes.FAAS_NAME, None)
         lambda_function_name = os.environ.get("AWS_LAMBDA_FUNCTION_NAME", None)
         http_route = span.attributes.get(SpanAttributes.HTTP_ROUTE, None)
         url_path = span.attributes.get(SpanAttributes.URL_PATH, None)
         if sw_apm_txn_name:
-            self.set_default_transaction_name(span, pool, sw_apm_txn_name)
+            self.set_default_transaction_name(span, sw_apm_txn_name)
         elif faas_name:
-            self.set_default_transaction_name(span, pool, faas_name)
+            self.set_default_transaction_name(span, faas_name)
         elif lambda_function_name:
             self.set_default_transaction_name(
                 span,
-                pool,
                 lambda_function_name[:INTL_SWO_TRANSACTION_ATTR_MAX],
             )
         elif http_route:
-            self.set_default_transaction_name(span, pool, http_route)
+            self.set_default_transaction_name(span, http_route)
         elif url_path:
-            self.set_default_transaction_name(
-                span, pool, url_path, resolve=True
-            )
+            self.set_default_transaction_name(span, url_path, resolve=True)
         else:
-            self.set_default_transaction_name(span, pool, span.name)
+            self.set_default_transaction_name(span, span.name)
 
         # Cache the entry span in current context to use upstream-managed
         # execution scope and handle async tracing, for custom naming
@@ -164,6 +154,45 @@ def on_start(
         )
         self.context_tokens[entry_trace_span_id] = token
 
+    def _on_ending(self, span: Span) -> None:
+        """
+        Finalize transaction name by registering with pool.
+
+        This is called before the span becomes immutable, allowing us to update
+        the transaction name attribute with the final pool-registered version.
+
+        See span on_start for order of precendence for transaction name setting.
+        If transaction name pool limit is reached here at _on_ending, then
+        name is set here as "other".
+
+        Parameters:
+        span (Span): The span that is ending (still mutable).
+        """
+        # Only process entry spans
+        parent_span_context = span.parent
+        if (
+            parent_span_context
+            and parent_span_context.is_valid
+            and not parent_span_context.is_remote
+        ):
+            return
+
+        # Read whatever transaction name is set (initial OR user-set)
+        txn_name = span.attributes.get(INTL_SWO_TRANSACTION_ATTR_KEY)
+        if txn_name:
+            pool = get_transaction_name_pool()
+            registered_name = pool.registered(txn_name)
+            if registered_name == TRANSACTION_NAME_DEFAULT:
+                logger.warning(
+                    "Transaction name pool is full; set as %s for span %s",
+                    TRANSACTION_NAME_DEFAULT,
+                    W3CTransformer.trace_and_span_id_from_context(
+                        span.context
+                    ),
+                )
+            # Update with pool-registered name
+            span.set_attribute(INTL_SWO_TRANSACTION_ATTR_KEY, registered_name)
+
     def on_end(self, span: ReadableSpan) -> None:
         """
         Clean up context for service entry spans.

tests/integration/test_set_transaction_name.py

@@ -335,7 +335,7 @@ def test_no_active_span_returns_false(self):
         assert result is False
 
     def test_long_name_truncated(self):
-        """Test long transaction names are truncated to 256 characters"""
+        """Test long transaction names are truncated to 255 characters"""
         timestamp = int(time.time())
         with mock.patch(
             target="solarwinds_apm.oboe.json_sampler.JsonSampler._read",
@@ -376,8 +376,8 @@ def test_long_name_truncated(self):
             entry_span = entry_spans[0]
             txn_name = entry_span.attributes.get(INTL_SWO_TRANSACTION_ATTR_KEY)
             assert txn_name is not None
-            assert len(txn_name) == 256
-            assert txn_name == "a" * 256
+            assert len(txn_name) == 255
+            assert txn_name == "a" * 255
 
             # Verify metrics also have correct transaction name
             metrics = self._get_metrics_for_transaction(txn_name)

tests/unit/test_api.py

@@ -24,13 +24,6 @@ def patch_set_name(
         mocker,
         span_ready=True,
     ):
-        mock_pool = mocker.patch(
-            "solarwinds_apm.api.get_transaction_name_pool"
-        )
-        mock_pool_instance = mocker.Mock()
-        mock_pool.return_value = mock_pool_instance
-        mock_pool_instance.registered.return_value = "mock-registered-name"
-
         mocker.patch(
             "solarwinds_apm.w3c_transformer.W3CTransformer.trace_and_span_id_from_context",
             return_value="foo",
@@ -54,39 +47,35 @@ def patch_set_name(
                 "get_value": mock_get_fn
             }
         )
-        return mock_context, mock_pool_instance, mock_current_span
+        return mock_context, mock_current_span
 
     def test_empty_string(self, mocker):
-        mock_context, mock_pool, mock_current_span = self.patch_set_name(mocker)
+        mock_context, mock_current_span = self.patch_set_name(mocker)
         assert set_transaction_name("") == False
         mock_context.get_value.assert_not_called()
-        mock_pool.registered.assert_not_called()
         mock_current_span.set_attribute.assert_not_called()
 
     def test_agent_not_enabled_noop_tracer_provider(self, mocker):
-        mock_context, mock_pool, mock_current_span = self.patch_set_name(mocker)
+        mock_context, mock_current_span = self.patch_set_name(mocker)
         mocker.patch(
             "solarwinds_apm.api.get_tracer_provider",
             return_value=NoOpTracerProvider()
         )
         assert set_transaction_name("foo") == True
         mock_context.get_value.assert_not_called()
-        mock_pool.registered.assert_not_called()
         mock_current_span.set_attribute.assert_not_called()
 
     def test_span_not_started(self, mocker):
-        mock_context, mock_pool, mock_current_span = self.patch_set_name(mocker, span_ready=False)
+        mock_context, mock_current_span = self.patch_set_name(mocker, span_ready=False)
         assert set_transaction_name("foo") == False
         mock_context.get_value.assert_called_once()
-        mock_pool.registered.assert_not_called()
         mock_current_span.set_attribute.assert_not_called()
 
     def test_agent_enabled(self, mocker):
-        mock_context, mock_pool, mock_current_span = self.patch_set_name(mocker)
+        mock_context, mock_current_span = self.patch_set_name(mocker)
         assert set_transaction_name("bar") == True
         mock_context.get_value.assert_called_once_with("sw-current-trace-entry-span")
-        mock_pool.registered.assert_called_once_with("bar")
-        mock_current_span.set_attribute.assert_called_once_with("sw.transaction", "mock-registered-name")
+        mock_current_span.set_attribute.assert_called_once_with("sw.transaction", "bar")
 
 class TestSolarWindsReady:
     def test_parentbasedsw_sampler_ready(self, mocker):

tests/unit/test_processors/test_serviceentry_processor.py

@@ -91,6 +91,7 @@ def test_on_start_valid_remote_parent_span(self, mocker):
             **{
                 "parent": mock_parent,
                 "attributes.get": mock_attrs_get,
+                "name": "default-span-name",
             }
         )
         processor = ServiceEntrySpanProcessor()
@@ -125,6 +126,7 @@ def test_on_start_invalid_remote_parent_span(self, mocker):
             **{
                 "parent": mock_parent,
                 "attributes.get": mock_attrs_get,
+                "name": "default-span-name",
             }
         )
         processor = ServiceEntrySpanProcessor()
@@ -159,6 +161,7 @@ def test_on_start_invalid_local_parent_span(self, mocker):
             **{
                 "parent": mock_parent,
                 "attributes.get": mock_attrs_get,
+                "name": "default-span-name",
             }
         )
         processor = ServiceEntrySpanProcessor()
@@ -186,6 +189,7 @@ def test_on_start_missing_parent(self, mocker):
             **{
                 "parent": None,
                 "attributes.get": mock_attrs_get,
+                "name": "default-span-name",
             }
         )
         processor = ServiceEntrySpanProcessor()
@@ -222,7 +226,7 @@ def test_on_start_sw_apm_transaction_name(self, mocker):
         processor.set_default_transaction_name = mocker.Mock()
         processor.on_start(mock_span, None)
         processor.set_default_transaction_name.assert_called_once_with(
-            mock_span, mock_pool, "sw-apm-transaction"[:INTL_SWO_TRANSACTION_ATTR_MAX]
+            mock_span, "sw-apm-transaction"[:INTL_SWO_TRANSACTION_ATTR_MAX]
         )
 
     def test_on_start_faas_name(self, mocker):
@@ -243,7 +247,7 @@ def test_on_start_faas_name(self, mocker):
         processor.set_default_transaction_name = mocker.Mock()
         processor.on_start(mock_span, None)
         processor.set_default_transaction_name.assert_called_once_with(
-            mock_span, mock_pool, "faas-value"
+            mock_span, "faas-value"
         )
 
     def test_on_start_lambda_function_name(self, mocker):
@@ -263,7 +267,7 @@ def test_on_start_lambda_function_name(self, mocker):
         processor.set_default_transaction_name = mocker.Mock()
         processor.on_start(mock_span, None)
         processor.set_default_transaction_name.assert_called_once_with(
-            mock_span, mock_pool, "lambda-function"[:INTL_SWO_TRANSACTION_ATTR_MAX]
+            mock_span, "lambda-function"[:INTL_SWO_TRANSACTION_ATTR_MAX]
         )
 
     def test_on_start_http_route(self, mocker):
@@ -284,7 +288,7 @@ def test_on_start_http_route(self, mocker):
         processor.set_default_transaction_name = mocker.Mock()
         processor.on_start(mock_span, None)
         processor.set_default_transaction_name.assert_called_once_with(
-            mock_span, mock_pool, "http-route"
+            mock_span, "http-route"
         )
 
     def test_on_start_url_path(self, mocker):
@@ -305,7 +309,7 @@ def test_on_start_url_path(self, mocker):
         processor.set_default_transaction_name = mocker.Mock()
         processor.on_start(mock_span, None)
         processor.set_default_transaction_name.assert_called_once_with(
-            mock_span, mock_pool, "url-path", resolve=True
+            mock_span, "url-path", resolve=True
         )
 
     def test_on_start_default(self, mocker):
@@ -325,7 +329,7 @@ def test_on_start_default(self, mocker):
         processor.set_default_transaction_name = mocker.Mock()
         processor.on_start(mock_span, None)
         processor.set_default_transaction_name.assert_called_once_with(
-            mock_span, mock_pool, "default-span-name"
+            mock_span, "default-span-name"
         )
 
     def test_on_end_valid_local_parent_span(self, mocker):