Skip to content

NH-104520 Update SwPropagator class; redesign propagator config #687

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tammy-baylis-swi merged 9 commits into from
Sep 8, 2025
Merged

NH-104520 Update SwPropagator class; redesign propagator config #687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c6f4d0a
SwPropagator subclasses TraceContextTextMapProp
tammy-baylis-swi Sep 3, 2025
80891e3
Simplify propagators config rules
tammy-baylis-swi Sep 4, 2025
3de2a7a
Update tests
tammy-baylis-swi Sep 4, 2025
86eb4a8
Update message
tammy-baylis-swi Sep 4, 2025
6f96e3c
Rm unnecessary is-none check
tammy-baylis-swi Sep 4, 2025
6be8da1
agent disabled if tracecontext in propagators
tammy-baylis-swi Sep 4, 2025
20529a2
Update comment
tammy-baylis-swi Sep 4, 2025
da697e1
Update message
tammy-baylis-swi Sep 4, 2025
c7ccfdb
Allow different order; add tests
tammy-baylis-swi Sep 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 10 additions & 26 deletions solarwinds_apm/apm_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

from solarwinds_apm import apm_logging
from solarwinds_apm.apm_constants import (
INTL_SWO_BAGGAGE_PROPAGATOR,
INTL_SWO_DEFAULT_PROPAGATORS,
INTL_SWO_PROPAGATOR,
INTL_SWO_TRACECONTEXT_PROPAGATOR,
Expand Down Expand Up @@ -230,36 +229,21 @@ def _calculate_agent_enabled_config(self) -> bool:
).split(",")
# If not using the default propagators,
# can any arbitrary list BUT
# (a) must include tracecontext and solarwinds_propagator
# (b) tracecontext must be before solarwinds_propagator
# (c) baggage, if configured, must be before solarwinds_propagator
if environ_propagators != INTL_SWO_DEFAULT_PROPAGATORS:
if (
INTL_SWO_TRACECONTEXT_PROPAGATOR not in environ_propagators
or INTL_SWO_PROPAGATOR not in environ_propagators
):
# (a) must include solarwinds_propagator
# (b) must not include tracecontext, which may conflict
if set(environ_propagators) != set(INTL_SWO_DEFAULT_PROPAGATORS):
if INTL_SWO_PROPAGATOR not in environ_propagators:
logger.error(
"Must include tracecontext and solarwinds_propagator in OTEL_PROPAGATORS to use SolarWinds APM. Tracing disabled."
"Must include solarwinds_propagator in OTEL_PROPAGATORS to use SolarWinds APM. Tracing disabled."
)
return False

if environ_propagators.index(
INTL_SWO_PROPAGATOR
) < environ_propagators.index(
INTL_SWO_TRACECONTEXT_PROPAGATOR
):
if INTL_SWO_TRACECONTEXT_PROPAGATOR in environ_propagators:
logger.error(
"tracecontext must be before solarwinds_propagator in OTEL_PROPAGATORS to use SolarWinds APM. Tracing disabled."
)
return False

if (
INTL_SWO_BAGGAGE_PROPAGATOR in environ_propagators
and environ_propagators.index(INTL_SWO_PROPAGATOR)
< environ_propagators.index(INTL_SWO_BAGGAGE_PROPAGATOR)
):
logger.error(
"baggage must be before solarwinds_propagator in OTEL_PROPAGATORS to use SolarWinds APM. Tracing disabled."
"It is unnecessary to configure tracecontext in "
"OTEL_PROPAGATORS when using SolarWinds APM >= 4.4.0, "
"which has built-in w3c context propagation. Please "
"update your configuration. Tracing disabled."
)
return False

