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)