From 44ee444dbdeee54f7854c4059168d6a8855852cd Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Wed, 29 Jul 2026 18:52:17 -0700 Subject: [PATCH 1/2] WIP --- solarwinds_apm/api/__init__.py | 19 ++--- .../trace/serviceentry_processor.py | 74 +++++++++++++------ .../integration/test_set_transaction_name.py | 6 +- tests/unit/test_api.py | 23 ++---- .../test_serviceentry_processor.py | 16 ++-- 5 files changed, 74 insertions(+), 64 deletions(-) diff --git a/solarwinds_apm/api/__init__.py b/solarwinds_apm/api/__init__.py index 57a412f99..32717c3fe 100644 --- a/solarwinds_apm/api/__init__.py +++ b/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 @@ -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 diff --git a/solarwinds_apm/trace/serviceentry_processor.py b/solarwinds_apm/trace/serviceentry_processor.py index 59733a641..177c6ccdf 100644 --- a/solarwinds_apm/trace/serviceentry_processor.py +++ b/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, @@ -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,42 @@ 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 pool-registered version. + Processes all transaction names (both initial and user-set via set_transaction_name). + + 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. diff --git a/tests/integration/test_set_transaction_name.py b/tests/integration/test_set_transaction_name.py index d0560d9df..0129e6fec 100644 --- a/tests/integration/test_set_transaction_name.py +++ b/tests/integration/test_set_transaction_name.py @@ -277,7 +277,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", @@ -318,8 +318,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 class TestSetTransactionNameDistributed(TestBaseSwHeadersAndAttributes): diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index 0aa07c958..c6be9f4df 100644 --- a/tests/unit/test_api.py +++ b/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): diff --git a/tests/unit/test_processors/test_serviceentry_processor.py b/tests/unit/test_processors/test_serviceentry_processor.py index cbfbd9983..3885ed721 100644 --- a/tests/unit/test_processors/test_serviceentry_processor.py +++ b/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): From 30f830520af5ecba6cb11f14c0091c22e8622e66 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 30 Jul 2026 13:11:54 -0700 Subject: [PATCH 2/2] Update docstring --- solarwinds_apm/api/__init__.py | 2 +- solarwinds_apm/trace/serviceentry_processor.py | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/solarwinds_apm/api/__init__.py b/solarwinds_apm/api/__init__.py index 32717c3fe..6dfd3b1fc 100644 --- a/solarwinds_apm/api/__init__.py +++ b/solarwinds_apm/api/__init__.py @@ -34,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: diff --git a/solarwinds_apm/trace/serviceentry_processor.py b/solarwinds_apm/trace/serviceentry_processor.py index 177c6ccdf..0d906b232 100644 --- a/solarwinds_apm/trace/serviceentry_processor.py +++ b/solarwinds_apm/trace/serviceentry_processor.py @@ -90,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. @@ -159,8 +159,11 @@ 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 pool-registered version. - Processes all transaction names (both initial and user-set via set_transaction_name). + 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).