Merge pull request #88 from appoptics/NH-24786-fix-xtraceoptions-resp…

…onse-calc NH-24786 fix xtraceoptions response calc
solarwinds · Dec 14, 2022 · 3b4a68c · 3b4a68c
2 parents 7f1e890 + 01efa30
commit 3b4a68c
Show file tree

Hide file tree

Showing 16 changed files with 1,163 additions and 213 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - x-trace-options header `custom-*` KVs written to entry span attributes ([#85](https://github.com/appoptics/solarwinds-apm-python/pull/85))
 - Fix `x-trace-options-signature` extraction ([#85](https://github.com/appoptics/solarwinds-apm-python/pull/85))
 - Fix validation of `x-trace-options` header ([#87](https://github.com/appoptics/solarwinds-apm-python/pull/87))
+- Fix calculation of `x-trace-options-response` header ([#88](https://github.com/appoptics/solarwinds-apm-python/pull/88))
 
 ## [0.3.0](https://github.com/appoptics/solarwinds-apm-python/releases/tag/rel-0.3.0) - 2022-11-24
 ### Changed

diff --git a/dev-requirements.txt b/dev-requirements.txt
@@ -1,6 +1,6 @@
-opentelemetry-test-utils==0.36b0
-opentelemetry-instrumentation-flask==0.36b0
-opentelemetry-instrumentation-requests==0.36b0
+opentelemetry-test-utils==0.35b0
+opentelemetry-instrumentation-flask==0.35b0
+opentelemetry-instrumentation-requests==0.35b0
 pytest
 pytest-cov
 pytest-mock

diff --git a/solarwinds_apm/apm_constants.py b/solarwinds_apm/apm_constants.py
@@ -6,6 +6,7 @@
 INTL_SWO_EQUALS_W3C_SANITIZED = "####"
 INTL_SWO_TRACESTATE_KEY = "sw"
 INTL_SWO_X_OPTIONS_KEY = "sw_xtraceoptions"
+INTL_SWO_X_OPTIONS_RESPONSE_KEY = "xtrace_options_response"
 INTL_SWO_SIGNATURE_KEY = "sw_signature"
 INTL_SWO_DEFAULT_TRACES_EXPORTER = "solarwinds_exporter"
 INTL_SWO_TRACECONTEXT_PROPAGATOR = "tracecontext"

diff --git a/solarwinds_apm/propagator.py b/solarwinds_apm/propagator.py
@@ -70,7 +70,8 @@ def inject(
         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"""
+        tracestate injected by previous propagators. Excludes any xtraceoptions_response
+        if in tracestate."""
         span = trace.get_current_span(context)
         span_context = span.get_span_context()
         sw_value = W3CTransformer.sw_from_context(span_context)
@@ -110,6 +111,9 @@ def inject(
                     INTL_SWO_TRACESTATE_KEY, sw_value
                 )
 
+        # Remove any xtrace_options_response stored for ResponsePropagator
+        trace_state = W3CTransformer.remove_response_from_sw(trace_state)
+
         setter.set(
             carrier, self._TRACESTATE_HEADER_NAME, trace_state.to_header()
         )

diff --git a/solarwinds_apm/response_propagator.py b/solarwinds_apm/response_propagator.py
@@ -12,8 +12,8 @@
     INTL_SWO_COMMA_W3C_SANITIZED,
     INTL_SWO_EQUALS,
     INTL_SWO_EQUALS_W3C_SANITIZED,
+    INTL_SWO_X_OPTIONS_RESPONSE_KEY,
 )
-from solarwinds_apm.traceoptions import XTraceOptions
 from solarwinds_apm.w3c_transformer import W3CTransformer
 
 logger = logging.getLogger(__name__)
@@ -72,9 +72,7 @@ def recover_response_from_tracestate(self, tracestate: TraceState) -> str:
         EQUALS_W3C_SANITIZED becomes EQUALS
         COMMA_W3C_SANITIZED becomes COMMA
         """
-        sanitized = tracestate.get(
-            XTraceOptions.get_sw_xtraceoptions_response_key(), None
-        )
+        sanitized = tracestate.get(INTL_SWO_X_OPTIONS_RESPONSE_KEY, None)
         if not sanitized:
             return ""
         return sanitized.replace(

diff --git a/solarwinds_apm/sampler.py b/solarwinds_apm/sampler.py
@@ -24,6 +24,7 @@
     INTL_SWO_COMMA_W3C_SANITIZED,
     INTL_SWO_EQUALS_W3C_SANITIZED,
     INTL_SWO_TRACESTATE_KEY,
+    INTL_SWO_X_OPTIONS_RESPONSE_KEY,
 )
 from solarwinds_apm.apm_noop import Context as NoopContext
 from solarwinds_apm.extension.oboe import Context
@@ -207,6 +208,7 @@ def create_xtraceoptions_response_value(
                 )
             )
 
+        # Include other trace options if valid signature or no signature
         if not decision["auth"] or decision["auth"] < 1:
             trigger_msg = ""
             if xtraceoptions.trigger_trace:
@@ -232,19 +234,19 @@ def create_xtraceoptions_response_value(
                 )
             )
 
-        if xtraceoptions.ignored:
-            response.append(
-                INTL_SWO_EQUALS_W3C_SANITIZED.join(
-                    [
-                        self._XTRACEOPTIONS_RESP_IGNORED,
-                        (
-                            INTL_SWO_COMMA_W3C_SANITIZED.join(
-                                xtraceoptions.ignored
-                            )
-                        ),
-                    ]
+            if xtraceoptions.ignored:
+                response.append(
+                    INTL_SWO_EQUALS_W3C_SANITIZED.join(
+                        [
+                            self._XTRACEOPTIONS_RESP_IGNORED,
+                            (
+                                INTL_SWO_COMMA_W3C_SANITIZED.join(
+                                    xtraceoptions.ignored
+                                )
+                            ),
+                        ]
+                    )
                 )
-            )
 
         return ";".join(response)
 
@@ -269,9 +271,9 @@ def create_new_trace_state(
                 )
             ]
         )
-        if xtraceoptions and xtraceoptions.trigger_trace:
+        if xtraceoptions and xtraceoptions.options_header:
             trace_state = trace_state.add(
-                XTraceOptions.get_sw_xtraceoptions_response_key(),
+                INTL_SWO_X_OPTIONS_RESPONSE_KEY,
                 self.create_xtraceoptions_response_value(
                     decision, parent_span_context, xtraceoptions
                 ),
@@ -313,22 +315,16 @@ def calculate_trace_state(
                 )
                 # Update trace_state with x-trace-options-response
                 # Not a propagated header, so always an add
-                if xtraceoptions and xtraceoptions.trigger_trace:
+                if xtraceoptions and xtraceoptions.options_header:
                     trace_state = trace_state.add(
-                        XTraceOptions.get_sw_xtraceoptions_response_key(),
+                        INTL_SWO_X_OPTIONS_RESPONSE_KEY,
                         self.create_xtraceoptions_response_value(
                             decision, parent_span_context, xtraceoptions
                         ),
                     )
                 logger.debug("Updated trace_state: %s", trace_state)
         return trace_state
 
-    def remove_response_from_sw(self, trace_state: TraceState) -> TraceState:
-        """Remove xtraceoptions response from tracestate"""
-        return trace_state.delete(
-            XTraceOptions.get_sw_xtraceoptions_response_key()
-        )
-
     def add_tracestate_capture_to_attributes_dict(
         self,
         attributes_dict: dict,
@@ -342,7 +338,9 @@ def add_tracestate_capture_to_attributes_dict(
             self._SW_TRACESTATE_CAPTURE_KEY, None
         )
         if not tracestate_capture:
-            trace_state_no_response = self.remove_response_from_sw(trace_state)
+            trace_state_no_response = W3CTransformer.remove_response_from_sw(
+                trace_state
+            )
         else:
             # Must retain all potential tracestate pairs for attributes
             attr_trace_state = TraceState.from_header([tracestate_capture])
@@ -353,7 +351,7 @@ def add_tracestate_capture_to_attributes_dict(
                     W3CTransformer.trace_flags_from_int(decision["do_sample"]),
                 ),
             )
-            trace_state_no_response = self.remove_response_from_sw(
+            trace_state_no_response = W3CTransformer.remove_response_from_sw(
                 new_attr_trace_state
             )
         attributes_dict[

diff --git a/solarwinds_apm/traceoptions.py b/solarwinds_apm/traceoptions.py
@@ -15,7 +15,6 @@
 class XTraceOptions:
     """Formats X-Trace-Options and signature for trigger tracing"""
 
-    _SW_XTRACEOPTIONS_RESPONSE_KEY = "xtrace_options_response"
     _XTRACEOPTIONS_CUSTOM = r"^custom-[^\s]*$"
     _XTRACEOPTIONS_CUSTOM_RE = re.compile(_XTRACEOPTIONS_CUSTOM)
 
@@ -116,7 +115,3 @@ def __init__(
         options_signature = context.get(INTL_SWO_SIGNATURE_KEY, None)
         if options_signature:
             self.signature = options_signature
-
-    @classmethod
-    def get_sw_xtraceoptions_response_key(cls) -> str:
-        return cls._SW_XTRACEOPTIONS_RESPONSE_KEY
diff --git a/solarwinds_apm/w3c_transformer.py b/solarwinds_apm/w3c_transformer.py
@@ -4,6 +4,9 @@
 import logging
 
 from opentelemetry.sdk.trace import SpanContext
+from opentelemetry.trace.span import TraceState
+
+from solarwinds_apm.apm_constants import INTL_SWO_X_OPTIONS_RESPONSE_KEY
 
 logger = logging.getLogger(__name__)
 
@@ -73,3 +76,8 @@ def sw_from_span_and_decision(cls, span_id: int, decision: str) -> str:
         Example: 1a2b3c4d5e6f7g8h-01"""
         sw = "-".join([cls._SPAN_ID_HEX, cls._DECISION])
         return sw.format(span_id, decision)
+
+    @classmethod
+    def remove_response_from_sw(cls, trace_state: TraceState) -> TraceState:
+        """Remove xtraceoptions response from tracestate"""
+        return trace_state.delete(INTL_SWO_X_OPTIONS_RESPONSE_KEY)