feat(telemetry): trace database queries as children of request spans - #822
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🔐 Codex Security Review
Review SummaryOverall Risk: NONE FindingsNo findings. NotesReviewed Generated by Codex Security Review | |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ankitgoswami
approved these changes
Jul 28, 2026
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.
Reviewable diff: +154/-92 across 18 files (excludes generated, test, and story files).
Summary
Extends the request tracing shipped in #813 down to the database layer: every sqlc query now emits an OpenTelemetry client span (
db.<MethodName>) nested under the incoming request span, so a single trace in Jaeger/Datadog shows which queries a request ran, how long each took, and which one failed. Queries on a context without an active span (background pollers, MQTT ingest) are deliberately not traced, so no new root traces are created.How it works
The
otelhttpmiddleware already puts the request span on the context, and that context is threaded through handlers → services → stores → sqlc. The newdb.NewTracingQuerierdecorator plugs into the generatedQueryRetrierseam (the same interceptor seamfailoverResetRetrieruses), which hands it the context plus the sqlc method name for all ~700 query methods. For each call it starts aCLIENTspan named after the method withdb.system=postgresql/db.operation.nameattributes, runs the query, and records the outcome: errors mark the span failed (exceptsql.ErrNoRows, an expected lookup outcome), and a panic unwinding through the deferredEndmarks the spanquery panickedinstead of exporting a success-looking span.The decorator is installed at every place a query handle is built, always outermost so one span covers all retry attempts:
NewFailoverResettingQuerier— the handle every SQL store usesNewPreparedQuerier— the prepared-statement handle (MQTT path)executeTransaction— the tx-scoped handle passed toWithTransactionactionsauthz.Service— previously built an ad-hoc untraced handle forListRoles; now holds the canonical failover-reset + retry + tracing stackTo let the traced handle flow through transactions,
WithTransaction/WithTransactionNoResultactions now take thesqlc.Querierinterface instead of the concrete*sqlc.Queries— that mechanical signature change is the bulk of the diff.Diagrams
flowchart TD A["otelhttp middleware<br/>(request span in ctx)"] --> B["Connect handler / service"] B --> C["store method (ctx)"] C --> D["NewTracingQuerier<br/>span: db.<Method>"] D --> E["retry / failover-reset querier"] E --> F["sqlc.Queries"] F --> G["RetryDB / *sql.Tx"] G --> H[("PostgreSQL")] B2["WithTransaction action"] --> D2["NewTracingQuerier"] D2 --> F2["sqlc.Queries (tx-bound)"] F2 --> GsequenceDiagram participant I as tracingInterceptor participant Q as query fn I->>I: ctx has valid span context? alt no (background poll, telemetry off) I->>Q: run query untraced else yes I->>I: start span db.Method (CLIENT) I->>Q: run query alt error (not ErrNoRows) I->>I: status Error (SQLSTATE code only) else panic I->>I: status Error "query panicked" end I->>I: end span endAreas of the code involved
server/internal/infrastructure/db/tracing_querier.go(new)NewTracingQuerierconstructorserver/internal/infrastructure/db/{failover_reset_querier,prepared_querier,with_transaction}.goWithTransactionaction signature*sqlc.Queries→sqlc.Querierserver/internal/domain/authz/service.gosqlc.New(s.conn)inListRolesserver/internal/{domain,infrastructure,handlers,testutil}/**(~20 files)sqlc.Querierserver/internal/infrastructure/db/tracing_querier_test.go(new)Key technical decisions & trade-offs
QueryRetrierseam instead of addingotelsql/otelpgxdriver-level instrumentation: method-name span granularity, zero new dependencies, and spans cover row scanning — at the cost of not covering raw-SQL paths outside sqlc.WithTransactionactions tosqlc.Querierinstead of wrapping the txDBTX: uniform span fidelity (aDBTXwrapper can't seeQueryRowerrors or row-iteration time) in exchange for a mechanical, compile-checked signature sweep.sql.ErrNoRowsis not a span failure — it's the normal not-found outcome for:onelookups and would otherwise pollute error-rate views.timescaledb/telemetry_store.go,metrics/store.go— span-per-row would flood the exporter) and the transaction itself (no begin/commit span yet). Failed spans export only a SQLSTATE code (SQLSTATE 23505) orquery failedas the status description, nevererr.Error()— raw Postgres messages can embed user-supplied values (e.g. unique-key DETAIL), which belong in server logs, not the tracing backend. Trade-off: correlating a specific span to its exact error means jumping to logs by trace/time.Testing & validation
just lintgreen (buf, eslint, golangci-lint — 0 issues).go build ./.../go vet ./...clean after rebasing onto current main.ErrNoRows, the parentless skip, panic marking, and an end-to-end call through the generated decorator; db package also run with-shuffle=onand-race.DB_PASSWORD=fleet,-p 1):infrastructure/db,authz,command,sqlstores(rerun on this base), plusapikey,handlers/{apikey,command,onboarding},timescaledb,metricsearlier on the same diff — all pass./code-review max --fixran over the diff; its fixes (parentless skip, panic status, authz querier) are included.🤖 Generated with Claude Code