Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consistently use of suppress_instrumentation utils #2590

Merged
merged 8 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- `opentelemetry-instrumentation-aiohttp-server`, `opentelemetry-instrumentation-httpx`, `opentelemetry-resource-detector-azure` Ensure consistently use of suppress_instrumentation utils
emdneto marked this conversation as resolved.
Show resolved Hide resolved
([#2590](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2590))

### Breaking changes

- `opentelemetry-instrumentation-asgi`, `opentelemetry-instrumentation-fastapi`, `opentelemetry-instrumentation-starlette` Use `tracer` and `meter` of originating components instead of one from `asgi` middleware
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@

propagator = TraceContextTextMapPropagator()

_SUPPRESS_INSTRUMENTATION_KEY_PLAIN = (
emdneto marked this conversation as resolved.
Show resolved Hide resolved
"suppress_instrumentation" # Set for backward compatibility
)


def extract_attributes_from_object(
obj: any, attributes: Sequence[str], existing: Dict[str, str] = None
Expand Down Expand Up @@ -161,9 +165,10 @@ def _python_path_without_directory(python_path, directory, path_separator):


def is_instrumentation_enabled() -> bool:
emdneto marked this conversation as resolved.
Show resolved Hide resolved
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:
Expand All @@ -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


Expand Down
54 changes: 54 additions & 0 deletions opentelemetry-instrumentation/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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))
emdneto marked this conversation as resolved.
Show resolved Hide resolved
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))
5 changes: 5 additions & 0 deletions resource/opentelemetry-resource-detector-azure/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)


Expand Down