Repository navigation
Add OpenTelemetry instrument for FastMCP - #2
personage-hub wants to merge 3 commits into
Conversation
personage-hub
left a comment
There was a problem hiding this comment.
Reviewed the branch locally on top of community-of-python#151: just lint clean, pytest 253 passed (26 in test_fastmcp.py). Also checked against FastMCP 3.4 from a real service: http_app() routes are plain Routes (/.well-known/oauth-protected-resource/mcp, /mcp, /health/, /metrics), the matcher resolves them exactly, /mcp/ with a trailing slash and scan paths get no route, and add_middleware after http_app() works.
The design is the right one: hooking http_app() is the only place where the ASGI app exists, the _is_instrumented_by_opentelemetry flag keeps interop with StarletteInstrumentor, exclusions combine settings and env like Litestar does after community-of-python#151, and the starlette instrumentation stays a dev-only dependency. Tests cover every case from the task, including the entry-point autoload and double http_app() calls.
Two small notes inline, neither blocks the merge. From the service side this removes the OpentelemetryConfig mixin, the manual use_instrument() and the instrument_app() call, which is exactly what we wanted.
| ) -> tuple[str, dict[str, str]]: | ||
| method: typing.Final = str(scope.get("method", "HTTP")).strip() | ||
| for route in routes: | ||
| if isinstance(route, (Route, Mount)) and route.matches(scope)[0] == Match.FULL: |
There was a problem hiding this comment.
Mount matches by prefix, so a Mount("", ...) or Mount("/", ...) (static files, a sub-app mounted at the root) would turn every unknown path into http.route="/". Cardinality stays low, so this is not a problem for the SLI, but the route then stops meaning "matched route". Worth either skipping mounts whose path is empty or /, or at least a line in the README. Not blocking.
|
|
||
|
|
||
| if typing.TYPE_CHECKING: | ||
| from fastmcp.server.http import StarletteWithLifespan |
There was a problem hiding this comment.
StarletteWithLifespan is imported from fastmcp.server.http, which is not a public import path, and the extra still allows fastmcp>=2,<4. It is behind TYPE_CHECKING, so runtime is safe, but mypy users on a fastmcp version where the symbol moved would get an error from the installed package. If 2.x is still meant to be supported, a quick check that the name exists there (or typing the hook with Starlette) would settle it.
8493dc5 to
b0eb8b7
Compare
4811965 to
b0f6bd8
Compare
b0eb8b7 to
5f8e27e
Compare
b0f6bd8 to
43ccc29
Compare
5f8e27e to
7564f48
Compare
…ity-of-python#151) LitestarOpenTelemetryInstrumentationMiddleware read exclusions only from OTEL_PYTHON_LITESTAR_EXCLUDED_URLS, so the opentelemetry_exclude_urls setting was ignored for Litestar. Combine define_exclude_urls() with the env-based list (CombinedExcludeList) and build it once instead of per request. define_exclude_urls moves to BaseOpentelemetryInstrument and build_span_name to instruments/opentelemetry_instrument.py so other bootstrappers can reuse them without importing the Litestar module. Co-authored-by: Alexander Niyazov <aleksandr.niiazov@raiffeisen.ru> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
FastMcpSettings now include OpentelemetryConfig, and FastMcpOpentelemetryInstrument wraps every ASGI application returned by http_app() in OpenTelemetryMiddleware. Server spans get http.route from Starlette route matching, exclusions combine opentelemetry_exclude_urls with OTEL_PYTHON_STARLETTE_EXCLUDED_URLS, and applications already instrumented by StarletteInstrumentor are skipped. KwargsFastMCP gets add_http_app_hook, since the ASGI application is created by the user after bootstrap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ethod Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f3abe5c to
bc2cef1
Compare
Motivation
FastMcpBootstrapperalready wires logging, Sentry, Pyroscope, health checks and Prometheus, but has no OpenTelemetry:FastMcpSettingsdoes not includeOpentelemetryConfigand no tracing instrument is registered. FastMCP services need thesame
SERVERspans that Litestar and FastAPI already produce:http.routeand the response statuscode are what availability is computed from in APM. Today services work around this by mixing
OpentelemetryConfigintotheir settings, registering
OpentelemetryInstrumentby hand and callingStarletteInstrumentor().instrument_app()on theresult of
http_app().Autoloaded
StarletteInstrumentor().instrument()does not help: it replacesstarlette.applications.Starlette, but FastMCPsubclasses the original class at import time (
StarletteWithLifespan), so apps fromhttp_app()stay uninstrumented.Stacked on community-of-python#151 (Honor
opentelemetry_exclude_urlsin the Litestar bootstrapper): reusesbuild_span_name,define_exclude_urlsandCombinedExcludeListmoved toinstruments/opentelemetry_instrument.pythere. Only the last commit belongs to this PR.Changes
FastMcpSettingsnow includeOpentelemetryConfig. No new required settings.KwargsFastMCP.http_app()calls the parent and passes the result through hooks registered withadd_http_app_hook().The return type stays
StarletteWithLifespan. This handles the case where theASGI app is created after bootstrap, and it also covers
run(transport="http"), which callshttp_app()internally.FastMcpOpentelemetryInstrumentaddsOpenTelemetryMiddlewareto every created app:http.routecomes fromapp.routes(Route/Mount,Match.FULLonly). Unknown paths produce a method-only spanwith no
http.route, which keeps cardinality low.excluded_urls = CombinedExcludeList(ExcludeList(define_exclude_urls()), get_excluded_urls("STARLETTE"))._is_instrumented_by_opentelemetryare skipped. The flag is set the same wayStarletteInstrumentorsets it, so requests are never traced twice.OpenTelemetryMiddleware(OTEL_SEMCONV_STABILITY_OPT_IN).FastMcpBootstrapper.bootstrap()is now typed as returningKwargsFastMCP(aFastMCPsubclass), soadd_http_app_hooktype-checks.opentelemetry-instrumentation-starletteto the dev group so the entry-point autoload test can run.Tests
tests/bootstrappers/test_fastmcp.py:GET /health/span name,http.routeand status attribute for unset /http/http/dupsemconv/metricsexcluded by default;opentelemetry_exclude_urls;OTEL_PYTHON_STARLETTE_EXCLUDED_URLS;opentelemetry_generate_health_check_spans=FalseGETspan withouthttp.route, status 404POST /mcphashttp.route == "/mcp"opentelemetry_instrumentors(httpx) applied and torn downhttp_app()calls produce two independently instrumented appsjust lint-ciandjust testpass (238 passed).🤖 Generated with Claude Code