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] 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)