Skip to content

Commit 4820302

Browse files
authored
Merge branch 'main' into feature/openai-agents-content-capture
2 parents c9aee0e + ba0644f commit 4820302

File tree

3 files changed

+66
-3
lines changed

3 files changed

+66
-3
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717
([#3624](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3624))
1818
- `opentelemetry-instrumentation-dbapi`: fix crash retrieving libpq version when enabling commenter with psycopg
1919
([#3796](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3796))
20+
- `opentelemetry-instrumentation-fastapi`: Fix handling of APIRoute subclasses
21+
([#3681](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3681))
2022

2123
### Added
2224

instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
191191
import fastapi
192192
from starlette.applications import Starlette
193193
from starlette.middleware.errors import ServerErrorMiddleware
194-
from starlette.routing import Match
194+
from starlette.routing import Match, Route
195195
from starlette.types import ASGIApp, Receive, Scope, Send
196196

197197
from opentelemetry.instrumentation._semconv import (
@@ -474,7 +474,11 @@ def _get_route_details(scope):
474474
route = None
475475

476476
for starlette_route in app.routes:
477-
match, _ = starlette_route.matches(scope)
477+
match, _ = (
478+
Route.matches(starlette_route, scope)
479+
if isinstance(starlette_route, Route)
480+
else starlette_route.matches(scope)
481+
)
478482
if match == Match.FULL:
479483
try:
480484
route = starlette_route.path

instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,17 @@
2020
import weakref as _weakref
2121
from contextlib import ExitStack
2222
from timeit import default_timer
23-
from typing import Any, cast
23+
from typing import Any, Final, cast
2424
from unittest.mock import Mock, call, patch
2525

2626
import fastapi
2727
import pytest
2828
from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware
2929
from fastapi.responses import JSONResponse, PlainTextResponse
30+
from fastapi.routing import APIRoute
3031
from fastapi.testclient import TestClient
32+
from starlette.routing import Match
33+
from starlette.types import Receive, Scope, Send
3134

3235
import opentelemetry.instrumentation.fastapi as otel_fastapi
3336
from opentelemetry import trace
@@ -131,6 +134,23 @@
131134
)
132135

133136

137+
class CustomMiddleware:
138+
def __init__(self, app: fastapi.FastAPI) -> None:
139+
self.app = app
140+
141+
async def __call__(
142+
self, scope: Scope, receive: Receive, send: Send
143+
) -> None:
144+
scope["nonstandard_field"] = "here"
145+
await self.app(scope, receive, send)
146+
147+
148+
class CustomRoute(APIRoute):
149+
def matches(self, scope: Scope) -> tuple[Match, Scope]:
150+
assert "nonstandard_field" in scope
151+
return super().matches(scope)
152+
153+
134154
class TestBaseFastAPI(TestBase):
135155
def _create_app(self):
136156
app = self._create_fastapi_app()
@@ -191,6 +211,7 @@ def setUp(self):
191211
self._instrumentor = otel_fastapi.FastAPIInstrumentor()
192212
self._app = self._create_app()
193213
self._app.add_middleware(HTTPSRedirectMiddleware)
214+
self._app.add_middleware(CustomMiddleware)
194215
self._client = TestClient(self._app, base_url="https://testserver:443")
195216
# run the lifespan, initialize the middleware stack
196217
# this is more in-line with what happens in a real application when the server starts up
@@ -210,6 +231,7 @@ def tearDown(self):
210231
def _create_fastapi_app():
211232
app = fastapi.FastAPI()
212233
sub_app = fastapi.FastAPI()
234+
custom_router = fastapi.APIRouter(route_class=CustomRoute)
213235

214236
@sub_app.get("/home")
215237
async def _():
@@ -235,6 +257,12 @@ async def _():
235257
async def _():
236258
raise UnhandledException("This is an unhandled exception")
237259

260+
@custom_router.get("/success")
261+
async def _():
262+
return None
263+
264+
app.include_router(custom_router, prefix="/custom-router")
265+
238266
app.mount("/sub", app=sub_app)
239267
app.host("testserver2", sub_app)
240268

@@ -313,6 +341,28 @@ def test_sub_app_fastapi_call(self):
313341
span.attributes[HTTP_URL],
314342
)
315343

344+
def test_custom_api_router(self):
345+
"""
346+
This test is to ensure that custom API routers the OpenTelemetryMiddleware does not cause issues with
347+
custom API routers that depend on non-standard fields on the ASGI scope.
348+
"""
349+
resp: Final = self._client.get("/custom-router/success")
350+
spans: Final = self.memory_exporter.get_finished_spans()
351+
spans_with_http_attributes = [
352+
span
353+
for span in spans
354+
if (HTTP_URL in span.attributes or HTTP_TARGET in span.attributes)
355+
]
356+
self.assertEqual(200, resp.status_code)
357+
for span in spans_with_http_attributes:
358+
self.assertEqual(
359+
"/custom-router/success", span.attributes[HTTP_TARGET]
360+
)
361+
self.assertEqual(
362+
"https://testserver/custom-router/success",
363+
span.attributes[HTTP_URL],
364+
)
365+
316366
def test_host_fastapi_call(self):
317367
client = TestClient(self._app, base_url="https://testserver2:443")
318368
client.get("/")
@@ -1017,6 +1067,7 @@ def test_metric_uninstrument(self):
10171067
def _create_fastapi_app():
10181068
app = fastapi.FastAPI()
10191069
sub_app = fastapi.FastAPI()
1070+
custom_router = fastapi.APIRouter(route_class=CustomRoute)
10201071

10211072
@sub_app.get("/home")
10221073
async def _():
@@ -1042,6 +1093,12 @@ async def _():
10421093
async def _():
10431094
raise UnhandledException("This is an unhandled exception")
10441095

1096+
@custom_router.get("/success")
1097+
async def _():
1098+
return None
1099+
1100+
app.include_router(custom_router, prefix="/custom-router")
1101+
10451102
app.mount("/sub", app=sub_app)
10461103

10471104
return app

0 commit comments

Comments
 (0)