chore(observability): add grafana/otel-lgtm to the local dev stack - #431
Open
cteyton wants to merge 18 commits into
Open
chore(observability): add grafana/otel-lgtm to the local dev stack#431cteyton wants to merge 18 commits into
cteyton wants to merge 18 commits into
Conversation
Adds a self-hosted OTLP backend (OTel Collector + Tempo + Prometheus + Loki + Grafana in one container) for local trace analysis, behind an `observability` compose profile so it does not weigh on the default dev stack. Tracing is off unless OTEL_EXPORTER_OTLP_ENDPOINT / VITE_OTEL_EXPORTER_URL are set, which keeps the exporter from retrying against a host that is not running. No custom collector config is mounted: the image already enables CORS on the OTLP/HTTP receiver, which is all the browser exporter needs. The image tag is pinned because its Grafana provisioning and collector config are internal details that change between releases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Adds apps/api/src/otel.ts, starting the OTel NodeSDK with the auto-instrumentation bundle and exporting OTLP. Gives request -> Nest controller -> SQL statement spans, plus Redis and outgoing LLM calls (the OpenAI/Anthropic/Google SDKs use global fetch, which the http instrumentation does not patch - undici does). The SDK starts synchronously at module top level and otel.ts is the first import of instrument.ts. Both matter: auto-instrumentation patches modules as they are required, so anything loaded first is never traced. That is also why otel.ts imports no @packmind/* module - @packmind/logger pulls in winston, and a winston required ahead of the hooks would lose log/trace correlation. Verified on the built bundle: the [otel] line prints before any application log, and Winston records then carry trace_id/span_id. Everything is gated on OTEL_EXPORTER_OTLP_ENDPOINT, which is unset in production and in tests, so the SDK never starts there. pg keeps enhancedDatabaseReporting off, so spans carry parameterized SQL but never bind values, and GenAI message content is left uncaptured - neither user data nor prompts reach the trace backend. OTel packages are added to apps/api/docker-package.json as well, since that file is hand-maintained and a missing entry breaks the production container. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
@opentelemetry/instrumentation-winston injects trace_id, span_id and trace_flags into every record logged inside an active span, so correlation needs no code here. But those three fields landed in the JSON metadata blob of every single console line, drowning the actual payload. Renders a short [trace=xxxxxxxx] marker instead and keeps the three fields out of the blob. The json() format still carries them in full, which is what Loki and Grafana's trace<->logs navigation use. Extracts the printf callback into an exported formatConsoleLine so it can be tested directly: the console transport is unreachable under Jest, which sets PACKMIND_LOG_LEVEL=silent globally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Adds OtelService with a WebTracerProvider exporting OTLP, so a page interaction and the API work it triggers land on a single trace. Both XHR and fetch are instrumented: Axios uses the XHR adapter in the browser, so XHR is what covers packmindApiService, while fetch covers React Router's own requests. Initialization lives in entry.client.tsx rather than root.tsx, where initSentry sits: React Router runs in SPA mode but still prerenders the shell at build time, so root.tsx module scope executes in Node, where window and XMLHttpRequest do not exist. Confirmed on a real build - the prerender logs Sentry's message but not this one, and the OTel code lands only in the entry.client chunk. No CORS or propagateTraceHeaderCorsUrls config is needed: the API is reached through the relative /api path, so requests are same-origin and traceparent is attached by default. Only the exporter POST is cross-origin, which the collector already allows. Gated on VITE_OTEL_EXPORTER_URL, unset by default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Contributor
Greptile SummaryThe PR adds opt-in local OpenTelemetry collection and Grafana dashboards, instruments API traces and logs, and integrates telemetry flushing into API shutdown.
Confidence Score: 4/5The PR is not yet safe to merge because the outstanding shutdown timeout can still terminate the API before its final telemetry export completes. The shutdown path awaits a promise that resolves after two seconds even when Files Needing Attention: apps/api/src/otel.ts, apps/api/src/main.ts Important Files Changed
Reviews (14): Last reviewed commit: "docs(observability): correct the multi-e..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Review feedback on #431: the SIGTERM/SIGINT listeners in otel.ts started sdk.shutdown() without awaiting it, while main.ts's own listener ended its sequence with process.exit - which terminates regardless of an in-flight export. Reproduced against the built bundle with a stub OTLP collector, real postgres and redis: sending SIGTERM inside the BatchSpanProcessor window produced zero exports, and a control run confirmed spans were being generated (a 22KB batch exported once the 5s window elapsed). So it was not a partial race - the entire final batch was lost, every time. Replaces the listeners with an exported shutdownOtel() that main.ts awaits as the last step before process.exit, in both the success and failure paths. Two listeners racing each other cannot be made correct; shutdown ordering belongs where it already lives. The flush is bounded by a 2s timeout so an unreachable collector cannot hold the process past a container's SIGKILL grace, and it never rethrows, so a failed flush cannot change the exit path. Verified after the change: trace export lands 26ms after SIGTERM and the process exits at 205ms; with the collector unreachable, shutdown completes in 2.2s instead of hanging on exporter retries. Also documents the Node runtime metrics the SDK exports by default, which the collector stub revealed and the README did not mention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Review feedback on #431. OTLP ingestion is unauthenticated and Grafana ships on default credentials, while traces carry SQL statements, request URLs and log lines - so publishing on every interface exposed all of it to the local network. Functionally free: the browser reaches the collector over loopback, and the backend reaches it as http://otel-lgtm:4318 over the compose network, which host port bindings do not affect. Confirmed via `docker compose config` that all three now resolve to host_ip 127.0.0.1. Documents the choice in both the compose file and the README, since the ports will look dead to anyone driving Docker from another machine. Note this does not make the dev stack safe on a hostile network: postgres, redis and pgadmin are still published on 0.0.0.0 with weak or absent credentials. Hardening those is a wider change than this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
VITE_OTEL_EXPORTER_URL is baked into the client bundle by Vite and cannot be changed after the image is built, so the Cloud/self-hosted split has to happen at build time. Gates it on the same expression the Sentry and Crisp values already use - proprietary edition AND not a release/* tag - since every release/* tag produces the self-hosted images. Adds a guard that fails the frontend build if any Cloud-only VITE_ value is non-empty on a release/* tag. The failure it prevents is invisible: a leaked endpoint in a customer's bundle would silently point their browsers at Packmind infrastructure, and nothing in the running product would look wrong. It covers the Sentry and Crisp values too, which had no such check. Verified both directions by building the frontend with a sentinel URL: the value reaches the bundle when set, and with the empty value the gate produces the bundle contains VITE_OTEL_EXPORTER_URL:"" and initOtel() returns early. Worth checking because getEnvVar reads import.meta.env with a dynamic key, which Vite can only satisfy by serializing the whole env object. The API needs no equivalent change - OTEL_EXPORTER_OTLP_ENDPOINT is read at runtime, so the image is neutral and self-hosted deployments never start the SDK. Documents that in the self-hosted compose file, and the whole split in docker/otel/README.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Captured real spans against a migrated postgres to check what the pg
instrumentation actually records, and the SQL is not under `db.statement` as
documented - instrumentation-pg has moved to the stable database semantic
conventions, so it is `db.query.text`, alongside `db.system.name` and
`db.namespace`. Anyone following the old name would have concluded the SQL was
not captured at all.
The substance was right: the full TypeORM-generated statement is recorded, and
bind values are not. Confirmed by calling check-email-availability with a real
address and finding only `LOWER("user"."email") = LOWER($1)` in the span, with
the address nowhere in it. Documents that example, since "parameterized" is
easier to trust when you can see what it looks like.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Captured a real trace for one endpoint call and it was close to unreadable: 24 spans, of which 18 were Express middleware and routing noise, mostly named "middleware - patched", burying the four spans that carry the story. Two causes: - Per-middleware spans (cookieParser, jsonParser, cors...) each wrapped in an uninformative parent. Dropped via ignoreLayersType. - Express 5 routes through the standalone `router` package, which the auto bundle instruments separately - so routing was traced twice, contributing 9 opaque spans plus a duplicate of the request-handler span express already emits. Disabled; express covers the same ground. The same request now produces 6 spans reading HTTP -> route -> controller -> use case -> SQL, which is the shape someone new to tracing can actually follow. Declares @opentelemetry/instrumentation-express directly rather than relying on it transitively, since pnpm's isolated node_modules would not resolve it at runtime, and adds it to docker-package.json for the production image. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Verified the correlation end to end by capturing traces and logs from one request and cross-referencing, and found that no log records were being exported at all. The trace_id visible in console output came from the injection half of instrumentation-winston, which made it look like everything worked. Cause: log sending needs @opentelemetry/winston-transport, which is an OPTIONAL peer of instrumentation-winston. Without it the instrumentation sends nothing and only emits an OTel diag warning - invisible unless OTEL_LOG_LEVEL is set. So Loki would have stayed empty, and the Tempo<->Loki click-through that justified enabling log sending in the first place would not have existed. Adds the package to the root dependencies and to docker-package.json, and warns against pruning it in both the code comment and the README. Confirmed after the fix, on a single request: - 5 log records carry the same trace_id as the HTTP span, with span ids belonging to spans in that trace - startup logs correctly carry no trace context - an inbound W3C traceparent is adopted: the server span takes the caller's trace id and parent span id, and the request's log records carry the caller's trace id too - which is the browser -> API -> SQL -> logs path Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Comment on lines
+139
to
+145
| await Promise.race([ | ||
| sdk.shutdown(), | ||
| new Promise<void>((resolve) => { | ||
| // unref so a pending timer cannot itself keep the process alive. | ||
| setTimeout(resolve, SHUTDOWN_TIMEOUT_MS).unref(); | ||
| }), | ||
| ]); |
Contributor
There was a problem hiding this comment.
When the collector is slow or unavailable during shutdown, the two-second timer resolves shutdownOtel() while sdk.shutdown() is still exporting. main.ts then immediately calls process.exit, terminating the in-flight export and dropping the final buffered telemetry batch.
Adds the "show me everything slower than X" entry point, and the distinction
that catches people out: duration filters a single span, traceDuration filters
the whole request, so {duration > 800ms} finds a slow query even inside a fast
request.
Verified against a real Tempo 3.0.3 and Loki 3.7.6 running locally, fed by the
built API through a splitter standing in for the collector. Looking up one trace
id returned its 5 log lines from Loki and all 6 spans from Tempo, and the
duration filters selected exactly the slow traces.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Answers "distribution time with percentiles for each unique method": Tempo's metrics-generator emits traces_spanmetrics_latency_bucket labelled by span_name, so percentiles per endpoint need no extra instrumentation - only a histogram_quantile query. Verified end to end rather than described, by running Tempo 3.0.3 with the span-metrics processor remote-writing to Prometheus 3.13.2 and driving the built API with mixed fast and lock-delayed traffic. Measured, from that run: span p50 p95 p99 POST /../check-email-availability 3.1ms 46.4ms 216.3ms pg.query:SELECT packmind 1.1ms 21.6ms 213.8ms which is also the example the docs use, since the matching p99s show the tail sitting in the query rather than in our code. Records two gotchas found while testing: rate() with no traffic in the window returns NaN, so a quiet environment looks broken when it is not, and unmatched routes collapse into a single bare "POST" span name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Writing PromQL and TraceQL by hand for every question is too much friction for day-to-day use, so the queries are now written once and provisioned. Adds docker/otel/grafana/dashboards/packmind-api.json - latency percentiles per endpoint, request and error rates, a latency heatmap with an endpoint selector, and database call percentiles - plus the provisioning file and the compose mounts. The otel-lgtm image provisions datasources but ships no dashboards, so this fills the gap; datasource uids (prometheus/tempo/loki) are stable in the image, so the panels bind without manual selection. Also documents that the image already installs the Drilldown apps (exploretraces, lokiexplore, metricsdrilldown), which are point-and-click and cover most exploration with no query language at all. The README now leads with those and frames the query sections as the escape hatch rather than the entry point. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Importing a community dashboard from grafana.com is the obvious reflex, and it will usually show empty panels. Span-metric names differ across the ecosystem: this stack's Tempo metrics-generator emits traces_spanmetrics_latency_bucket (confirmed by querying Prometheus directly), while the Collector's spanmetrics connector and Alloy emit traces_spanmetrics_duration_milliseconds_bucket, and older Tempo used traces_spanmetrics_duration_seconds_bucket. Documents the mismatch and the two ways out, so an empty panel does not get read as a broken pipeline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
…ud move
"Just change the endpoint" covers the API and nothing else, so this writes down
what actually has to happen.
Verified: the SDK reads OTEL_EXPORTER_OTLP_HEADERS natively, so authenticating
to Grafana Cloud needs no code change. Confirmed by pointing the built API at an
endpoint requiring auth and observing Authorization: Basic on /v1/traces and
/v1/logs.
Not covered by an endpoint change, and now documented:
- browser traces cannot go direct, since VITE_OTEL_EXPORTER_URL is resolved by
the browser and would expose the Cloud token in a public bundle
- span metrics are off by default in Cloud and billed as active series, so the
RED panels stay empty until enabled
- nothing is sampled and every log line is exported, which is fine on a laptop
and expensive in production
Also replaces the dashboard's hardcoded datasource uid with a ${ds} variable, so
the same JSON works against otel-lgtm and against Cloud, where uids differ.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
Removes the frontend side of the OpenTelemetry work: OtelService, its call in entry.client.tsx, the four browser-only packages, VITE_OTEL_EXPORTER_URL in compose and in both workflows, and the matching documentation. Sending telemetry from a page has no good answer here. The endpoint has to be reachable by the browser, so it either ships a credential in a public bundle or becomes an unauthenticated ingest path on our own domain - and fetching the credential from an API changes nothing, since anything the page can read, anyone can. Neither trade was worth what browser spans add on top of server-side tracing. Keeping it server-side also removes a whole class of problem: the API reads its endpoint at runtime, so the image is neutral and no build-time gate is needed to keep telemetry out of self-hosted images. The CI guard stays for the Sentry and Crisp values, which are still baked into the bundle and had no such check before. Inbound traceparent handling is untouched, so an instrumented caller can still join the trace if browser tracing is ever revisited. Verified: frontend typecheck, lint, 1070 tests and a from-scratch build all pass, and the rebuilt client bundle contains no OpenTelemetry code. API typecheck and build unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
deployment.environment.name used to fall back to NODE_ENV, which the API image hardcodes to production. Staging would therefore have announced itself as production, merging its traces, logs and latency percentiles into the production ones with nothing looking wrong. The environment now has to be declared in OTEL_RESOURCE_ATTRIBUTES, which is where the SDK's own resource detector reads it, so there is a single source of truth and no default that can silently win. With an endpoint set and no environment, the SDK does not start and logs an error. The API still boots and serves traffic in that case: a telemetry misconfiguration must not take the service down, while mislabelled telemetry is worse than none - hence loud, but never fatal. Verified on the built bundle: - endpoint set, environment missing -> API up, error logged, collector receives nothing - endpoint + environment=staging -> exports with deployment.environment.name=staging - no OTEL_* variables at all -> API up, no output, no spurious error Compose defaults the local value so local remains a one-variable opt-in, and the README gains the per-environment table. It also records that the attribute does not reach span metrics, so two environments sharing one Prometheus would blend their percentiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
The previous wording said the environment attribute cannot reach span metrics
and implied separate stacks were the only reliable answer. Grafana Cloud's
metrics-generator can promote span and resource attributes to dimensions, so
deployment.environment.name can become a label on traces_spanmetrics_* on a
single stack - with the caveat that each dimension multiplies active series and
is billed.
Documents both options and when each is the right trade, plus the multi-stack
datasource that queries several stacks at once. Notes that the bundled
dashboard's ${ds} variable is what makes one JSON serve both environments.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s
|
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
2 participants
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.
Adds a self-hosted OTLP backend (OTel Collector + Tempo + Prometheus + Loki +
Grafana in one container) for local trace analysis, behind an
observabilitycompose profile so it does not weigh on the default dev stack.
Tracing is off unless OTEL_EXPORTER_OTLP_ENDPOINT / VITE_OTEL_EXPORTER_URL are
set, which keeps the exporter from retrying against a host that is not running.
No custom collector config is mounted: the image already enables CORS on the
OTLP/HTTP receiver, which is all the browser exporter needs. The image tag is
pinned because its Grafana provisioning and collector config are internal
details that change between releases.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01NJvWXKCQBSLAEps8nZS55s