Uh oh!
There was an error while loading. Please reload this page.
feat(webapp): per-client database pool metrics that survive the driver adapter - #4541
feat(webapp): per-client database pool metrics that survive the driver adapter#4541ericallam wants to merge 4 commits into
Conversation
…r adapter Report Prisma pool and query metrics for every configured client (control-plane writer/replica, run-ops writer/replica, legacy writer/replica) instead of only the control-plane writer, tagged with db_client and db_driver attributes. Pool figures come from the authoritative source per driver: pg.Pool (totalCount/idleCount/waitingCount + connect/remove counters) for driver-adapter clients, and the Rust engine metrics for quaint clients. Query counters and duration histograms come from prisma metrics for both. Adds a db.pool.connections.waiting gauge. Stops exporting Prisma metrics from the Prometheus /metrics route; pool observability now lives entirely in the OTel pipeline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KHjfL7qXHia5DTnxi1RePS
|
Observability mapAs of 18/100 over 413 measured of 429 entry points (base 18, no change) What this PR changed FIX FIRST
AUDIT 3 of 50 sensitive mutations record an actor. 47 without one. What the score is made ofThe score and findings here are report-only and never gate the merge. Separately, a required test suite keeps this tool's symbol and route lists in sync with the code they name, and can fail a pull request that renames or removes a symbol they reference, or that adds the first route with a segment they anticipate. Each failure names the list to edit. The rules and their reasons: internal-packages/observability-map/README.md. |
WalkthroughThe change adds shared database metric registration and normalization. Standard Prisma and driver-adapter clients register pool, connection, query, and histogram sources. The tracer collects metrics for every registered client and records client, driver, pool, and waiting-connection observations. The metrics route now serves 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…tribute Label each pool with its full datasource role (control-plane-writer, control-plane-replica, run-ops-writer, run-ops-replica, legacy-run-ops-writer, legacy-run-ops-replica) instead of the generic writer/reader, matching the db.datasource span attribute so metrics and traces correlate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KHjfL7qXHia5DTnxi1RePS
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Back the metrics-source registry with singleton() keyed by clientType, matching the app's other process-wide registries and deduping so a re-evaluated module or a repeated label registers once. Document the removal of prisma_* from the Prometheus /metrics endpoint in the server-changes note. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KHjfL7qXHia5DTnxi1RePS
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
… note Run oxfmt on databaseMetrics.server.ts (code-quality check). Reword the server-changes note to a single user-facing sentence with no infra names, per the release-note guidance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KHjfL7qXHia5DTnxi1RePS
| let json: PrismaMetricsJson | undefined; | ||
| try { | ||
| json = await source.client.$metrics.json(); | ||
| } catch { | ||
| json = undefined; | ||
| } | ||
| return normalizeDatabaseMetrics(source, json); |
There was a problem hiding this comment.
🟡 Database metrics report zeros instead of being skipped when a client's stats read fails
Query and pool figures for a database connection are reported as zero (json = undefined at apps/webapp/app/utils/databaseMetrics.server.ts:139-142, then zero-filled by normalizeDatabaseMetrics) whenever reading that connection's stats fails, so dashboards show phantom drops to zero and cumulative totals appear to reset.
Impact: Monitoring can briefly show a connection pool as empty and query totals as restarting from zero, producing false alerts and misleading rate spikes on the next successful read.
Cumulative counters observed as 0 on a failed $metrics read (behavior change from previous early-return)
Previously the batch observable callback read Prisma metrics once and, on any failure, returned without observing anything — a failed scrape simply produced no data points. Now collectDatabaseClientMetrics swallows the per-client failure (apps/webapp/app/utils/databaseMetrics.server.ts:136-142) and normalizeDatabaseMetrics zero-fills every $metrics-derived value (apps/webapp/app/utils/databaseMetrics.server.ts:81-88, 104-110, 118-123). The callback then unconditionally observes those zeros for the monotonic instruments db.client.queries.total, db.datasource.queries.total, and (for quaint clients) db.pool.connections.opened.total / closed.total (apps/webapp/app/v3/tracer.server.ts:524-527).
For cumulative counters, a value of 0 followed by the real value is interpreted downstream as a counter reset, yielding an increase equal to the entire lifetime total. For quaint clients the pool gauges (open, busy, idle) also read 0, which looks like a dead/empty pool.
A safer shape is to signal "stats unavailable" from the collector (e.g. leave the engine-derived fields undefined) and skip res.observe for those instruments, while still observing the live pg.Pool figures for adapter clients.
Prompt for agents
In apps/webapp/app/utils/databaseMetrics.server.ts, collectDatabaseClientMetrics swallows a failed `source.client.$metrics.json()` and passes `undefined` to normalizeDatabaseMetrics, which zero-fills every engine-derived value (query counters, query gauges, and for quaint clients the pool gauges/counters). The batch observable callback in apps/webapp/app/v3/tracer.server.ts then observes those zeros for cumulative counters (db.client.queries.total, db.datasource.queries.total, db.pool.connections.opened.total/closed.total) and gauges. The previous implementation returned early on failure and observed nothing, so a failed read produced no data points instead of a spurious zero. Consider propagating an 'engine metrics unavailable' signal from the collector (e.g. an `engineMetricsAvailable: boolean` field or leaving the engine-derived fields undefined) so the tracer callback skips observing engine-derived instruments for that client, while still observing live pg.Pool figures for adapter clients. Update the unit tests in databaseMetrics.server.test.ts accordingly.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Order matters, core metrics end with `# EOF`, prisma metrics don't | ||
| const metrics = prismaMetrics + coreMetrics; | ||
| const metrics = await metricsRegister.metrics(); |
There was a problem hiding this comment.
🔍 Removing prisma metrics from /metrics also removes the only Prometheus-side pool visibility
The Prometheus route no longer emits any prisma_* series; pool visibility now exists only through the OTLP metric exporter. Note this is conditional: when INTERNAL_OTEL_METRIC_EXPORTER_ENABLED === "0", setupMetrics() returns the no-op global meter and never calls configurePrismaMetrics (apps/webapp/app/v3/tracer.server.ts:389-392), so such deployments (including self-hosters) lose database pool observability entirely rather than merely relocating it. No in-repo dashboards reference prisma_*, so nothing in this repo breaks.
Was this helpful? React with 👍 or 👎 to provide feedback.
What
Follow-up to #4539. The driver-adapter work is inert until a client flips to the pg driver adapter, but the moment one does, our database observability degrades: the OTel metrics pipeline reads pool stats from Prisma's
$metrics, which is owned by the Rust engine'squaintpool. Under the adapter,pg.Poolowns the pool, so those gauges read zero. The pipeline also only ever scraped a single client (the control-plane writer singleton).This PR makes database metrics driver-agnostic and per-client:
db_clientanddb_driver(quaint|pg-adapter) attributes.db_clientuses our canonical datasource-role labels (control-plane-writer,control-plane-replica,run-ops-writer,run-ops-replica,legacy-run-ops-writer,legacy-run-ops-replica) — the same strings used for thedb.datasourcespan attribute, so a metric and a trace point at the same pool.pg.Pool(totalCount/idleCount/waitingCount, plus cumulative opened/closed fromconnect/removeevents).$metricspool gauges/counters, exactly as before.$metricsfor both drivers (the Rust engine executes queries in both cases).db.pool.connections.waitinggauge (pg.Pool exposes this; quaint reports 0)./metricsroute. Pool observability now lives entirely in the OTel pipeline, per driver, per client.Why
So we can flip any client (including the control-plane writer, the primary desync-fix target) to the driver adapter without losing pool visibility. Existing dashboards keyed on the same metric names keep working; they gain a per-client dimension.
Testing
Unit (
apps/webapp/app/utils/databaseMetrics.server.test.ts): the pure normalizer — quaint reads pool from$metrics; adapter reads pool frompg.Pooland keeps engine query metrics;busynever goes negative; graceful zeroing when$metricsis unavailable (adapter still reports live pool figures).Live smoke test against a prod-shaped local stack: three physically-distinct Postgres DBs (control-plane, run-ops, legacy) behind dual PgBouncers, split mode on, with a mix of adapter and quaint clients. Reading the actual emitted OTel metrics, every pool shows up as its own series:
Confirms: metrics are attributed per pool with the correct driver; adapter pools' figures come from
pg.Pool; and query counters/duration histograms keep incrementing under the pg adapter. Also verified/metrics(Prometheus) now returns zeroprisma_*series while still serving the app's own metrics.pnpm run typecheck --filter webapppasses.Notes
/metrics(Prometheus) no longer includesprisma_*series. Anything scraping that endpoint for Prisma metrics should read the equivalentdb.*metrics from the OTel exporter instead.?schema=gotcha (separate from this PR, worth flagging for rollout): since feat(webapp,database): opt-in per-client Prisma driver adapters #4539 parses?schema=from the DSN and passes{ schema }to the adapter, node-postgres sendssearch_pathas a startup parameter. A transaction-mode PgBouncer rejects that withFATAL: unsupported startup parameter: search_path. Our prod control-plane DSNs use the defaultpublicschema with no?schema=param, so this is latent, but any client we flip to the adapter must not carry?schema=in its DSN (or the pooler needsignore_startup_parameters = search_path).