Skip to content
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.

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

NH-34752 Parse txn filters and calculate tracing mode #136

Merged
merged 12 commits into from
Apr 25, 2023
48 changes: 39 additions & 9 deletions solarwinds_apm/apm_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import json
import logging
import os
import re
import sys
from collections import defaultdict
from functools import reduce
Expand Down Expand Up @@ -88,8 +89,8 @@ def __init__(
self.__config = {}
# Update the config with default values
self.__config = {
# 'tracing_mode' is unset by default and not supported in NH Python
"tracing_mode": None,
# 'tracing_mode' is unset by default
"tracing_mode": OboeTracingMode.get_oboe_trace_mode("unset"),
# 'trigger_trace' is enabled by default
"trigger_trace": "enabled",
"collector": "", # the collector address in host:port format.
Expand Down Expand Up @@ -532,15 +533,15 @@ def update_transaction_filters(self, cnf_dict: dict) -> None:
"""Update configured transaction_filters using config dict"""
txn_settings = cnf_dict.get("transactionSettings")
if not txn_settings or not isinstance(txn_settings, list):
logger.error(
logger.warning(
"Transaction filters must be a non-empty list of filters. Ignoring."
)
return
for filter in txn_settings:
if set(filter) != set(["regex", "tracing"]) or filter[
"tracing"
] not in ["enabled", "disabled"]:
logger.error(
logger.warning(
"Invalid transaction filter rule. Ignoring: %s", filter
)
continue
Expand All @@ -549,7 +550,37 @@ def update_transaction_filters(self, cnf_dict: dict) -> None:
cfilter["regex"]
for cfilter in self.__config["transaction_filters"]
]:
self.__config["transaction_filters"].append(filter)
txn_filter = {}
txn_filter[
"tracing_mode"
] = OboeTracingMode.get_oboe_trace_mode(filter["tracing"])

if not isinstance(filter["regex"], str):
logger.warning(
"Transaction filter regex must be string or regex. Ignoring: %s",
filter,
)
continue

if not len(filter["regex"]) > 0:
logger.warning(
"Transaction filter regex must not be empty. Ignoring: %s",
filter,
)
continue

txn_filter_re = None
try:
txn_filter_re = re.compile(filter["regex"])
except re.error:
logger.warning(
"Transaction filter regex invalid. Ignoring: %s",
filter,
)
continue
txn_filter["regex"] = txn_filter_re
self.__config["transaction_filters"].append(txn_filter)

logger.debug(
"Set up transaction filters: %s",
self.__config["transaction_filters"],
Expand Down Expand Up @@ -629,10 +660,9 @@ def _convert_to_bool(val):
val = "enabled" if val == "always" else "disabled"
if val not in ["enabled", "disabled"]:
raise ValueError
self.__config[key] = val
self.context.setTracingMode(
OboeTracingMode.get_oboe_trace_mode(val)
)
oboe_trace_mode = OboeTracingMode.get_oboe_trace_mode(val)
self.__config[key] = oboe_trace_mode
self.context.setTracingMode(oboe_trace_mode)
elif keys == ["trigger_trace"]:
if not isinstance(val, str):
raise ValueError
Expand Down
47 changes: 44 additions & 3 deletions solarwinds_apm/sampler.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,56 @@ class _SwSampler(Sampler):
def __init__(self, apm_config: "SolarWindsApmConfig"):
self.apm_config = apm_config
self.context = None

if self.apm_config.agent_enabled:
self.context = Context
else:
self.context = NoopContext

if self.apm_config.get("tracing_mode") is not None:
self.tracing_mode = self.apm_config.get("tracing_mode")
else:
self.tracing_mode = self._UNSET

def get_description(self) -> str:
return "SolarWinds custom opentelemetry sampler"

def calculate_tracing_mode(
self,
name: str,
kind: SpanKind = None,
attributes: Attributes = None,
) -> int:
"""Calculates tracing mode per-request or global tracing mode, if set.
Can still be overridden by remote settings."""
# If future span matches txn filter, use filter's tracing mode
if self.apm_config.get("transaction_filters"):
for txn_filter in self.apm_config.get("transaction_filters"):
# TODO (NH-34752) Check `attributes` for http.* then filter web requests
# after OTel instrumentation library updates released
# https://github.com/open-telemetry/opentelemetry-python-contrib/issues/936

# Only matches span kind and name at this time
identifier = f"{kind.name}:{name}"
if txn_filter.get("regex").search(identifier):
logger.debug("Got a match for identifier %s", identifier)
logger.debug(
"Setting tracing_mode as %s",
txn_filter.get("tracing_mode"),
)
return txn_filter.get("tracing_mode")

# Else use global 'tracing_mode'
logger.debug("Using global tracing_mode as %s", self.tracing_mode)
return self.tracing_mode

# pylint: disable=too-many-locals
def calculate_liboboe_decision(
self,
parent_span_context: SpanContext,
name: str,
kind: SpanKind = None,
attributes: Attributes = None,
xtraceoptions: Optional[XTraceOptions] = None,
) -> dict:
"""Calculates oboe trace decision based on parent span context and APM config."""
Expand All @@ -91,8 +129,11 @@ def calculate_liboboe_decision(
INTL_SWO_TRACESTATE_KEY
)

# 'tracing_mode' is not supported in NH Python, so give as unset
tracing_mode = self._UNSET
tracing_mode = self.calculate_tracing_mode(
name,
kind,
attributes,
)

trigger_trace_mode = OboeTracingMode.get_oboe_trigger_trace_mode(
self.apm_config.get("trigger_trace")
Expand Down Expand Up @@ -486,7 +527,7 @@ def should_sample(
xtraceoptions = XTraceOptions(parent_context)

liboboe_decision = self.calculate_liboboe_decision(
parent_span_context, xtraceoptions
parent_span_context, name, kind, attributes, xtraceoptions
)

# Always calculate trace_state for propagation
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_apm_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ def test_set_config_value_default_proxy(self, caplog, mock_env_vars):
def test_set_config_value_default_tracing_mode(self, caplog, mock_env_vars):
test_config = apm_config.SolarWindsApmConfig()
test_config._set_config_value("tracing_mode", "not-valid-mode")
assert test_config.get("tracing_mode") == None
assert test_config.get("tracing_mode") == -1
assert "Ignore config option" in caplog.text

# pylint:disable=unused-argument
Expand Down
9 changes: 7 additions & 2 deletions tests/unit/test_sampler/fixtures/sampler.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,22 @@

from solarwinds_apm.sampler import _SwSampler

def side_effect_fn(param):

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns

Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
if param == "tracing_mode":
return -1
elif param == "transaction_filters":
return []

@pytest.fixture(name="sw_sampler")
def fixture_swsampler(mocker):
mock_apm_config = mocker.Mock()
mock_get = mocker.Mock(
return_value=1 # enabled
side_effect=side_effect_fn
)
mock_apm_config.configure_mock(
**{
"agent_enabled": True,
"get": mock_get,
"tracing_mode": None, # mapped to -1
}
)
return _SwSampler(mock_apm_config)
9 changes: 9 additions & 0 deletions tests/unit/test_sampler/test_sampler.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,9 @@ def test_calculate_liboboe_decision_root_span(
):
sw_sampler.calculate_liboboe_decision(
parent_span_context_invalid,
'foo',
None,
{'foo': 'bar'},
mock_xtraceoptions_signed_tt,
)
solarwinds_apm.extension.oboe.Context.getDecisions.assert_called_once_with(
Expand All @@ -219,6 +222,9 @@ def test_calculate_liboboe_decision_parent_valid_remote(
):
sw_sampler.calculate_liboboe_decision(
parent_span_context_valid_remote,
'foo',
None,
{'foo': 'bar'},
)
solarwinds_apm.extension.oboe.Context.getDecisions.assert_called_once_with(
"foo-bar",
Expand Down Expand Up @@ -574,6 +580,9 @@ def test_should_sample(

_SwSampler.calculate_liboboe_decision.assert_called_once_with(
"my_span_context",
'foo',
None,
{'foo': 'bar'},
mock_xtraceoptions
)
_SwSampler.calculate_trace_state.assert_called_once_with(
Expand Down