Expand Down
3 changes: 1 addition & 2 deletions solarwinds_apm/apm_constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@
INTL_SWO_BAGGAGE_PROPAGATOR = "baggage"
INTL_SWO_PROPAGATOR = "solarwinds_propagator"
INTL_SWO_DEFAULT_PROPAGATORS = [
INTL_SWO_TRACECONTEXT_PROPAGATOR,
INTL_SWO_BAGGAGE_PROPAGATOR,
INTL_SWO_PROPAGATOR,
INTL_SWO_BAGGAGE_PROPAGATOR,
]
INTL_SWO_DEFAULT_RESOURCE_DETECTORS = [
"process",
Expand Down
27 changes: 16 additions & 11 deletions solarwinds_apm/propagator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
from opentelemetry import trace
from opentelemetry.context.context import Context
from opentelemetry.propagators import textmap
from opentelemetry.trace.propagation.tracecontext import (
TraceContextTextMapPropagator,
)
from opentelemetry.trace.span import TraceState

from solarwinds_apm.apm_constants import (
Expand All @@ -25,9 +28,9 @@
logger = logging.getLogger(__name__)


class SolarWindsPropagator(textmap.TextMapPropagator):
"""Extracts and injects SolarWinds headers for trace propagation.
Must be used in composite with TraceContextTextMapPropagator.
class SolarWindsPropagator(TraceContextTextMapPropagator):
"""Extracts and injects SolarWinds headers and W3C trace context
headers for trace propagation.
"""

_INVALID_SPAN_ID = 0x0000000000000000
Expand All @@ -41,11 +44,10 @@ def extract(
context: Context | None = None,
getter: textmap.Getter = textmap.default_getter,
) -> Context:
"""Extracts sw trace options and signature from carrier into OTel
Context. Note: tracestate is extracted by TraceContextTextMapPropagator
"""Extracts traceparent, tracestate, sw trace options, and signature
from carrier into OTel Context.
"""
if context is None:
context = Context()
context = super().extract(carrier, context, getter)

xtraceoptions_header = getter.get(
carrier, self._XTRACEOPTIONS_HEADER_NAME
Expand All @@ -67,9 +69,12 @@ def inject(
context: Context | None = None,
setter: textmap.Setter = textmap.default_setter,
) -> None:
"""Injects valid sw tracestate from carrier into carrier for HTTP request, to get
tracestate injected by previous propagators. Excludes any xtraceoptions_response
if in tracestate."""
"""Injects traceparent, tracestate with valid sw into carrier
for HTTP request. Excludes any xtraceoptions_response if in
tracestate.
"""
super().inject(carrier, context, setter)

span = trace.get_current_span(context)
span_context = span.get_span_context()
sw_value = W3CTransformer.sw_from_context(span_context)
Expand Down Expand Up @@ -126,4 +131,4 @@ def fields(
self,
) -> typing.Set[str]: # pylint: disable=deprecated-typing-alias
"""Returns a set with the fields set in `inject`"""
return {self._TRACESTATE_HEADER_NAME}
return super().fields
6 changes: 3 additions & 3 deletions tests/integration/test_base_sw_headers_attrs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def setUp(self):

# Load Distro
SolarWindsDistro().configure()
assert os.environ["OTEL_PROPAGATORS"] == "tracecontext,baggage,solarwinds_propagator"
assert os.environ["OTEL_PROPAGATORS"] == "solarwinds_propagator,baggage"

# Load Configurator to Configure SW custom SDK components
# except use TestBase InMemorySpanExporter
Expand Down Expand Up @@ -129,8 +129,8 @@ def setUp(self):

# Make sure SW SDK components were set
propagators = get_global_textmap()._propagators
assert len(propagators) == 3
assert isinstance(propagators[2], SolarWindsPropagator)
assert len(propagators) == 2
assert isinstance(propagators[0], SolarWindsPropagator)
assert isinstance(trace_api.get_tracer_provider().sampler, ParentBased)

# We need to instrument and create test app for every test
Expand Down
49 changes: 18 additions & 31 deletions tests/unit/test_apm_config/test_apm_config_agent_enabled.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
#
# 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
import os
import pytest

from solarwinds_apm import apm_config

Expand All @@ -18,6 +20,12 @@
# pylint: disable=unused-import
from .fixtures.env_vars import fixture_mock_env_vars


@pytest.fixture
def setup_caplog():
apm_logger = logging.getLogger("solarwinds_apm")
apm_logger.propagate = True

class TestSolarWindsApmConfigAgentEnabled:
def test_calculate_agent_enabled_service_key_missing(self, mocker):
# Save any service key in os for later
Expand Down Expand Up @@ -351,7 +359,7 @@ def test_calculate_agent_enabled_ok_all_env_vars(
mock_env_vars,
):
mocker.patch.dict(os.environ, {
"OTEL_PROPAGATORS": "foo,tracecontext,bar,solarwinds_propagator",
"OTEL_PROPAGATORS": "foo,solarwinds_propagator,bar",
"SW_APM_SERVICE_KEY": "valid:key",
"SW_APM_AGENT_ENABLED": "true",
})
Expand Down Expand Up @@ -401,10 +409,10 @@ def test_calculate_agent_enabled_no_tracecontext_propagator(self, mocker):
}
)
resulting_config = apm_config.SolarWindsApmConfig()
assert not resulting_config._calculate_agent_enabled()
assert resulting_config.service_name == ""
assert resulting_config._calculate_agent_enabled()
assert resulting_config.service_name == "key"

def test_calculate_agent_enabled_sw_before_tracecontext_propagator(self, mocker):
def test_calculate_agent_enabled_sw_with_tracecontext_propagator(self, caplog, mocker):
mocker.patch.dict(os.environ, {
"OTEL_PROPAGATORS": "solarwinds_propagator,tracecontext",
"SW_APM_SERVICE_KEY": "valid:key",
Expand All @@ -421,10 +429,11 @@ def test_calculate_agent_enabled_sw_before_tracecontext_propagator(self, mocker)
resulting_config = apm_config.SolarWindsApmConfig()
assert not resulting_config._calculate_agent_enabled()
assert resulting_config.service_name == ""
assert "It is unnecessary to configure tracecontext in OTEL_PROPAGATORS when using SolarWinds APM >= 4.4.0, which has built-in w3c context propagation" in caplog.text

def test_calculate_agent_enabled_sw_after_tracecontext_propagator(self, mocker):
def test_calculate_agent_enabled_sw_and_baggage_propagator(self, mocker):
mocker.patch.dict(os.environ, {
"OTEL_PROPAGATORS": "tracecontext,solarwinds_propagator",
"OTEL_PROPAGATORS": "solarwinds_propagator,baggage",
"SW_APM_SERVICE_KEY": "valid:key",
})
mock_apm_logging = mocker.patch(
Expand All @@ -440,31 +449,9 @@ def test_calculate_agent_enabled_sw_after_tracecontext_propagator(self, mocker):
assert resulting_config._calculate_agent_enabled()
assert resulting_config.service_name == "key"

def test_calculate_agent_enabled_sw_before_baggage_propagator(self, mocker):
def test_calculate_agent_enabled_baggage_and_sw_propagator(self, mocker):
mocker.patch.dict(os.environ, {
"OTEL_PROPAGATORS": "tracecontext,solarwinds_propagator,baggage",
"SW_APM_SERVICE_KEY": "valid:key",
})
mock_apm_logging = mocker.patch(
"solarwinds_apm.apm_config.apm_logging"
)
mock_apm_logging.configure_mock(
**{
"set_sw_log_level": mocker.Mock(),
"ApmLoggingLevel.default_level": mocker.Mock(return_value=2)
}
)
resulting_config = apm_config.SolarWindsApmConfig()
assert not resulting_config._calculate_agent_enabled()
assert resulting_config.service_name == ""

def test_calculate_agent_enabled_sw_after_baggage_propagator(
self,
mocker,
mock_env_vars,
):
mocker.patch.dict(os.environ, {
"OTEL_PROPAGATORS": "tracecontext,baggage,solarwinds_propagator",
"OTEL_PROPAGATORS": "baggage,solarwinds_propagator",
"SW_APM_SERVICE_KEY": "valid:key",
})
mock_apm_logging = mocker.patch(
Expand All @@ -478,4 +465,4 @@ def test_calculate_agent_enabled_sw_after_baggage_propagator(
)
resulting_config = apm_config.SolarWindsApmConfig()
assert resulting_config._calculate_agent_enabled()
assert resulting_config.service_name == "key"
assert resulting_config.service_name == "key"
130 changes: 125 additions & 5 deletions tests/unit/test_configurator/test_configurator_propagators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,12 @@ def test_configure_propagator_none_uses_default(
[
mocker.call(
group="opentelemetry_propagator",
name="tracecontext",
name="solarwinds_propagator",
),
mocker.call(
group="opentelemetry_propagator",
name="baggage"
),
mocker.call(
group="opentelemetry_propagator",
name="solarwinds_propagator",
),
]
)
mock_composite_propagator.assert_called_once()
Expand Down Expand Up @@ -153,6 +149,130 @@ def test_configure_propagator_valid(
if old_propagators:
os.environ["OTEL_PROPAGATORS"] = old_propagators

def test_configure_propagator_valid_baggage_sw(
self,
mocker,
mock_composite_propagator,
mock_set_global_textmap,
):
# Save any PROPAGATOR env var for later
old_propagators = os.environ.get("OTEL_PROPAGATORS", None)
if old_propagators:
del os.environ["OTEL_PROPAGATORS"]

# Mock entry points
mock_propagator_class = mocker.MagicMock()
mock_propagator_entry_point = mocker.Mock()
mock_propagator_entry_point.configure_mock(
**{
"load": mock_propagator_class
}
)
mock_points = iter(
[
mock_propagator_entry_point,
mock_propagator_entry_point,
mock_propagator_entry_point,
]
)
mock_entry_points = mocker.patch(
"solarwinds_apm.configurator.entry_points"
)
mock_entry_points.configure_mock(
return_value=mock_points
)

mocker.patch.dict(
os.environ,
{
"OTEL_PROPAGATORS": "baggage,solarwinds_propagator"
}
)

# Test!
test_configurator = configurator.SolarWindsConfigurator()
test_configurator._configure_propagator()
mock_entry_points.assert_has_calls(
[
mocker.call(
group="opentelemetry_propagator",
name="baggage"
),
mocker.call(
group="opentelemetry_propagator",
name="solarwinds_propagator"
),
]
)
mock_composite_propagator.assert_called_once()
mock_set_global_textmap.assert_called_once()

# Restore old PROPAGATOR
if old_propagators:
os.environ["OTEL_PROPAGATORS"] = old_propagators

def test_configure_propagator_valid_sw_baggage(
self,
mocker,
mock_composite_propagator,
mock_set_global_textmap,
):
# Save any PROPAGATOR env var for later
old_propagators = os.environ.get("OTEL_PROPAGATORS", None)
if old_propagators:
del os.environ["OTEL_PROPAGATORS"]

# Mock entry points
mock_propagator_class = mocker.MagicMock()
mock_propagator_entry_point = mocker.Mock()
mock_propagator_entry_point.configure_mock(
**{
"load": mock_propagator_class
}
)
mock_points = iter(
[
mock_propagator_entry_point,
mock_propagator_entry_point,
mock_propagator_entry_point,
]
)
mock_entry_points = mocker.patch(
"solarwinds_apm.configurator.entry_points"
)
mock_entry_points.configure_mock(
return_value=mock_points
)

mocker.patch.dict(
os.environ,
{
"OTEL_PROPAGATORS": "solarwinds_propagator,baggage"
}
)

# Test!
test_configurator = configurator.SolarWindsConfigurator()
test_configurator._configure_propagator()
mock_entry_points.assert_has_calls(
[
mocker.call(
group="opentelemetry_propagator",
name="solarwinds_propagator"
),
mocker.call(
group="opentelemetry_propagator",
name="baggage"
),
]
)
mock_composite_propagator.assert_called_once()
mock_set_global_textmap.assert_called_once()

# Restore old PROPAGATOR
if old_propagators:
os.environ["OTEL_PROPAGATORS"] = old_propagators

def test_configure_propagator_valid_invalid_mixed(
self,
mocker,
Expand Down
Loading
Loading