Skip to content

Commit

Permalink
feat(instrumentation/aasgi): add target to metrics
Browse files Browse the repository at this point in the history
This PR adds the target information for metrics reported by instrumentation/asgi.

Unfortunately, there's no ASGI standard to reliably get this information, and I was only able to get it for FastAPI.

I also tried to get the info with Sanic and Starlette (encode/starlette#685), but there's nothing in the scope allowing to recreate the route.

Besides the included unit tests, the logic was tested using the following app:

```python
import io
import fastapi

app = fastapi.FastAPI()

def dump_scope(scope):
    b = io.StringIO()
    print(scope, file=b)
    return b.getvalue()

@app.get("/test/{id}")
def test(id: str, req: fastapi.Request):
    print(req.scope)
    return {"target": _collect_target_attribute(req.scope), "scope": dump_scope(req.scope)}

sub_app = fastapi.FastAPI()

@sub_app.get("/test/{id}")
def sub_test(id: str, req: fastapi.Request):
    print(req.scope)
    return {"target": _collect_target_attribute(req.scope), "scope": dump_scope(req.scope)}

app.mount("/sub", sub_app)
```

Partially fixes open-telemetry#1116

Note to reviewers: I tried to touch as less as possible, so that we don;t require a refactor before this change. However, we could consider changing `collect_request_attributes` so that it returns both a trace attributes and a metrics attributes.
Wihout that change we cannot add the `HTTP_TARGET` attribute to the list of metric atttributes, because it will be present but with high cardinality.
  • Loading branch information
sk- committed Sep 10, 2022
1 parent 4c23823 commit 5468671
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 0 deletions.
Expand Up @@ -365,6 +365,34 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]:
return span_name, {}


def _collect_target_attribute(
scope: typing.Dict[str, typing.Any]
) -> typing.Optional[str]:
"""
Returns the target path as defined by the Semantic Conventions.
This value is suitable to use in metrics as it should replace concrete
values with a parameterized name. Example: /api/users/{user_id}
Refer to the specification
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/http-metrics.md#parameterized-attributes
Note: this function requires specific code for each framework, as there's no
standard attribute to use.
"""
# FastAPI
root_path = scope.get("root_path", "")

route = scope.get("route")
if not route:
return None
path_format = getattr(route, "path_format", None)
if path_format:
return f"{root_path}{path_format}"

return None


class OpenTelemetryMiddleware:
"""The ASGI application middleware.
Expand Down Expand Up @@ -448,6 +476,12 @@ async def __call__(self, scope, receive, send):
attributes
)
duration_attrs = _parse_duration_attrs(attributes)

target = _collect_target_attribute(scope)
if target:
active_requests_count_attrs[SpanAttributes.HTTP_TARGET] = target
duration_attrs[SpanAttributes.HTTP_TARGET] = target

if scope["type"] == "http":
self.active_requests_counter.add(1, active_requests_count_attrs)
try:
Expand Down
Expand Up @@ -612,6 +612,37 @@ def test_basic_metric_success(self):
)
self.assertEqual(point.value, 0)

def test_metric_target_attribute(self):
expected_target = "/api/user/{id}"

class TestRoute:
path_format = expected_target

self.scope["route"] = TestRoute()
app = otel_asgi.OpenTelemetryMiddleware(simple_asgi)
self.seed_app(app)
self.send_default_request()

metrics_list = self.memory_metrics_reader.get_metrics_data()
assertions = 0
for resource_metric in metrics_list.resource_metrics:
for scope_metrics in resource_metric.scope_metrics:
for metric in scope_metrics.metrics:
for point in metric.data.data_points:
if isinstance(point, HistogramDataPoint):
self.assertEqual(
point.attributes["http.target"],
expected_target,
)
assertions += 1
elif isinstance(point, NumberDataPoint):
self.assertEqual(
point.attributes["http.target"],
expected_target,
)
assertions += 1
self.assertEqual(assertions, 2)

def test_no_metric_for_websockets(self):
self.scope = {
"type": "websocket",
Expand Down Expand Up @@ -705,6 +736,36 @@ def test_credential_removal(self):
attrs[SpanAttributes.HTTP_URL], "http://httpbin.org/status/200"
)

def test_collect_target_attribute_missing(self):
self.assertIsNone(otel_asgi._collect_target_attribute(self.scope))

def test_collect_target_attribute_fastapi(self):
class TestRoute:
path_format = "/api/users/{user_id}"

self.scope["route"] = TestRoute()
self.assertEqual(
otel_asgi._collect_target_attribute(self.scope),
"/api/users/{user_id}",
)

def test_collect_target_attribute_fastapi_mounted(self):
class TestRoute:
path_format = "/users/{user_id}"

self.scope["route"] = TestRoute()
self.scope["root_path"] = "/api/v2"
self.assertEqual(
otel_asgi._collect_target_attribute(self.scope),
"/api/v2/users/{user_id}",
)

def test_collect_target_attribute_fastapi_starlette_invalid(self):
self.scope["route"] = object()
self.assertIsNone(
otel_asgi._collect_target_attribute(self.scope), None
)


class TestWrappedApplication(AsgiTestBase):
def test_mark_span_internal_in_presence_of_span_from_other_framework(self):
Expand Down

0 comments on commit 5468671

Please sign in to comment.