Repository navigation
Conversation
Instrument the server with OpenTelemetry tracing, exported over OTLP and disabled unless OTEL_EXPORTER_OTLP_ENDPOINT is set. Health checks and metrics scrapes can be excluded via OTEL_PYTHON_FASTAPI_EXCLUDED_URLS. Add an OpenTelemetry Collector and Jaeger to the example monitoring stack. The Collector receives traces and forwards them to Jaeger, and also scrapes and re-exposes Tiled's Prometheus metrics. Add a 'Distributed Tracing' user-guide page.
Send traces to both Jaeger and Grafana Tempo: the OpenTelemetry Collector now fans traces out to a Tempo service in addition to Jaeger, and Tempo is added as a Grafana datasource so traces can be explored in Grafana with TraceQL. Bump Grafana to a version that supports TraceQL, and add the required apiVersion to the datasource provisioning files. Illustrate the telemetry flow in the tracing docs with a diagram.
Wrap record_timing in an OpenTelemetry span so the phases it already times (access control, read, tokenize, pack) appear as child spans in a request's trace, giving a per-request breakdown of where time is spent. The span is a no-op when OpenTelemetry is not installed or no tracer provider is configured.
Member
|
Ah, the Copilot review request was an errant click. Oh well. |
Member
|
@ZohebShaikh @dylanmcreynolds Feel no obligation, of course, but if you have a moment to give feedback on this (or suggest other reviewers from your institutions) we'd interested in any thoughts. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved issues affect Compose behavior, image compatibility, the advertised Tempo/Grafana setup, and tracing test coverage.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 5
Open (5)
What changed in this PR
Adds OpenTelemetry tracing support, OTLP Collector/Jaeger monitoring configuration, and user documentation.
Changes:
- Adds conditional FastAPI instrumentation and tracing dependencies.
- Adds Collector, Jaeger, and metrics configuration.
- Adds tracing documentation, navigation, and changelog updates.
| File | Description |
|---|---|
tiled/server/app.py |
Configures conditional OpenTelemetry tracing. |
pyproject.toml |
Adds OpenTelemetry dependencies. |
monitoring_example/prometheus/prometheus.yml |
Documents Collector metrics handling. |
monitoring_example/otel-collector/otel-collector.yml |
Defines trace and metrics pipelines. |
docs/source/user-guide/tracing.md |
Documents tracing setup and usage. |
docs/source/_toc.yml |
Adds tracing documentation navigation. |
compose.yml |
Adds tracing environment variables. |
compose.monitoring.yml |
Adds Collector and Jaeger services. |
compose.dev.yml |
Adds tracing environment variables for development. |
CHANGELOG.md |
Records the tracing feature. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
record_timing opened a phase span unconditionally, so requests that are excluded from tracing (health checks, metrics scrapes) produced orphaned single-span traces that cluttered the trace UI. Only open a phase span when there is an active recording span.
FastAPI >=0.142 ships built-in OpenTelemetry support that, when
OTEL_EXPORTER_OTLP_ENDPOINT is set, registers its own OTLP export
pipeline on the global tracer provider. Combined with the tracing
pipeline Tiled configures, this exported every span twice. Pass
telemetry={"auto_configure": False} to FastAPI() so Tiled remains the
sole exporter.
Use an in-memory span exporter to verify, without a running collector or backend: a traced request emits the FastAPI server span and child phase spans (single trace, no duplicate span IDs); excluded endpoints emit no spans; tracing stays off when OTEL_EXPORTER_OTLP_ENDPOINT is unset; and FastAPI's built-in telemetry does not register a second export pipeline.
FastAPI >=0.142 ships its own OpenTelemetry integration that emits request spans on any globally installed tracer provider when the app is not instrumented by opentelemetry-instrumentation-fastapi. The in-memory provider the tests install made test_tracing_disabled_by_default capture those spans and fail on CI. Assert instead that the tracing hook did not instrument the app (its documented off-by-default behavior). Also use single backticks in comments/docstrings.
The base compose.yml and compose.dev.yml set OTEL_EXPORTER_OTLP_ENDPOINT pointing at otel-collector, which is only defined in compose.monitoring.yml. Running the base files on their own therefore enabled tracing against an unreachable host, causing continuous export failures. Move the OTEL_* variables into compose.monitoring.yml, next to the collector they target, so tracing is off unless that overlay is used. Also update the tracing user guide to launch the example with compose.dev.yml (which builds the image from this checkout) instead of compose.yml (whose pinned published image may not include tracing yet).
2 tasks done
Contributor
|
@dan-fernandes From our side Will have a look at this PR. He has worked on this tech before so will be a good person for this |
The FastAPI auto_configure opt-out entry was lost while resolving a CHANGELOG conflict when merging main.
Member
|
That would be excellent! @genematx has some experience with this stack from previous jobs, as does @dylanmcreynolds I believe, but it's brand new to NSLS-II. Feedback from @dan-fernadandes would be very appreciated! |
Member
|
I think it's slick that this slots right into the |
danielballan
reviewed
Oct 8, 2026
danielballan
left a comment
danielballan
left a comment
Member
There was a problem hiding this comment.
This is great. I have two implementation nit-picks.
`opentelemetry` is a namespace package shared by all `opentelemetry-*`
distributions, so `find_spec("opentelemetry")` succeeds even when the
API package (which provides `opentelemetry.trace`) is not installed, and
importing `tiled.server.utils` then fails. Check for `opentelemetry.trace`
itself.
Import `importlib.metadata` explicitly: `import importlib` does not load
the submodule, and `app.py` only worked because another import loaded it.
1 of 2 tasks
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds minimal tracing capabilities with OpenTelemetry.
The traces are exported over OTLP, which is enabled by setting
OTEL_EXPORTER_OTLP_ENDPOINT. Health checks and metrics scrapes can be excluded viaOTEL_PYTHON_FASTAPI_EXCLUDED_URLS.The example monitoring stack (
compose.monitoring.yml) gains an OpenTelemetry Collector and two trace backends, Jaeger and Grafana Tempo. The Collector receives traces and fans them out to both backends so their capabilities can be compared: Jaeger has its own UI, while Tempo is added as a Grafana datasource so traces can be explored in Grafana with TraceQL. The Collector also scrapes and re-exposes Tiled's Prometheus metrics.A new "Distributed Tracing" user-guide page documents how to enable tracing and try the example stack, including a diagram of the telemetry flow.
Related Issue: #1000
Checklist