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 <riccardo.magliocchetti@gmail.com> Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com>
This commit is contained in:
parent
e211bffc0a
commit
86d26ce1b8
|
|
@ -23,6 +23,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||||
([#3679](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3679))
|
([#3679](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3679))
|
||||||
- `opentelemetry-instrumentation`: Avoid calls to `context.detach` with `None` token.
|
- `opentelemetry-instrumentation`: Avoid calls to `context.detach` with `None` token.
|
||||||
([#3673](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3673))
|
([#3673](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3673))
|
||||||
|
- `opentelemetry-instrumentation-starlette`/`opentelemetry-instrumentation-fastapi`: Fixes a crash when host-based routing is used
|
||||||
|
([#3507](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3507))
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -516,7 +516,11 @@ def _get_route_details(scope):
|
||||||
for starlette_route in app.routes:
|
for starlette_route in app.routes:
|
||||||
match, _ = starlette_route.matches(scope)
|
match, _ = starlette_route.matches(scope)
|
||||||
if match == Match.FULL:
|
if match == Match.FULL:
|
||||||
route = starlette_route.path
|
try:
|
||||||
|
route = starlette_route.path
|
||||||
|
except AttributeError:
|
||||||
|
# routes added via host routing won't have a path attribute
|
||||||
|
route = scope.get("path")
|
||||||
break
|
break
|
||||||
if match == Match.PARTIAL:
|
if match == Match.PARTIAL:
|
||||||
route = starlette_route.path
|
route = starlette_route.path
|
||||||
|
|
|
||||||
|
|
@ -234,6 +234,7 @@ class TestBaseFastAPI(TestBase):
|
||||||
raise UnhandledException("This is an unhandled exception")
|
raise UnhandledException("This is an unhandled exception")
|
||||||
|
|
||||||
app.mount("/sub", app=sub_app)
|
app.mount("/sub", app=sub_app)
|
||||||
|
app.host("testserver2", sub_app)
|
||||||
|
|
||||||
return app
|
return app
|
||||||
|
|
||||||
|
|
@ -310,6 +311,26 @@ class TestBaseManualFastAPI(TestBaseFastAPI):
|
||||||
span.attributes[HTTP_URL],
|
span.attributes[HTTP_URL],
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_host_fastapi_call(self):
|
||||||
|
client = TestClient(self._app, base_url="https://testserver2")
|
||||||
|
client.get("/")
|
||||||
|
spans = self.memory_exporter.get_finished_spans()
|
||||||
|
|
||||||
|
spans_with_http_attributes = [
|
||||||
|
span
|
||||||
|
for span in spans
|
||||||
|
if (HTTP_URL in span.attributes or HTTP_TARGET in span.attributes)
|
||||||
|
]
|
||||||
|
|
||||||
|
self.assertEqual(1, len(spans_with_http_attributes))
|
||||||
|
|
||||||
|
for span in spans_with_http_attributes:
|
||||||
|
self.assertEqual("/", span.attributes[HTTP_TARGET])
|
||||||
|
self.assertEqual(
|
||||||
|
"https://testserver2:443/",
|
||||||
|
span.attributes[HTTP_URL],
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class TestBaseAutoFastAPI(TestBaseFastAPI):
|
class TestBaseAutoFastAPI(TestBaseFastAPI):
|
||||||
@classmethod
|
@classmethod
|
||||||
|
|
|
||||||
|
|
@ -354,7 +354,11 @@ def _get_route_details(scope: dict[str, Any]) -> str | None:
|
||||||
for starlette_route in app.routes:
|
for starlette_route in app.routes:
|
||||||
match, _ = starlette_route.matches(scope)
|
match, _ = starlette_route.matches(scope)
|
||||||
if match == Match.FULL:
|
if match == Match.FULL:
|
||||||
route = starlette_route.path
|
try:
|
||||||
|
route = starlette_route.path
|
||||||
|
except AttributeError:
|
||||||
|
# routes added via host routing won't have a path attribute
|
||||||
|
route = scope.get("path")
|
||||||
break
|
break
|
||||||
if match == Match.PARTIAL:
|
if match == Match.PARTIAL:
|
||||||
route = starlette_route.path
|
route = starlette_route.path
|
||||||
|
|
|
||||||
|
|
@ -18,7 +18,7 @@ from unittest.mock import patch
|
||||||
|
|
||||||
from starlette import applications
|
from starlette import applications
|
||||||
from starlette.responses import PlainTextResponse
|
from starlette.responses import PlainTextResponse
|
||||||
from starlette.routing import Mount, Route
|
from starlette.routing import Host, Mount, Route
|
||||||
from starlette.testclient import TestClient
|
from starlette.testclient import TestClient
|
||||||
from starlette.websockets import WebSocket
|
from starlette.websockets import WebSocket
|
||||||
|
|
||||||
|
|
@ -140,6 +140,24 @@ class TestStarletteManualInstrumentation(TestBase):
|
||||||
span.attributes[HTTP_URL],
|
span.attributes[HTTP_URL],
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_host_starlette_call(self):
|
||||||
|
client = TestClient(self._app, base_url="http://testserver2")
|
||||||
|
client.get("/home")
|
||||||
|
spans = self.memory_exporter.get_finished_spans()
|
||||||
|
|
||||||
|
spans_with_http_attributes = [
|
||||||
|
span
|
||||||
|
for span in spans
|
||||||
|
if (HTTP_URL in span.attributes or HTTP_TARGET in span.attributes)
|
||||||
|
]
|
||||||
|
|
||||||
|
for span in spans_with_http_attributes:
|
||||||
|
self.assertEqual("/home", span.attributes[HTTP_TARGET])
|
||||||
|
self.assertEqual(
|
||||||
|
"http://testserver2/home",
|
||||||
|
span.attributes[HTTP_URL],
|
||||||
|
)
|
||||||
|
|
||||||
def test_starlette_route_attribute_added(self):
|
def test_starlette_route_attribute_added(self):
|
||||||
"""Ensure that starlette routes are used as the span name."""
|
"""Ensure that starlette routes are used as the span name."""
|
||||||
self._client.get("/user/123")
|
self._client.get("/user/123")
|
||||||
|
|
@ -294,6 +312,7 @@ class TestStarletteManualInstrumentation(TestBase):
|
||||||
Route("/user/{username}", home),
|
Route("/user/{username}", home),
|
||||||
Route("/healthzz", health),
|
Route("/healthzz", health),
|
||||||
Mount("/sub", app=sub_app),
|
Mount("/sub", app=sub_app),
|
||||||
|
Host("testserver2", sub_app),
|
||||||
],
|
],
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue