Skip to content

Commit 86d26ce

Browse files
frankie567xrmxemdneto
authored
starlette/fastapi: fix error on host-based routing (#3507)
* starlette/fastapi: fix error on host-based routing Fix #3506 * Update CHANGELOG.md * Update instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py * Update CHANGELOG.md --------- Co-authored-by: Riccardo Magliocchetti <[email protected]> Co-authored-by: Emídio Neto <[email protected]>
1 parent e211bff commit 86d26ce

File tree

5 files changed

+53
-3
lines changed

5 files changed

+53
-3
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2323
([#3679](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3679))
2424
- `opentelemetry-instrumentation`: Avoid calls to `context.detach` with `None` token.
2525
([#3673](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3673))
26+
- `opentelemetry-instrumentation-starlette`/`opentelemetry-instrumentation-fastapi`: Fixes a crash when host-based routing is used
27+
([#3507](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3507))
2628

2729
### Added
2830

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,11 @@ def _get_route_details(scope):
516516
for starlette_route in app.routes:
517517
match, _ = starlette_route.matches(scope)
518518
if match == Match.FULL:
519-
route = starlette_route.path
519+
try:
520+
route = starlette_route.path
521+
except AttributeError:
522+
# routes added via host routing won't have a path attribute
523+
route = scope.get("path")
520524
break
521525
if match == Match.PARTIAL:
522526
route = starlette_route.path

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ async def _():
234234
raise UnhandledException("This is an unhandled exception")
235235

236236
app.mount("/sub", app=sub_app)
237+
app.host("testserver2", sub_app)
237238

238239
return app
239240

@@ -310,6 +311,26 @@ def test_sub_app_fastapi_call(self):
310311
span.attributes[HTTP_URL],
311312
)
312313

314+
def test_host_fastapi_call(self):
315+
client = TestClient(self._app, base_url="https://testserver2")
316+
client.get("/")
317+
spans = self.memory_exporter.get_finished_spans()
318+
319+
spans_with_http_attributes = [
320+
span
321+
for span in spans
322+
if (HTTP_URL in span.attributes or HTTP_TARGET in span.attributes)
323+
]
324+
325+
self.assertEqual(1, len(spans_with_http_attributes))
326+
327+
for span in spans_with_http_attributes:
328+
self.assertEqual("/", span.attributes[HTTP_TARGET])
329+
self.assertEqual(
330+
"https://testserver2:443/",
331+
span.attributes[HTTP_URL],
332+
)
333+
313334

314335
class TestBaseAutoFastAPI(TestBaseFastAPI):
315336
@classmethod

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,11 @@ def _get_route_details(scope: dict[str, Any]) -> str | None:
354354
for starlette_route in app.routes:
355355
match, _ = starlette_route.matches(scope)
356356
if match == Match.FULL:
357-
route = starlette_route.path
357+
try:
358+
route = starlette_route.path
359+
except AttributeError:
360+
# routes added via host routing won't have a path attribute
361+
route = scope.get("path")
358362
break
359363
if match == Match.PARTIAL:
360364
route = starlette_route.path

instrumentation/opentelemetry-instrumentation-starlette/tests/test_starlette_instrumentation.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
from starlette import applications
2020
from starlette.responses import PlainTextResponse
21-
from starlette.routing import Mount, Route
21+
from starlette.routing import Host, Mount, Route
2222
from starlette.testclient import TestClient
2323
from starlette.websockets import WebSocket
2424

@@ -140,6 +140,24 @@ def test_sub_app_starlette_call(self):
140140
span.attributes[HTTP_URL],
141141
)
142142

143+
def test_host_starlette_call(self):
144+
client = TestClient(self._app, base_url="http://testserver2")
145+
client.get("/home")
146+
spans = self.memory_exporter.get_finished_spans()
147+
148+
spans_with_http_attributes = [
149+
span
150+
for span in spans
151+
if (HTTP_URL in span.attributes or HTTP_TARGET in span.attributes)
152+
]
153+
154+
for span in spans_with_http_attributes:
155+
self.assertEqual("/home", span.attributes[HTTP_TARGET])
156+
self.assertEqual(
157+
"http://testserver2/home",
158+
span.attributes[HTTP_URL],
159+
)
160+
143161
def test_starlette_route_attribute_added(self):
144162
"""Ensure that starlette routes are used as the span name."""
145163
self._client.get("/user/123")
@@ -294,6 +312,7 @@ def sub_home(_):
294312
Route("/user/{username}", home),
295313
Route("/healthzz", health),
296314
Mount("/sub", app=sub_app),
315+
Host("testserver2", sub_app),
297316
],
298317
)
299318

0 commit comments

Comments
 (0)