Merge pull request #136 from solarwindscloud/NH-34752-txn-filters-and…

…-tracing-mode

NH-34752 Parse txn filters and calculate tracing mode

solarwinds_apm/apm_config.py

@@ -7,6 +7,7 @@
 import json
 import logging
 import os
+import re
 import sys
 from collections import defaultdict
 from functools import reduce
@@ -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.
@@ -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
@@ -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"],
@@ -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

solarwinds_apm/sampler.py

@@ -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."""
@@ -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")
@@ -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

tests/unit/test_apm_config.py

@@ -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

tests/unit/test_sampler/fixtures/sampler.py

@@ -2,17 +2,22 @@
 
 from solarwinds_apm.sampler import _SwSampler
 
+def side_effect_fn(param):
+    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)

tests/unit/test_sampler/test_sampler.py

@@ -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(
@@ -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",
@@ -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(