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
35 changes: 32 additions & 3 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 @@ -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,35 @@ 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

try:
re.compile(filter["regex"])
Copy link
Member

@raphael-theriault-swi raphael-theriault-swi Apr 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're already compiling the regex in here wouldn't it make sense to store the result in txn_filter so it doesn't have to get recompiled on every match

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! Changed in d28c040.

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

logger.debug(
"Set up transaction filters: %s",
self.__config["transaction_filters"],
Expand Down
48 changes: 45 additions & 3 deletions solarwinds_apm/sampler.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import enum
import logging
import re
from types import MappingProxyType
from typing import TYPE_CHECKING, Optional, Sequence

Expand Down Expand Up @@ -67,18 +68,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 re.search(txn_filter.get("regex"), 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 +130,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 +528,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
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