pydantic / logfire

Uncomplicated Observability for Python and beyond! 🪵🔥
https://logfire.pydantic.dev/docs/
MIT License
2.21k stars 66 forks source link

Web Server Monitoring Dashboard request miscount with FastAPI #533

Open JacobHayes opened 1 month ago

JacobHayes commented 1 month ago

Description

When using the FastAPI instrumentation, the default "Percent of 2xx Requests" chart query matches both the root {HTTP METHOD} {URL} span + the FastAPI arguments inner span. However, the "FastAPI arguments" inner span does not record a http.status_code (naturally) so that row has http_status_code=null, which then doubles count_all_requests, capping percent_2xx_requests at 50%.

Adding and attributes ? 'http.status_code' or and parent_span_id is NULL to the WHERE clause in the first CTE might be the easiest fix.

If this isn't the right place to submit bugs for the console, happy to post elsewhere!

Python, Logfire & OS Versions, related packages (not required)

logfire="1.2.0"
platform="macOS-15.0.1-arm64-arm-64bit"
python="3.12.3 (main, Apr 15 2024, 17:43:11) [Clang 17.0.6 ]"
[related_packages]
requests="2.32.3"
pydantic="2.9.2"
fastapi="0.115.2"
protobuf="4.25.5"
rich="13.9.2"
executing="2.1.0"
opentelemetry-api="1.27.0"
opentelemetry-exporter-otlp-proto-common="1.27.0"
opentelemetry-exporter-otlp-proto-http="1.27.0"
opentelemetry-instrumentation="0.48b0"
opentelemetry-instrumentation-asgi="0.48b0"
opentelemetry-instrumentation-fastapi="0.48b0"
opentelemetry-instrumentation-httpx="0.48b0"
opentelemetry-instrumentation-sqlalchemy="0.48b0"
opentelemetry-instrumentation-system-metrics="0.48b0"
opentelemetry-proto="1.27.0"
opentelemetry-sdk="1.27.0"
opentelemetry-semantic-conventions="0.48b0"
opentelemetry-util-http="0.48b0"
JacobHayes commented 1 month ago

This also seems to be an issue with:

The other charts seem to either filter by attributes ? 'http.status_code' (eg: Requests) or parent_span_id is null (eg: Exceptions).


Adding the filters to the queries might be fine, but alternatively maybe it'd make sense to remove the http.method and http.route attributes from the FastAPI Arguments span (logfire.fastapi) since it'll always(?) be nested within the opentelemetry.instrumentation.fastapi span.