From 927d70ce840b8a62f592b320531b4c6a0f012d79 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Fri, 17 Feb 2023 12:04:35 -0700 Subject: [PATCH 1/8] 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. --- CHANGELOG.md | 5 ++ .../pyproject.toml | 1 + .../instrumentation/botocore/__init__.py | 32 +++++--- .../tests/test_botocore_instrumentation.py | 73 +++++++++++++------ 4 files changed, 77 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 885bf2bec9..b84e4b205f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-logging` Add `otelTraceSampled` to instrumetation-logging ([#1773](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1773)) +### Changed + +- `opentelemetry-instrumentation-botocore` now uses the AWS X-Ray propagator by + default + ([#1741](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1741)) ### Fixed diff --git a/instrumentation/opentelemetry-instrumentation-botocore/pyproject.toml b/instrumentation/opentelemetry-instrumentation-botocore/pyproject.toml index 6125bcfa60..3f2454a742 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/pyproject.toml +++ b/instrumentation/opentelemetry-instrumentation-botocore/pyproject.toml @@ -28,6 +28,7 @@ dependencies = [ "opentelemetry-api ~= 1.12", "opentelemetry-instrumentation == 0.40b0.dev", "opentelemetry-semantic-conventions == 0.40b0.dev", + "opentelemetry-propagator-aws-xray == 1.0.1", ] [project.optional-dependencies] diff --git a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py index 40a760d525..23402a7cbe 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py @@ -101,7 +101,9 @@ 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 @@ -109,14 +111,6 @@ def response_hook(span, service_name, operation_name, result): 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): diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py index 9a5d8429b5..63f7316b4e 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py +++ b/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") From 349ca3885c60081df602ce9b9f59a199875ec68b Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Fri, 31 Mar 2023 16:59:26 -0600 Subject: [PATCH 2/8] Update instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py Co-authored-by: Diego Hurtado --- .../src/opentelemetry/instrumentation/botocore/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py index 23402a7cbe..7190e8eaf4 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py @@ -136,7 +136,7 @@ def _instrument(self, **kwargs): self.response_hook = kwargs.get("response_hook") propagator = kwargs.get("propagator") - if propagator != None: + if propagator is not None: self.propagator = propagator wrap_function_wrapper( From c11fe33ff2732000966ea99ecea0cafdc8b35f82 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Fri, 31 Mar 2023 17:02:48 -0600 Subject: [PATCH 3/8] Update instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py Co-authored-by: Diego Hurtado --- .../src/opentelemetry/instrumentation/botocore/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py index 7190e8eaf4..eb653854a4 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py @@ -157,7 +157,7 @@ def _uninstrument(self, **kwargs): # pylint: disable=unused-argument def _patched_endpoint_prepare_request( - self, wrapped, instance, args, kwargs + self, wrapped, instance, *args, **kwargs ): request = args[0] headers = request.headers From 7f0f4425caa37e2dfc7c9c196fc2551cc8530b42 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Fri, 12 May 2023 04:48:32 -0600 Subject: [PATCH 4/8] patched_endpoint_prepare_request takes non-variable length args --- .../src/opentelemetry/instrumentation/botocore/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py index eb653854a4..7190e8eaf4 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py @@ -157,7 +157,7 @@ def _uninstrument(self, **kwargs): # pylint: disable=unused-argument def _patched_endpoint_prepare_request( - self, wrapped, instance, *args, **kwargs + self, wrapped, instance, args, kwargs ): request = args[0] headers = request.headers From 10b444e292535e65f3b63b3d4b2c189b58fec8cc Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Fri, 12 May 2023 10:42:38 -0600 Subject: [PATCH 5/8] resolve linting issues with import formatting --- .../src/opentelemetry/instrumentation/botocore/__init__.py | 4 +--- .../tests/test_botocore_instrumentation.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py index 7190e8eaf4..baf8a56e71 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py @@ -101,9 +101,7 @@ def response_hook(span, service_name, operation_name, result): _SUPPRESS_INSTRUMENTATION_KEY, unwrap, ) -from opentelemetry.propagators.aws.aws_xray_propagator import ( - AwsXRayPropagator, -) +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 diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py index 63f7316b4e..0a8fa93b0e 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py @@ -36,13 +36,11 @@ from opentelemetry.instrumentation.botocore import BotocoreInstrumentor from opentelemetry.instrumentation.utils import _SUPPRESS_INSTRUMENTATION_KEY from opentelemetry.propagate import get_global_textmap, set_global_textmap +from opentelemetry.propagators.aws.aws_xray_propagator import TRACE_HEADER_KEY 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}" From fd6a0be458406bf22e72cabe3499f83b3925ba29 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Fri, 12 May 2023 11:20:51 -0600 Subject: [PATCH 6/8] fix linting issues found by flake8 in test_botocore_instrumentation --- .../tests/test_botocore_instrumentation.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py index 0a8fa93b0e..8e8f7e34ee 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py @@ -40,7 +40,7 @@ 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.trace.span import format_span_id, format_trace_id _REQUEST_ID_REGEX_MATCH = r"[A-Z0-9]{52}" @@ -336,9 +336,6 @@ def check_headers(**kwargs): 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) From 87db1cbf50574c13ed65ff265d728db198cd60e6 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Sat, 13 May 2023 04:23:41 -0600 Subject: [PATCH 7/8] Update instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py Co-authored-by: Diego Hurtado --- .../tests/test_botocore_instrumentation.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py index 8e8f7e34ee..ce302a6394 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py @@ -335,7 +335,6 @@ def check_headers(**kwargs): ) ec2.describe_instances() - request_id = "fdcdcab1-ae5c-489e-9c33-4637c5dda355" self.assertNotIn(MockTextMapPropagator.TRACE_ID_KEY, headers) self.assertNotIn(MockTextMapPropagator.SPAN_ID_KEY, headers) From 44c3d2b14ac1954820ef9239f7631580f30ab284 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Mon, 15 May 2023 14:58:11 -0600 Subject: [PATCH 8/8] remove unused variable from botocore test_override_xray_propagator_injects_into_request --- .../tests/test_botocore_instrumentation.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py index ce302a6394..3d25dcbf2d 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py @@ -335,7 +335,6 @@ def check_headers(**kwargs): ) ec2.describe_instances() - self.assertNotIn(MockTextMapPropagator.TRACE_ID_KEY, headers) self.assertNotIn(MockTextMapPropagator.SPAN_ID_KEY, headers)