Skip to content

Commit 50168db

Browse files
committed
revert: feat(instrumentation/asgi): add target to metrics
Unfortunately I made a mistake in open-telemetry#1323 where I assumed that the scope was laready being processed by the framework at the moment of reporting, but of course that's not true as the framework has not even seen the request at that point. Is for that reason that we are not able to extract any route information in the middleware to report what the target is. Sorry for the noise, and be happy to help if anyone has any idea how we could fix this.
1 parent 7acc336 commit 50168db

File tree

2 files changed

+0
-99
lines changed

2 files changed

+0
-99
lines changed

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

-35
Original file line numberDiff line numberDiff line change
@@ -425,33 +425,6 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]:
425425

426426
return span_name, {}
427427

428-
429-
def _collect_target_attribute(
430-
scope: typing.Dict[str, typing.Any]
431-
) -> typing.Optional[str]:
432-
"""
433-
Returns the target path as defined by the Semantic Conventions.
434-
435-
This value is suitable to use in metrics as it should replace concrete
436-
values with a parameterized name. Example: /api/users/{user_id}
437-
438-
Refer to the specification
439-
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#parameterized-attributes
440-
441-
Note: this function requires specific code for each framework, as there's no
442-
standard attribute to use.
443-
"""
444-
# FastAPI
445-
root_path = scope.get("root_path", "")
446-
447-
route = scope.get("route")
448-
path_format = getattr(route, "path_format", None)
449-
if path_format:
450-
return f"{root_path}{path_format}"
451-
452-
return None
453-
454-
455428
class OpenTelemetryMiddleware:
456429
"""The ASGI application middleware.
457430
@@ -473,7 +446,6 @@ class OpenTelemetryMiddleware:
473446
the current globally configured one is used.
474447
"""
475448

476-
# pylint: disable=too-many-branches
477449
def __init__(
478450
self,
479451
app,
@@ -542,11 +514,6 @@ async def __call__(self, scope, receive, send):
542514
)
543515
duration_attrs = _parse_duration_attrs(attributes)
544516

545-
target = _collect_target_attribute(scope)
546-
if target:
547-
active_requests_count_attrs[SpanAttributes.HTTP_TARGET] = target
548-
duration_attrs[SpanAttributes.HTTP_TARGET] = target
549-
550517
if scope["type"] == "http":
551518
self.active_requests_counter.add(1, active_requests_count_attrs)
552519
try:
@@ -589,8 +556,6 @@ async def __call__(self, scope, receive, send):
589556
if token:
590557
context.detach(token)
591558

592-
# pylint: enable=too-many-branches
593-
594559
def _get_otel_receive(self, server_span_name, scope, receive):
595560
@wraps(receive)
596561
async def otel_receive():

instrumentation/opentelemetry-instrumentation-asgi/tests/test_asgi_middleware.py

-64
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
# pylint: disable=too-many-lines
16-
1715
import sys
1816
import unittest
1917
from timeit import default_timer
@@ -585,37 +583,6 @@ def test_basic_metric_success(self):
585583
)
586584
self.assertEqual(point.value, 0)
587585

588-
def test_metric_target_attribute(self):
589-
expected_target = "/api/user/{id}"
590-
591-
class TestRoute:
592-
path_format = expected_target
593-
594-
self.scope["route"] = TestRoute()
595-
app = otel_asgi.OpenTelemetryMiddleware(simple_asgi)
596-
self.seed_app(app)
597-
self.send_default_request()
598-
599-
metrics_list = self.memory_metrics_reader.get_metrics_data()
600-
assertions = 0
601-
for resource_metric in metrics_list.resource_metrics:
602-
for scope_metrics in resource_metric.scope_metrics:
603-
for metric in scope_metrics.metrics:
604-
for point in metric.data.data_points:
605-
if isinstance(point, HistogramDataPoint):
606-
self.assertEqual(
607-
point.attributes["http.target"],
608-
expected_target,
609-
)
610-
assertions += 1
611-
elif isinstance(point, NumberDataPoint):
612-
self.assertEqual(
613-
point.attributes["http.target"],
614-
expected_target,
615-
)
616-
assertions += 1
617-
self.assertEqual(assertions, 2)
618-
619586
def test_no_metric_for_websockets(self):
620587
self.scope = {
621588
"type": "websocket",
@@ -709,37 +676,6 @@ def test_credential_removal(self):
709676
attrs[SpanAttributes.HTTP_URL], "http://httpbin.org/status/200"
710677
)
711678

712-
def test_collect_target_attribute_missing(self):
713-
self.assertIsNone(otel_asgi._collect_target_attribute(self.scope))
714-
715-
def test_collect_target_attribute_fastapi(self):
716-
class TestRoute:
717-
path_format = "/api/users/{user_id}"
718-
719-
self.scope["route"] = TestRoute()
720-
self.assertEqual(
721-
otel_asgi._collect_target_attribute(self.scope),
722-
"/api/users/{user_id}",
723-
)
724-
725-
def test_collect_target_attribute_fastapi_mounted(self):
726-
class TestRoute:
727-
path_format = "/users/{user_id}"
728-
729-
self.scope["route"] = TestRoute()
730-
self.scope["root_path"] = "/api/v2"
731-
self.assertEqual(
732-
otel_asgi._collect_target_attribute(self.scope),
733-
"/api/v2/users/{user_id}",
734-
)
735-
736-
def test_collect_target_attribute_fastapi_starlette_invalid(self):
737-
self.scope["route"] = object()
738-
self.assertIsNone(
739-
otel_asgi._collect_target_attribute(self.scope),
740-
"HTTP_TARGET values is not None",
741-
)
742-
743679

744680
class TestWrappedApplication(AsgiTestBase):
745681
def test_mark_span_internal_in_presence_of_span_from_other_framework(self):

0 commit comments

Comments
 (0)