From 361da3e45e99cc42e571c6e3f9913d37e51da89d Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Fri, 14 Jun 2024 00:56:04 +0200 Subject: [PATCH 1/5] botocore: bump moto to latest (#2605) So we can bump Werkzeug too. --- .../test-requirements.txt | 6 +- .../tests/test_botocore_dynamodb.py | 48 +++++++++----- .../tests/test_botocore_instrumentation.py | 62 ++++++++----------- .../tests/test_botocore_lambda.py | 8 +-- .../tests/test_botocore_sns.py | 10 +-- .../tests/test_botocore_sqs.py | 10 +-- 6 files changed, 76 insertions(+), 68 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-botocore/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-botocore/test-requirements.txt index f52060f22e..c61d546e07 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/test-requirements.txt +++ b/instrumentation/opentelemetry-instrumentation-botocore/test-requirements.txt @@ -13,8 +13,8 @@ importlib-metadata==6.11.0 iniconfig==2.0.0 Jinja2==3.1.4 jmespath==1.0.1 -MarkupSafe==2.0.1 -moto==3.1.19 +MarkupSafe==2.1.5 +moto==5.0.9 packaging==24.0 pluggy==1.5.0 py-cpuinfo==9.0.0 @@ -31,7 +31,7 @@ six==1.16.0 tomli==2.0.1 typing_extensions==4.9.0 urllib3==1.26.18 -Werkzeug==2.1.2 +Werkzeug==3.0.3 wrapt==1.16.0 xmltodict==0.13.0 zipp==3.17.0 diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_dynamodb.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_dynamodb.py index 12ebe8f2b7..2240baff3a 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_dynamodb.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_dynamodb.py @@ -16,7 +16,7 @@ from unittest import mock import botocore.session -from moto import mock_dynamodb2 # pylint: disable=import-error +from moto import mock_aws # pylint: disable=import-error from opentelemetry.instrumentation.botocore import BotocoreInstrumentor from opentelemetry.instrumentation.botocore.extensions.dynamodb import ( @@ -184,7 +184,7 @@ def assert_extension_item_col_metrics(self, operation: str): ) self.assert_item_col_metrics(span) - @mock_dynamodb2 + @mock_aws def test_batch_get_item(self): table_name1 = "test_table1" table_name2 = "test_table2" @@ -203,7 +203,7 @@ def test_batch_get_item(self): self.assert_table_names(span, table_name1, table_name2) self.assert_consumed_capacity(span, table_name1, table_name2) - @mock_dynamodb2 + @mock_aws def test_batch_write_item(self): table_name1 = "test_table1" table_name2 = "test_table2" @@ -224,7 +224,7 @@ def test_batch_write_item(self): self.assert_consumed_capacity(span, table_name1, table_name2) self.assert_item_col_metrics(span) - @mock_dynamodb2 + @mock_aws def test_create_table(self): local_sec_idx = { "IndexName": "local_sec_idx", @@ -268,7 +268,7 @@ def test_create_table(self): ) self.assert_provisioned_read_cap(span, 42) - @mock_dynamodb2 + @mock_aws def test_delete_item(self): self._create_prepared_table() @@ -297,7 +297,7 @@ def test_delete_item_consumed_capacity(self): def test_delete_item_item_collection_metrics(self): self.assert_extension_item_col_metrics("DeleteItem") - @mock_dynamodb2 + @mock_aws def test_delete_table(self): self._create_prepared_table() @@ -306,7 +306,7 @@ def test_delete_table(self): span = self.assert_span("DeleteTable") self.assert_table_names(span, self.default_table_name) - @mock_dynamodb2 + @mock_aws def test_describe_table(self): self._create_prepared_table() @@ -315,15 +315,31 @@ def test_describe_table(self): span = self.assert_span("DescribeTable") self.assert_table_names(span, self.default_table_name) - @mock_dynamodb2 - def test_get_item(self): + @mock_aws + def test_get_item_expression(self): + self._create_prepared_table() + + self.client.get_item( + TableName=self.default_table_name, + Key={"id": {"S": "1"}}, + ConsistentRead=True, + ProjectionExpression="PE", + ReturnConsumedCapacity="TOTAL", + ) + + span = self.assert_span("GetItem") + self.assert_table_names(span, self.default_table_name) + self.assert_consistent_read(span, True) + self.assert_consumed_capacity(span, self.default_table_name) + + @mock_aws + def test_get_item_non_expression(self): self._create_prepared_table() self.client.get_item( TableName=self.default_table_name, Key={"id": {"S": "1"}}, ConsistentRead=True, - AttributesToGet=["id"], ProjectionExpression="PE", ReturnConsumedCapacity="TOTAL", ) @@ -334,7 +350,7 @@ def test_get_item(self): self.assert_projection(span, "PE") self.assert_consumed_capacity(span, self.default_table_name) - @mock_dynamodb2 + @mock_aws def test_list_tables(self): self._create_table(TableName="my_table") self._create_prepared_table() @@ -351,7 +367,7 @@ def test_list_tables(self): ) self.assertEqual(5, span.attributes[SpanAttributes.AWS_DYNAMODB_LIMIT]) - @mock_dynamodb2 + @mock_aws def test_put_item(self): table = "test_table" self._create_prepared_table(TableName=table) @@ -372,7 +388,7 @@ def test_put_item(self): def test_put_item_item_collection_metrics(self): self.assert_extension_item_col_metrics("PutItem") - @mock_dynamodb2 + @mock_aws def test_query(self): self._create_prepared_table() @@ -407,7 +423,7 @@ def test_query(self): self.assert_select(span, "ALL_ATTRIBUTES") self.assert_consumed_capacity(span, self.default_table_name) - @mock_dynamodb2 + @mock_aws def test_scan(self): self._create_prepared_table() @@ -444,7 +460,7 @@ def test_scan(self): self.assert_select(span, "ALL_ATTRIBUTES") self.assert_consumed_capacity(span, self.default_table_name) - @mock_dynamodb2 + @mock_aws def test_update_item(self): self._create_prepared_table() @@ -465,7 +481,7 @@ def test_update_item(self): def test_update_item_item_collection_metrics(self): self.assert_extension_item_col_metrics("UpdateItem") - @mock_dynamodb2 + @mock_aws def test_update_table(self): self._create_prepared_table() diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py index bb6d283399..62357a3336 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py @@ -12,19 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. import json -from unittest.mock import Mock, patch +from unittest.mock import ANY, Mock, patch import botocore.session from botocore.exceptions import ParamValidationError -from moto import ( # pylint: disable=import-error - mock_ec2, - mock_kinesis, - mock_kms, - mock_s3, - mock_sqs, - mock_sts, - mock_xray, -) +from moto import mock_aws # pylint: disable=import-error from opentelemetry import trace as trace_api from opentelemetry.instrumentation.botocore import BotocoreInstrumentor @@ -39,7 +31,7 @@ from opentelemetry.test.test_base import TestBase from opentelemetry.trace.span import format_span_id, format_trace_id -_REQUEST_ID_REGEX_MATCH = r"[A-Z0-9]{52}" +_REQUEST_ID_REGEX_MATCH = r"[A-Za-z0-9]{52}" # pylint:disable=too-many-public-methods @@ -102,7 +94,7 @@ def assert_span( self.assertEqual(f"{service}.{operation}", span.name) return span - @mock_ec2 + @mock_aws def test_traced_client(self): ec2 = self._make_client("ec2") @@ -111,7 +103,7 @@ def test_traced_client(self): request_id = "fdcdcab1-ae5c-489e-9c33-4637c5dda355" self.assert_span("EC2", "DescribeInstances", request_id=request_id) - @mock_ec2 + @mock_aws def test_not_recording(self): mock_tracer = Mock() mock_span = Mock() @@ -126,7 +118,7 @@ def test_not_recording(self): self.assertFalse(mock_span.set_attribute.called) self.assertFalse(mock_span.set_status.called) - @mock_s3 + @mock_aws def test_exception(self): s3 = self._make_client("s3") @@ -149,14 +141,14 @@ def test_exception(self): self.assertIn(SpanAttributes.EXCEPTION_TYPE, event.attributes) self.assertIn(SpanAttributes.EXCEPTION_MESSAGE, event.attributes) - @mock_s3 + @mock_aws def test_s3_client(self): s3 = self._make_client("s3") s3.list_buckets() self.assert_span("S3", "ListBuckets") - @mock_s3 + @mock_aws def test_s3_put(self): s3 = self._make_client("s3") @@ -174,7 +166,7 @@ def test_s3_put(self): s3.get_object(Bucket="mybucket", Key="foo") self.assert_span("S3", "GetObject", request_id=_REQUEST_ID_REGEX_MATCH) - @mock_sqs + @mock_aws def test_sqs_client(self): sqs = self._make_client("sqs") @@ -184,7 +176,7 @@ def test_sqs_client(self): "SQS", "ListQueues", request_id=_REQUEST_ID_REGEX_MATCH ) - @mock_sqs + @mock_aws def test_sqs_send_message(self): sqs = self._make_client("sqs") test_queue_name = "test_queue_name" @@ -205,14 +197,14 @@ def test_sqs_send_message(self): attributes={"aws.queue_url": queue_url}, ) - @mock_kinesis + @mock_aws def test_kinesis_client(self): kinesis = self._make_client("kinesis") kinesis.list_streams() self.assert_span("Kinesis", "ListStreams") - @mock_kinesis + @mock_aws def test_unpatch(self): kinesis = self._make_client("kinesis") @@ -221,7 +213,7 @@ def test_unpatch(self): kinesis.list_streams() self.assertEqual(0, len(self.memory_exporter.get_finished_spans())) - @mock_ec2 + @mock_aws def test_uninstrument_does_not_inject_headers(self): headers = {} @@ -240,7 +232,7 @@ def intercept_headers(**kwargs): self.assertNotIn(TRACE_HEADER_KEY, headers) - @mock_sqs + @mock_aws def test_double_patch(self): sqs = self._make_client("sqs") @@ -252,19 +244,19 @@ def test_double_patch(self): "SQS", "ListQueues", request_id=_REQUEST_ID_REGEX_MATCH ) - @mock_kms + @mock_aws def test_kms_client(self): kms = self._make_client("kms") kms.list_keys(Limit=21) span = self.assert_only_span() + expected = self._default_span_attributes("KMS", "ListKeys") + expected["aws.request_id"] = ANY # check for exact attribute set to make sure not to leak any kms secrets - self.assertEqual( - self._default_span_attributes("KMS", "ListKeys"), span.attributes - ) + self.assertEqual(expected, dict(span.attributes)) - @mock_sts + @mock_aws def test_sts_client(self): sts = self._make_client("sts") @@ -272,11 +264,11 @@ def test_sts_client(self): span = self.assert_only_span() expected = self._default_span_attributes("STS", "GetCallerIdentity") - expected["aws.request_id"] = "c6104cbe-af31-11e0-8154-cbc7ccf896c7" + expected["aws.request_id"] = ANY # check for exact attribute set to make sure not to leak any sts secrets - self.assertEqual(expected, span.attributes) + self.assertEqual(expected, dict(span.attributes)) - @mock_ec2 + @mock_aws def test_propagator_injects_into_request(self): headers = {} previous_propagator = get_global_textmap() @@ -316,7 +308,7 @@ def check_headers(**kwargs): finally: set_global_textmap(previous_propagator) - @mock_ec2 + @mock_aws def test_override_xray_propagator_injects_into_request(self): headers = {} @@ -335,7 +327,7 @@ def check_headers(**kwargs): self.assertNotIn(MockTextMapPropagator.TRACE_ID_KEY, headers) self.assertNotIn(MockTextMapPropagator.SPAN_ID_KEY, headers) - @mock_xray + @mock_aws def test_suppress_instrumentation_xray_client(self): xray_client = self._make_client("xray") with suppress_instrumentation(): @@ -343,7 +335,7 @@ def test_suppress_instrumentation_xray_client(self): xray_client.put_trace_segments(TraceSegmentDocuments=["str2"]) self.assertEqual(0, len(self.get_finished_spans())) - @mock_xray + @mock_aws def test_suppress_http_instrumentation_xray_client(self): xray_client = self._make_client("xray") with suppress_http_instrumentation(): @@ -351,7 +343,7 @@ def test_suppress_http_instrumentation_xray_client(self): xray_client.put_trace_segments(TraceSegmentDocuments=["str2"]) self.assertEqual(2, len(self.get_finished_spans())) - @mock_s3 + @mock_aws def test_request_hook(self): request_hook_service_attribute_name = "request_hook.service_name" request_hook_operation_attribute_name = "request_hook.operation_name" @@ -386,7 +378,7 @@ def request_hook(span, service_name, operation_name, api_params): }, ) - @mock_s3 + @mock_aws def test_response_hook(self): response_hook_service_attribute_name = "request_hook.service_name" response_hook_operation_attribute_name = "response_hook.operation_name" diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_lambda.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_lambda.py index 7388323100..098edfc896 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_lambda.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_lambda.py @@ -19,7 +19,7 @@ from unittest import mock import botocore.session -from moto import mock_iam, mock_lambda # pylint: disable=import-error +from moto import mock_aws # pylint: disable=import-error from pytest import mark from opentelemetry.instrumentation.botocore import BotocoreInstrumentor @@ -96,12 +96,12 @@ def _create_extension(operation: str) -> _LambdaExtension: mock_call_context = mock.MagicMock(operation=operation, params={}) return _LambdaExtension(mock_call_context) - @mock_lambda + @mock_aws def test_list_functions(self): self.client.list_functions() self.assert_span("ListFunctions") - @mock_iam + @mock_aws def _create_role_and_get_arn(self) -> str: return self.iam_client.create_role( RoleName="my-role", @@ -131,7 +131,7 @@ def _create_lambda_function(self, function_name: str, function_code: str): sys.platform == "win32", reason="requires docker and Github CI Windows does not have docker installed by default", ) - @mock_lambda + @mock_aws def test_invoke(self): previous_propagator = get_global_textmap() try: diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_sns.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_sns.py index e2b4c55732..5d6b94f145 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_sns.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_sns.py @@ -18,7 +18,7 @@ import botocore.session from botocore.awsrequest import AWSResponse -from moto import mock_sns +from moto import mock_aws from opentelemetry.instrumentation.botocore import BotocoreInstrumentor from opentelemetry.semconv.trace import ( @@ -91,11 +91,11 @@ def assert_injected_span(self, message_attrs: Dict[str, Any], span: Span): self.assertEqual(span_context.trace_id, int(trace_parent[1], 16)) self.assertEqual(span_context.span_id, int(trace_parent[2], 16)) - @mock_sns + @mock_aws def test_publish_to_topic_arn(self): self._test_publish_to_arn("TopicArn") - @mock_sns + @mock_aws def test_publish_to_target_arn(self): self._test_publish_to_arn("TargetArn") @@ -125,7 +125,7 @@ def _test_publish_to_arn(self, arg_name: str): span.attributes["messaging.destination.name"], ) - @mock_sns + @mock_aws def test_publish_to_phone_number(self): phone_number = "+10000000000" self.client.publish( @@ -138,7 +138,7 @@ def test_publish_to_phone_number(self): phone_number, span.attributes[SpanAttributes.MESSAGING_DESTINATION] ) - @mock_sns + @mock_aws def test_publish_injects_span(self): message_attrs = {} topic_arn = self._create_topic() diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_sqs.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_sqs.py index 6bcffd9274..cdf39e4ece 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_sqs.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_sqs.py @@ -1,5 +1,5 @@ import botocore.session -from moto import mock_sqs +from moto import mock_aws from opentelemetry.instrumentation.botocore import BotocoreInstrumentor from opentelemetry.semconv.trace import SpanAttributes @@ -22,7 +22,7 @@ def tearDown(self): super().tearDown() BotocoreInstrumentor().uninstrument() - @mock_sqs + @mock_aws def test_sqs_messaging_send_message(self): create_queue_result = self.client.create_queue( QueueName="test_queue_name" @@ -51,7 +51,7 @@ def test_sqs_messaging_send_message(self): response["MessageId"], ) - @mock_sqs + @mock_aws def test_sqs_messaging_send_message_batch(self): create_queue_result = self.client.create_queue( QueueName="test_queue_name" @@ -85,7 +85,7 @@ def test_sqs_messaging_send_message_batch(self): response["Successful"][0]["MessageId"], ) - @mock_sqs + @mock_aws def test_sqs_messaging_receive_message(self): create_queue_result = self.client.create_queue( QueueName="test_queue_name" @@ -116,7 +116,7 @@ def test_sqs_messaging_receive_message(self): message_result["Messages"][0]["MessageId"], ) - @mock_sqs + @mock_aws def test_sqs_messaging_failed_operation(self): with self.assertRaises(Exception): self.client.send_message( From 881a179e3b16ecff305faf4d74566b6b954c4dd2 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 13 Jun 2024 17:50:45 -0600 Subject: [PATCH 2/5] Add xray propagators that prioritizes xray environment variable (#2573) * Add AwsXrayLambdaPropagator Fixes #2457 * Remove unnecessary AWS_TRACE_HEADER_PROP * Add docstring * Fix nit * Add no environment variable test case * Add test case for valid context * Remove ipdb * Fix lint * Add missing entry point --- CHANGELOG.md | 5 + .../pyproject.toml | 1 + .../propagators/aws/aws_xray_propagator.py | 32 ++++ .../tests/test_aws_xray_lambda_propagator.py | 164 ++++++++++++++++++ 4 files changed, 202 insertions(+) create mode 100644 propagator/opentelemetry-propagator-aws-xray/tests/test_aws_xray_lambda_propagator.py diff --git a/CHANGELOG.md b/CHANGELOG.md index e17542b104..eead4dd886 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- `opentelemetry-sdk-extension-aws` Add AwsXrayLambdaPropagator + ([#2573](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2573)) + ### Breaking changes - `opentelemetry-instrumentation-asgi`, `opentelemetry-instrumentation-fastapi`, `opentelemetry-instrumentation-starlette` Use `tracer` and `meter` of originating components instead of one from `asgi` middleware diff --git a/propagator/opentelemetry-propagator-aws-xray/pyproject.toml b/propagator/opentelemetry-propagator-aws-xray/pyproject.toml index 69fd0bbbfa..4a3e22269a 100644 --- a/propagator/opentelemetry-propagator-aws-xray/pyproject.toml +++ b/propagator/opentelemetry-propagator-aws-xray/pyproject.toml @@ -30,6 +30,7 @@ dependencies = [ [project.entry-points.opentelemetry_propagator] xray = "opentelemetry.propagators.aws:AwsXRayPropagator" +xray_lambda = "opentelemetry.propagators.aws:AwsXRayLambdaPropagator" [project.urls] Homepage = "https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/propagator/opentelemetry-propagator-aws-xray" diff --git a/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/aws_xray_propagator.py b/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/aws_xray_propagator.py index 4e4a6872ea..4966218211 100644 --- a/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/aws_xray_propagator.py +++ b/propagator/opentelemetry-propagator-aws-xray/src/opentelemetry/propagators/aws/aws_xray_propagator.py @@ -58,6 +58,7 @@ import logging import typing +from os import environ from opentelemetry import trace from opentelemetry.context import Context @@ -71,6 +72,7 @@ ) TRACE_HEADER_KEY = "X-Amzn-Trace-Id" +AWS_TRACE_HEADER_ENV_KEY = "_X_AMZN_TRACE_ID" KV_PAIR_DELIMITER = ";" KEY_AND_VALUE_DELIMITER = "=" @@ -324,3 +326,33 @@ def fields(self): """Returns a set with the fields set in `inject`.""" return {TRACE_HEADER_KEY} + + +class AwsXrayLambdaPropagator(AwsXRayPropagator): + """Implementation of the AWS X-Ray Trace Header propagation protocol but + with special handling for Lambda's ``_X_AMZN_TRACE_ID` environment + variable. + """ + + def extract( + self, + carrier: CarrierT, + context: typing.Optional[Context] = None, + getter: Getter[CarrierT] = default_getter, + ) -> Context: + + xray_context = super().extract(carrier, context=context, getter=getter) + + if trace.get_current_span(context=context).get_span_context().is_valid: + return xray_context + + trace_header = environ.get(AWS_TRACE_HEADER_ENV_KEY) + + if trace_header is None: + return xray_context + + return super().extract( + {TRACE_HEADER_KEY: trace_header}, + context=xray_context, + getter=getter, + ) diff --git a/propagator/opentelemetry-propagator-aws-xray/tests/test_aws_xray_lambda_propagator.py b/propagator/opentelemetry-propagator-aws-xray/tests/test_aws_xray_lambda_propagator.py new file mode 100644 index 0000000000..a0432d1457 --- /dev/null +++ b/propagator/opentelemetry-propagator-aws-xray/tests/test_aws_xray_lambda_propagator.py @@ -0,0 +1,164 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from os import environ +from unittest import TestCase +from unittest.mock import patch + +from requests.structures import CaseInsensitiveDict + +from opentelemetry.context import get_current +from opentelemetry.propagators.aws.aws_xray_propagator import ( + TRACE_HEADER_KEY, + AwsXrayLambdaPropagator, +) +from opentelemetry.propagators.textmap import DefaultGetter +from opentelemetry.sdk.trace import ReadableSpan +from opentelemetry.trace import ( + Link, + NonRecordingSpan, + SpanContext, + TraceState, + get_current_span, + use_span, +) + + +class AwsXRayLambdaPropagatorTest(TestCase): + + def test_extract_no_environment_variable(self): + + actual_context = get_current_span( + AwsXrayLambdaPropagator().extract( + {}, context=get_current(), getter=DefaultGetter() + ) + ).get_span_context() + + self.assertEqual(hex(actual_context.trace_id), "0x0") + self.assertEqual(hex(actual_context.span_id), "0x0") + self.assertFalse( + actual_context.trace_flags.sampled, + ) + self.assertEqual(actual_context.trace_state, TraceState.get_default()) + + def test_extract_no_environment_variable_valid_context(self): + + with use_span(NonRecordingSpan(SpanContext(1, 2, False))): + + actual_context = get_current_span( + AwsXrayLambdaPropagator().extract( + {}, context=get_current(), getter=DefaultGetter() + ) + ).get_span_context() + + self.assertEqual(hex(actual_context.trace_id), "0x1") + self.assertEqual(hex(actual_context.span_id), "0x2") + self.assertFalse( + actual_context.trace_flags.sampled, + ) + self.assertEqual( + actual_context.trace_state, TraceState.get_default() + ) + + @patch.dict( + environ, + { + "_X_AMZN_TRACE_ID": ( + "Root=1-00000001-d188f8fa79d48a391a778fa6;" + "Parent=53995c3f42cd8ad8;Sampled=1;Foo=Bar" + ) + }, + ) + def test_extract_from_environment_variable(self): + + actual_context = get_current_span( + AwsXrayLambdaPropagator().extract( + {}, context=get_current(), getter=DefaultGetter() + ) + ).get_span_context() + + self.assertEqual( + hex(actual_context.trace_id), "0x1d188f8fa79d48a391a778fa6" + ) + self.assertEqual(hex(actual_context.span_id), "0x53995c3f42cd8ad8") + self.assertTrue( + actual_context.trace_flags.sampled, + ) + self.assertEqual(actual_context.trace_state, TraceState.get_default()) + + @patch.dict( + environ, + { + "_X_AMZN_TRACE_ID": ( + "Root=1-00000002-240000000000000000000002;" + "Parent=1600000000000002;Sampled=1;Foo=Bar" + ) + }, + ) + def test_add_link_from_environment_variable(self): + + propagator = AwsXrayLambdaPropagator() + + default_getter = DefaultGetter() + + carrier = CaseInsensitiveDict( + { + TRACE_HEADER_KEY: ( + "Root=1-00000001-240000000000000000000001;" + "Parent=1600000000000001;Sampled=1" + ) + } + ) + + extracted_context = propagator.extract( + carrier, context=get_current(), getter=default_getter + ) + + link_context = propagator.extract( + carrier, context=extracted_context, getter=default_getter + ) + + span = ReadableSpan( + "test", parent=extracted_context, links=[Link(link_context)] + ) + + span_parent_context = get_current_span(span.parent).get_span_context() + + self.assertEqual( + hex(span_parent_context.trace_id), "0x2240000000000000000000002" + ) + self.assertEqual( + hex(span_parent_context.span_id), "0x1600000000000002" + ) + self.assertTrue( + span_parent_context.trace_flags.sampled, + ) + self.assertEqual( + span_parent_context.trace_state, TraceState.get_default() + ) + + span_link_context = get_current_span( + span.links[0].context + ).get_span_context() + + self.assertEqual( + hex(span_link_context.trace_id), "0x1240000000000000000000001" + ) + self.assertEqual(hex(span_link_context.span_id), "0x1600000000000001") + self.assertTrue( + span_link_context.trace_flags.sampled, + ) + self.assertEqual( + span_link_context.trace_state, TraceState.get_default() + ) From 6be205e60445c7c485a487158fb26538e3ab1c03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Fri, 14 Jun 2024 13:53:28 -0300 Subject: [PATCH 3/5] consistently use of suppress_instrumentation utils (#2590) --- CHANGELOG.md | 2 + .../aiohttp_server/__init__.py | 14 ++--- .../tests/test_aiohttp_server_integration.py | 29 +++++++++- .../instrumentation/httpx/__init__.py | 10 ++-- .../tests/test_httpx_integration.py | 17 ++---- .../opentelemetry/instrumentation/utils.py | 15 ++++-- .../tests/test_utils.py | 54 +++++++++++++++++++ .../CHANGELOG.md | 5 ++ .../resource/detector/azure/vm.py | 24 ++++----- 9 files changed, 125 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eead4dd886..05199a98a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2538](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2538)) - Add Python 3.12 support ([#2572](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2572)) +- `opentelemetry-instrumentation-aiohttp-server`, `opentelemetry-instrumentation-httpx` Ensure consistently use of suppress_instrumentation utils + ([#2590](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2590)) ## Version 1.25.0/0.46b0 (2024-05-31) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py index c1ab960818..2e519ac1c5 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py @@ -19,12 +19,14 @@ from aiohttp import web from multidict import CIMultiDictProxy -from opentelemetry import context, metrics, trace -from opentelemetry.context import _SUPPRESS_HTTP_INSTRUMENTATION_KEY +from opentelemetry import metrics, trace from opentelemetry.instrumentation.aiohttp_server.package import _instruments from opentelemetry.instrumentation.aiohttp_server.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor -from opentelemetry.instrumentation.utils import http_status_to_status_code +from opentelemetry.instrumentation.utils import ( + http_status_to_status_code, + is_http_instrumentation_enabled, +) from opentelemetry.propagate import extract from opentelemetry.propagators.textmap import Getter from opentelemetry.semconv.metrics import MetricInstruments @@ -191,10 +193,8 @@ def keys(self, carrier: Dict) -> List: @web.middleware async def middleware(request, handler): """Middleware for aiohttp implementing tracing logic""" - if ( - context.get_value("suppress_instrumentation") - or context.get_value(_SUPPRESS_HTTP_INSTRUMENTATION_KEY) - or _excluded_urls.url_disabled(request.url.path) + if not is_http_instrumentation_enabled() or _excluded_urls.url_disabled( + request.url.path ): return await handler(request) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-server/tests/test_aiohttp_server_integration.py b/instrumentation/opentelemetry-instrumentation-aiohttp-server/tests/test_aiohttp_server_integration.py index b5e8ec468f..e9dfb11389 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-server/tests/test_aiohttp_server_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-server/tests/test_aiohttp_server_integration.py @@ -23,6 +23,7 @@ from opentelemetry.instrumentation.aiohttp_server import ( AioHttpServerInstrumentor, ) +from opentelemetry.instrumentation.utils import suppress_http_instrumentation from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.globals_test import reset_trace_globals from opentelemetry.test.test_base import TestBase @@ -64,16 +65,25 @@ async def default_handler(request, status=200): return aiohttp.web.Response(status=status) +@pytest.fixture(name="suppress") +def fixture_suppress(): + return False + + @pytest_asyncio.fixture(name="server_fixture") -async def fixture_server_fixture(tracer, aiohttp_server): +async def fixture_server_fixture(tracer, aiohttp_server, suppress): _, memory_exporter = tracer AioHttpServerInstrumentor().instrument() app = aiohttp.web.Application() app.add_routes([aiohttp.web.get("/test-path", default_handler)]) + if suppress: + with suppress_http_instrumentation(): + server = await aiohttp_server(app) + else: + server = await aiohttp_server(app) - server = await aiohttp_server(app) yield server, app memory_exporter.clear() @@ -128,3 +138,18 @@ async def test_status_code_instrumentation( f"http://{server.host}:{server.port}{url}" == span.attributes[SpanAttributes.HTTP_URL] ) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("suppress", [True]) +async def test_suppress_instrumentation( + tracer, server_fixture, aiohttp_client +): + _, memory_exporter = tracer + server, _ = server_fixture + assert len(memory_exporter.get_finished_spans()) == 0 + + client = await aiohttp_client(server) + await client.get("/test-path") + + assert len(memory_exporter.get_finished_spans()) == 0 diff --git a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py index 850e76eea3..5404b2f025 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -196,11 +196,13 @@ async def async_response_hook(span, request, response): import httpx -from opentelemetry import context from opentelemetry.instrumentation.httpx.package import _instruments from opentelemetry.instrumentation.httpx.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor -from opentelemetry.instrumentation.utils import http_status_to_status_code +from opentelemetry.instrumentation.utils import ( + http_status_to_status_code, + is_http_instrumentation_enabled, +) from opentelemetry.propagate import inject from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import SpanKind, TracerProvider, get_tracer @@ -347,7 +349,7 @@ def handle_request( httpx.Response, ]: """Add request info to span.""" - if context.get_value("suppress_instrumentation"): + if not is_http_instrumentation_enabled(): return self._transport.handle_request(*args, **kwargs) method, url, headers, stream, extensions = _extract_parameters( @@ -438,7 +440,7 @@ async def handle_async_request(self, *args, **kwargs) -> typing.Union[ httpx.Response, ]: """Add request info to span.""" - if context.get_value("suppress_instrumentation"): + if not is_http_instrumentation_enabled(): return await self._transport.handle_async_request(*args, **kwargs) method, url, headers, stream, extensions = _extract_parameters( diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index d64db1a8f5..06ad963ab0 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -21,12 +21,13 @@ import respx import opentelemetry.instrumentation.httpx -from opentelemetry import context, trace +from opentelemetry import trace from opentelemetry.instrumentation.httpx import ( AsyncOpenTelemetryTransport, HTTPXClientInstrumentor, SyncOpenTelemetryTransport, ) +from opentelemetry.instrumentation.utils import suppress_http_instrumentation from opentelemetry.propagate import get_global_textmap, set_global_textmap from opentelemetry.sdk import resources from opentelemetry.semconv.trace import SpanAttributes @@ -191,14 +192,9 @@ def test_not_foundbasic(self): ) def test_suppress_instrumentation(self): - token = context.attach( - context.set_value("suppress_instrumentation", True) - ) - try: + with suppress_http_instrumentation(): result = self.perform_request(self.URL) self.assertEqual(result.text, "Hello!") - finally: - context.detach(token) self.assert_span(num_spans=0) @@ -512,15 +508,10 @@ def test_not_recording(self): def test_suppress_instrumentation_new_client(self): HTTPXClientInstrumentor().instrument() - token = context.attach( - context.set_value("suppress_instrumentation", True) - ) - try: + with suppress_http_instrumentation(): client = self.create_client() result = self.perform_request(self.URL, client=client) self.assertEqual(result.text, "Hello!") - finally: - context.detach(token) self.assert_span(num_spans=0) HTTPXClientInstrumentor().uninstrument() diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 318aaeaa74..73c000ee9c 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -37,6 +37,10 @@ propagator = TraceContextTextMapPropagator() +_SUPPRESS_INSTRUMENTATION_KEY_PLAIN = ( + "suppress_instrumentation" # Set for backward compatibility +) + def extract_attributes_from_object( obj: any, attributes: Sequence[str], existing: Dict[str, str] = None @@ -161,9 +165,10 @@ def _python_path_without_directory(python_path, directory, path_separator): def is_instrumentation_enabled() -> bool: - if context.get_value(_SUPPRESS_INSTRUMENTATION_KEY): - return False - return True + return not ( + context.get_value(_SUPPRESS_INSTRUMENTATION_KEY) + or context.get_value(_SUPPRESS_INSTRUMENTATION_KEY_PLAIN) + ) def is_http_instrumentation_enabled() -> bool: @@ -188,7 +193,9 @@ def _suppress_instrumentation(*keys: str) -> Iterable[None]: @contextmanager def suppress_instrumentation() -> Iterable[None]: """Suppress instrumentation within the context.""" - with _suppress_instrumentation(_SUPPRESS_INSTRUMENTATION_KEY): + with _suppress_instrumentation( + _SUPPRESS_INSTRUMENTATION_KEY, _SUPPRESS_INSTRUMENTATION_KEY_PLAIN + ): yield diff --git a/opentelemetry-instrumentation/tests/test_utils.py b/opentelemetry-instrumentation/tests/test_utils.py index cf6cfdfd37..d3807a1bdb 100644 --- a/opentelemetry-instrumentation/tests/test_utils.py +++ b/opentelemetry-instrumentation/tests/test_utils.py @@ -15,10 +15,20 @@ import unittest from http import HTTPStatus +from opentelemetry.context import ( + _SUPPRESS_HTTP_INSTRUMENTATION_KEY, + _SUPPRESS_INSTRUMENTATION_KEY, + get_current, + get_value, +) from opentelemetry.instrumentation.sqlcommenter_utils import _add_sql_comment from opentelemetry.instrumentation.utils import ( _python_path_without_directory, http_status_to_status_code, + is_http_instrumentation_enabled, + is_instrumentation_enabled, + suppress_http_instrumentation, + suppress_instrumentation, ) from opentelemetry.trace import StatusCode @@ -186,3 +196,47 @@ def test_add_sql_comments_without_comments(self): ) self.assertEqual(commented_sql_without_semicolon, "Select 1") + + def test_is_instrumentation_enabled_by_default(self): + self.assertTrue(is_instrumentation_enabled()) + self.assertTrue(is_http_instrumentation_enabled()) + + def test_suppress_instrumentation(self): + with suppress_instrumentation(): + self.assertFalse(is_instrumentation_enabled()) + self.assertFalse(is_http_instrumentation_enabled()) + + self.assertTrue(is_instrumentation_enabled()) + self.assertTrue(is_http_instrumentation_enabled()) + + def test_suppress_http_instrumentation(self): + with suppress_http_instrumentation(): + self.assertFalse(is_http_instrumentation_enabled()) + self.assertTrue(is_instrumentation_enabled()) + + self.assertTrue(is_instrumentation_enabled()) + self.assertTrue(is_http_instrumentation_enabled()) + + def test_suppress_instrumentation_key(self): + self.assertIsNone(get_value(_SUPPRESS_INSTRUMENTATION_KEY)) + self.assertIsNone(get_value("suppress_instrumentation")) + + with suppress_instrumentation(): + ctx = get_current() + self.assertIn(_SUPPRESS_INSTRUMENTATION_KEY, ctx) + self.assertIn("suppress_instrumentation", ctx) + self.assertTrue(get_value(_SUPPRESS_INSTRUMENTATION_KEY)) + self.assertTrue(get_value("suppress_instrumentation")) + + self.assertIsNone(get_value(_SUPPRESS_INSTRUMENTATION_KEY)) + self.assertIsNone(get_value("suppress_instrumentation")) + + def test_suppress_http_instrumentation_key(self): + self.assertIsNone(get_value(_SUPPRESS_HTTP_INSTRUMENTATION_KEY)) + + with suppress_http_instrumentation(): + ctx = get_current() + self.assertIn(_SUPPRESS_HTTP_INSTRUMENTATION_KEY, ctx) + self.assertTrue(get_value(_SUPPRESS_HTTP_INSTRUMENTATION_KEY)) + + self.assertIsNone(get_value(_SUPPRESS_HTTP_INSTRUMENTATION_KEY)) diff --git a/resource/opentelemetry-resource-detector-azure/CHANGELOG.md b/resource/opentelemetry-resource-detector-azure/CHANGELOG.md index f77fce18f1..5e16c83d63 100644 --- a/resource/opentelemetry-resource-detector-azure/CHANGELOG.md +++ b/resource/opentelemetry-resource-detector-azure/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +- Ensure consistently use of suppress_instrumentation utils + ([#2590](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2590)) + ## Version 0.1.5 (2024-05-16) - Ignore vm detector if already in other rps diff --git a/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/vm.py b/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/vm.py index 2112282949..63281a46e5 100644 --- a/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/vm.py +++ b/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/vm.py @@ -17,12 +17,7 @@ from urllib.error import URLError from urllib.request import Request, urlopen -from opentelemetry.context import ( - _SUPPRESS_INSTRUMENTATION_KEY, - attach, - detach, - set_value, -) +from opentelemetry.instrumentation.utils import suppress_instrumentation from opentelemetry.sdk.resources import Resource, ResourceDetector from opentelemetry.semconv.resource import ( CloudPlatformValues, @@ -46,15 +41,14 @@ class AzureVMResourceDetector(ResourceDetector): def detect(self) -> "Resource": attributes = {} if not _can_ignore_vm_detect(): - token = attach(set_value(_SUPPRESS_INSTRUMENTATION_KEY, True)) - metadata_json = _get_azure_vm_metadata() - if not metadata_json: - return Resource(attributes) - for attribute_key in _EXPECTED_AZURE_AMS_ATTRIBUTES: - attributes[attribute_key] = _get_attribute_from_metadata( - metadata_json, attribute_key - ) - detach(token) + with suppress_instrumentation(): + metadata_json = _get_azure_vm_metadata() + if not metadata_json: + return Resource(attributes) + for attribute_key in _EXPECTED_AZURE_AMS_ATTRIBUTES: + attributes[attribute_key] = _get_attribute_from_metadata( + metadata_json, attribute_key + ) return Resource(attributes) From 795c93376c9db0a426aa62d79e28389e4ef27cf6 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Fri, 14 Jun 2024 19:01:11 +0200 Subject: [PATCH 4/5] instrumentation/pymysql: bump pymysql to 1.1.1 in test requirements (#2608) --- .../opentelemetry-instrumentation-pymysql/test-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-pymysql/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-pymysql/test-requirements.txt index 9a1076a543..5f2d8b7783 100644 --- a/instrumentation/opentelemetry-instrumentation-pymysql/test-requirements.txt +++ b/instrumentation/opentelemetry-instrumentation-pymysql/test-requirements.txt @@ -5,7 +5,7 @@ iniconfig==2.0.0 packaging==24.0 pluggy==1.5.0 py-cpuinfo==9.0.0 -PyMySQL==1.1.0 +PyMySQL==1.1.1 pytest==7.4.4 pytest-benchmark==4.0.0 tomli==2.0.1 From 91a69d4c9971a6b7e1c2b7b3eeb5856a7a300b0f Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Fri, 14 Jun 2024 19:11:24 +0200 Subject: [PATCH 5/5] aiohttp: bump aiohttp in test requirements to latest (#2607) --- .../test-requirements.txt | 2 +- .../test-requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-aiohttp-client/test-requirements.txt index 0a9c451d33..1110776262 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/test-requirements.txt +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/test-requirements.txt @@ -1,4 +1,4 @@ -aiohttp==3.9.3 +aiohttp==3.9.4 aiosignal==1.3.1 asgiref==3.7.2 async-timeout==4.0.3 diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-server/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-aiohttp-server/test-requirements.txt index c62da59804..fe7582a2bb 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-server/test-requirements.txt +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-server/test-requirements.txt @@ -1,4 +1,4 @@ -aiohttp==3.9.3 +aiohttp==3.9.4 aiosignal==1.3.1 asgiref==3.7.2 async-timeout==4.0.3