botocore: always use x-ray for http header injection

AWS will only propagate the x-ray header's context through to
other services. Injecting context to any other header with a
different propagator will result in the context being dropped.

The propagator is overridable by the user as an argument to
BotocoreInstrumentor to give them final control.

Author: tsloughter

CHANGELOG.md

@@ -14,6 +14,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Make Django request span attributes available for `start_span`. 
   ([#1730](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1730))
 
+### Changed
+
+- `opentelemetry-instrumentation-botocore` now uses the AWS X-Ray propagator by
+  default
+  ([#1741](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1741))
 
 ## Version 1.17.0/0.38b0 (2023-03-22)
 

instrumentation/opentelemetry-instrumentation-botocore/pyproject.toml

@@ -28,6 +28,7 @@ dependencies = [
   "opentelemetry-api ~= 1.12",
   "opentelemetry-instrumentation == 0.39b0.dev",
   "opentelemetry-semantic-conventions == 0.39b0.dev",
+  "opentelemetry-propagator-aws-xray == 1.0.1",
 ]
 
 [project.optional-dependencies]

instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py

@@ -101,22 +101,16 @@ def response_hook(span, service_name, operation_name, result):
     _SUPPRESS_INSTRUMENTATION_KEY,
     unwrap,
 )
-from opentelemetry.propagate import inject
+from opentelemetry.propagators.aws.aws_xray_propagator import (
+    AwsXRayPropagator,
+)
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.trace import get_tracer
 from opentelemetry.trace.span import Span
 
 logger = logging.getLogger(__name__)
 
 
-# pylint: disable=unused-argument
-def _patched_endpoint_prepare_request(wrapped, instance, args, kwargs):
-    request = args[0]
-    headers = request.headers
-    inject(headers)
-    return wrapped(*args, **kwargs)
-
-
 class BotocoreInstrumentor(BaseInstrumentor):
     """An instrumentor for Botocore.
 
@@ -127,6 +121,7 @@ def __init__(self):
         super().__init__()
         self.request_hook = None
         self.response_hook = None
+        self.propagator = AwsXRayPropagator()
 
     def instrumentation_dependencies(self) -> Collection[str]:
         return _instruments
@@ -140,6 +135,10 @@ def _instrument(self, **kwargs):
         self.request_hook = kwargs.get("request_hook")
         self.response_hook = kwargs.get("response_hook")
 
+        propagator = kwargs.get("propagator")
+        if propagator != None:
+            self.propagator = propagator
+
         wrap_function_wrapper(
             "botocore.client",
             "BaseClient._make_api_call",
@@ -149,13 +148,26 @@ def _instrument(self, **kwargs):
         wrap_function_wrapper(
             "botocore.endpoint",
             "Endpoint.prepare_request",
-            _patched_endpoint_prepare_request,
+            self._patched_endpoint_prepare_request,
         )
 
     def _uninstrument(self, **kwargs):
         unwrap(BaseClient, "_make_api_call")
         unwrap(Endpoint, "prepare_request")
 
+    # pylint: disable=unused-argument
+    def _patched_endpoint_prepare_request(
+        self, wrapped, instance, args, kwargs
+    ):
+        request = args[0]
+        headers = request.headers
+
+        # Only the x-ray header is propagated by AWS services. Using any
+        # other propagator will lose the trace context.
+        self.propagator.inject(headers)
+
+        return wrapped(*args, **kwargs)
+
     # pylint: disable=too-many-branches
     def _patched_api_call(self, original_func, instance, args, kwargs):
         if context_api.get_value(_SUPPRESS_INSTRUMENTATION_KEY):

instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py

@@ -39,6 +39,10 @@
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.test.mock_textmap import MockTextMapPropagator
 from opentelemetry.test.test_base import TestBase
+from opentelemetry.trace.span import Span, format_span_id, format_trace_id
+from opentelemetry.propagators.aws.aws_xray_propagator import (
+    TRACE_HEADER_KEY,
+)
 
 _REQUEST_ID_REGEX_MATCH = r"[A-Z0-9]{52}"
 
@@ -225,27 +229,21 @@ def test_unpatch(self):
     @mock_ec2
     def test_uninstrument_does_not_inject_headers(self):
         headers = {}
-        previous_propagator = get_global_textmap()
-        try:
-            set_global_textmap(MockTextMapPropagator())
 
-            def intercept_headers(**kwargs):
-                headers.update(kwargs["request"].headers)
+        def intercept_headers(**kwargs):
+            headers.update(kwargs["request"].headers)
 
-            ec2 = self._make_client("ec2")
+        ec2 = self._make_client("ec2")
 
-            BotocoreInstrumentor().uninstrument()
+        BotocoreInstrumentor().uninstrument()
 
-            ec2.meta.events.register_first(
-                "before-send.ec2.DescribeInstances", intercept_headers
-            )
-            with self.tracer_provider.get_tracer("test").start_span("parent"):
-                ec2.describe_instances()
+        ec2.meta.events.register_first(
+            "before-send.ec2.DescribeInstances", intercept_headers
+        )
+        with self.tracer_provider.get_tracer("test").start_span("parent"):
+            ec2.describe_instances()
 
-            self.assertNotIn(MockTextMapPropagator.TRACE_ID_KEY, headers)
-            self.assertNotIn(MockTextMapPropagator.SPAN_ID_KEY, headers)
-        finally:
-            set_global_textmap(previous_propagator)
+        self.assertNotIn(TRACE_HEADER_KEY, headers)
 
     @mock_sqs
     def test_double_patch(self):
@@ -306,20 +304,47 @@ def check_headers(**kwargs):
                 "EC2", "DescribeInstances", request_id=request_id
             )
 
-            self.assertIn(MockTextMapPropagator.TRACE_ID_KEY, headers)
-            self.assertEqual(
-                str(span.get_span_context().trace_id),
-                headers[MockTextMapPropagator.TRACE_ID_KEY],
+            # only x-ray propagation is used in HTTP requests
+            self.assertIn(TRACE_HEADER_KEY, headers)
+            xray_context = headers[TRACE_HEADER_KEY]
+            formated_trace_id = format_trace_id(
+                span.get_span_context().trace_id
             )
-            self.assertIn(MockTextMapPropagator.SPAN_ID_KEY, headers)
-            self.assertEqual(
-                str(span.get_span_context().span_id),
-                headers[MockTextMapPropagator.SPAN_ID_KEY],
+            formated_trace_id = (
+                formated_trace_id[:8] + "-" + formated_trace_id[8:]
             )
 
+            self.assertEqual(
+                xray_context.lower(),
+                f"root=1-{formated_trace_id};parent={format_span_id(span.get_span_context().span_id)};sampled=1".lower(),
+            )
         finally:
             set_global_textmap(previous_propagator)
 
+    @mock_ec2
+    def test_override_xray_propagator_injects_into_request(self):
+        headers = {}
+
+        def check_headers(**kwargs):
+            nonlocal headers
+            headers = kwargs["request"].headers
+
+        BotocoreInstrumentor().instrument()
+
+        ec2 = self._make_client("ec2")
+        ec2.meta.events.register_first(
+            "before-send.ec2.DescribeInstances", check_headers
+        )
+        ec2.describe_instances()
+
+        request_id = "fdcdcab1-ae5c-489e-9c33-4637c5dda355"
+        span = self.assert_span(
+            "EC2", "DescribeInstances", request_id=request_id
+        )
+
+        self.assertNotIn(MockTextMapPropagator.TRACE_ID_KEY, headers)
+        self.assertNotIn(MockTextMapPropagator.SPAN_ID_KEY, headers)
+
     @mock_xray
     def test_suppress_instrumentation_xray_client(self):
         xray_client = self._make_client("xray")