Uh oh!
There was an error while loading. Please reload this page.
NH-125600 Add ServiceEntrySpanProcessor._on_ending implementation to reduce overall name pool registrations - #833
NH-125600 Add ServiceEntrySpanProcessor._on_ending implementation to reduce overall name pool registrations#833tammy-baylis-swi wants to merge 13 commits into
ServiceEntrySpanProcessor._on_ending implementation to reduce overall name pool registrations#833Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the SolarWinds APM OpenTelemetry distribution to defer transaction-name pool registration until span ending by implementing ServiceEntrySpanProcessor._on_ending (available in newer OTel SDKs), reducing repeated pool.registered(...) calls and improving pool utilization. It also updates unit/integration tests to match the new “set attribute first, register at ending” behavior and corrects truncation expectations to 255 characters.
Changes:
- Add
ServiceEntrySpanProcessor._on_endingto register the finalsw.transactionvalue with the transaction name pool just before the span becomes immutable. - Update
set_transaction_name()to only set (and truncate) the span attribute; pool registration is deferred to_on_ending. - Adjust unit/integration tests to reflect the new flow and the 255-character truncation limit.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
solarwinds_apm/trace/serviceentry_processor.py | Defers pool registration to _on_ending and updates default transaction-name setting logic. |
solarwinds_apm/api/__init__.py | Stops pool registration in set_transaction_name() and truncates the attribute value to INTL_SWO_TRANSACTION_ATTR_MAX. |
tests/unit/test_processors/test_serviceentry_processor.py | Updates unit tests for new set_default_transaction_name signature and default span naming inputs. |
tests/unit/test_api.py | Updates set_transaction_name() unit tests to expect direct attribute setting (no pool registration). |
tests/integration/test_set_transaction_name.py | Corrects long-name truncation assertions to 255 characters. |
Comments suppressed due to low confidence (1)
solarwinds_apm/trace/serviceentry_processor.py:162
- _on_ending() introduces a new critical code path that performs the transaction name pool registration and applies the "other" fallback. There are unit tests for on_start()/on_end(), but none that assert _on_ending() registers exactly once for entry spans, skips non-entry spans, and updates the span attribute based on pool.registered(). Adding targeted unit tests would reduce regression risk for this behavior change.
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Implements
ServiceEntrySpanProcessor._on_ending, made available with OTel 1.40.0/0.61b0. This reduces total calls topool.registered(transaction_name)fromn + 1to1, uses the pool more efficiently (e.g. fewer intermediates taking spots). See ticket for more info.Also includes accidental bugfix to 256 --> 255 char limit in integration tests.
API's
set_transaction_nameusage and behaviour are still the same; it will log fewerwarningwhenever pool capacity reached. Name setting and response time metrics attribute setting are also the same.