Skip to content

Commit

Permalink
Do not use asgi name and version for tracer/meter for instrumentati…
Browse files Browse the repository at this point in the history
…ons using Asgi Middleware (#2580)
  • Loading branch information
lzchen committed Jun 6, 2024
1 parent bb9eebb commit 5b84128
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 9 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### Breaking changes

- `opentelemetry-instrumentation-asgi`, `opentelemetry-instrumentation-fastapi`, `opentelemetry-instrumentation-starlette` Use `tracer` and `meter` of originating components instead of one from `asgi` middleware
([#2580](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2580))

### Fixed

- `opentelemetry-instrumentation-httpx` Ensure httpx.get or httpx.request like methods are instrumented
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,8 @@ class OpenTelemetryMiddleware:
scope and event which are sent as dictionaries for when the method send is called.
tracer_provider: The optional tracer provider to use. If omitted
the current globally configured one is used.
meter_provider: The optional meter provider to use. If omitted
the current globally configured one is used.
"""

# pylint: disable=too-many-branches
Expand All @@ -474,17 +476,22 @@ def __init__(
client_response_hook: ClientResponseHook = None,
tracer_provider=None,
meter_provider=None,
tracer=None,
meter=None,
http_capture_headers_server_request: list[str] | None = None,
http_capture_headers_server_response: list[str] | None = None,
http_capture_headers_sanitize_fields: list[str] | None = None,
):
self.app = guarantee_single_callable(app)
self.tracer = trace.get_tracer(
__name__,
__version__,
tracer_provider,
schema_url="https://opentelemetry.io/schemas/1.11.0",
self.tracer = (
trace.get_tracer(
__name__,
__version__,
tracer_provider,
schema_url="https://opentelemetry.io/schemas/1.11.0",
)
if tracer is None
else tracer
)
self.meter = (
get_meter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,10 @@ def validate_outputs(self, outputs, error=None, modifiers=None):
self.assertEqual(span.name, expected["name"])
self.assertEqual(span.kind, expected["kind"])
self.assertDictEqual(dict(span.attributes), expected["attributes"])
self.assertEqual(
span.instrumentation_scope.name,
"opentelemetry.instrumentation.asgi",
)

def test_basic_asgi_call(self):
"""Test that spans are emitted as expected."""
Expand Down Expand Up @@ -728,6 +732,10 @@ def test_asgi_metrics(self):
self.assertTrue(len(resource_metric.scope_metrics) != 0)
for scope_metric in resource_metric.scope_metrics:
self.assertTrue(len(scope_metric.metrics) != 0)
self.assertEqual(
scope_metric.scope.name,
"opentelemetry.instrumentation.asgi",
)
for metric in scope_metric.metrics:
self.assertIn(metric.name, _expected_metric_names)
data_points = list(metric.data.data_points)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
from opentelemetry.metrics import get_meter
from opentelemetry.semconv.trace import SpanAttributes
from opentelemetry.trace import get_tracer
from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls

_excluded_urls_from_env = get_excluded_urls("FASTAPI")
Expand Down Expand Up @@ -221,6 +222,12 @@ def instrument_app(
excluded_urls = _excluded_urls_from_env
else:
excluded_urls = parse_excluded_urls(excluded_urls)
tracer = get_tracer(
__name__,
__version__,
tracer_provider,
schema_url="https://opentelemetry.io/schemas/1.11.0",
)
meter = get_meter(
__name__,
__version__,
Expand All @@ -235,7 +242,8 @@ def instrument_app(
server_request_hook=server_request_hook,
client_request_hook=client_request_hook,
client_response_hook=client_response_hook,
tracer_provider=tracer_provider,
# Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation
tracer=tracer,
meter=meter,
)
app._is_instrumented_by_opentelemetry = True
Expand Down Expand Up @@ -298,6 +306,12 @@ class _InstrumentedFastAPI(fastapi.FastAPI):

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
tracer = get_tracer(
__name__,
__version__,
_InstrumentedFastAPI._tracer_provider,
schema_url="https://opentelemetry.io/schemas/1.11.0",
)
meter = get_meter(
__name__,
__version__,
Expand All @@ -311,7 +325,8 @@ def __init__(self, *args, **kwargs):
server_request_hook=_InstrumentedFastAPI._server_request_hook,
client_request_hook=_InstrumentedFastAPI._client_request_hook,
client_response_hook=_InstrumentedFastAPI._client_response_hook,
tracer_provider=_InstrumentedFastAPI._tracer_provider,
# Pass in tracer/meter to get __name__and __version__ of fastapi instrumentation
tracer=tracer,
meter=meter,
)
self._is_instrumented_by_opentelemetry = True
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ def test_instrument_app_with_instrument(self):
self.assertEqual(len(spans), 3)
for span in spans:
self.assertIn("GET /foobar", span.name)
self.assertEqual(
span.instrumentation_scope.name,
"opentelemetry.instrumentation.fastapi",
)

def test_uninstrument_app(self):
self._client.get("/foobar")
Expand Down Expand Up @@ -197,6 +201,10 @@ def test_fastapi_metrics(self):
for resource_metric in metrics_list.resource_metrics:
self.assertTrue(len(resource_metric.scope_metrics) == 1)
for scope_metric in resource_metric.scope_metrics:
self.assertEqual(
scope_metric.scope.name,
"opentelemetry.instrumentation.fastapi",
)
self.assertTrue(len(scope_metric.metrics) == 3)
for metric in scope_metric.metrics:
self.assertIn(metric.name, _expected_metric_names)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
from opentelemetry.instrumentation.starlette.version import __version__
from opentelemetry.metrics import get_meter
from opentelemetry.semconv.trace import SpanAttributes
from opentelemetry.trace import get_tracer
from opentelemetry.util.http import get_excluded_urls

_excluded_urls = get_excluded_urls("STARLETTE")
Expand All @@ -208,6 +209,12 @@ def instrument_app(
tracer_provider=None,
):
"""Instrument an uninstrumented Starlette application."""
tracer = get_tracer(
__name__,
__version__,
tracer_provider,
schema_url="https://opentelemetry.io/schemas/1.11.0",
)
meter = get_meter(
__name__,
__version__,
Expand All @@ -222,7 +229,8 @@ def instrument_app(
server_request_hook=server_request_hook,
client_request_hook=client_request_hook,
client_response_hook=client_response_hook,
tracer_provider=tracer_provider,
# Pass in tracer/meter to get __name__and __version__ of starlette instrumentation
tracer=tracer,
meter=meter,
)
app.is_instrumented_by_opentelemetry = True
Expand Down Expand Up @@ -278,6 +286,12 @@ class _InstrumentedStarlette(applications.Starlette):

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
tracer = get_tracer(
__name__,
__version__,
_InstrumentedStarlette._tracer_provider,
schema_url="https://opentelemetry.io/schemas/1.11.0",
)
meter = get_meter(
__name__,
__version__,
Expand All @@ -291,7 +305,8 @@ def __init__(self, *args, **kwargs):
server_request_hook=_InstrumentedStarlette._server_request_hook,
client_request_hook=_InstrumentedStarlette._client_request_hook,
client_response_hook=_InstrumentedStarlette._client_response_hook,
tracer_provider=_InstrumentedStarlette._tracer_provider,
# Pass in tracer/meter to get __name__and __version__ of starlette instrumentation
tracer=tracer,
meter=meter,
)
self._is_instrumented_by_opentelemetry = True
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ def test_basic_starlette_call(self):
self.assertEqual(len(spans), 3)
for span in spans:
self.assertIn("GET /foobar", span.name)
self.assertEqual(
span.instrumentation_scope.name,
"opentelemetry.instrumentation.starlette",
)

def test_starlette_route_attribute_added(self):
"""Ensure that starlette routes are used as the span name."""
Expand Down Expand Up @@ -132,6 +136,10 @@ def test_starlette_metrics(self):
for resource_metric in metrics_list.resource_metrics:
self.assertTrue(len(resource_metric.scope_metrics) == 1)
for scope_metric in resource_metric.scope_metrics:
self.assertEqual(
scope_metric.scope.name,
"opentelemetry.instrumentation.starlette",
)
self.assertTrue(len(scope_metric.metrics) == 3)
for metric in scope_metric.metrics:
self.assertIn(metric.name, _expected_metric_names)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,8 @@ class OpenTelemetryMiddleware:
incoming request.
tracer_provider: Optional tracer provider to use. If omitted the current
globally configured one is used.
meter_provider: Optional meter provider to use. If omitted the current
globally configured one is used.
"""

def __init__(
Expand Down

0 comments on commit 5b84128

Please sign in to comment.