Skip to content

fix(edge-otel): bounded DB span names, parameterised db.query.text, http.route + error.type - #682

Merged
dash0-dev[bot] merged 11 commits into
mainfrom
fix/edge-otel-db-span-cardinality
Oct 3, 2026
Merged

dash0-dev[bot] merged 11 commits into
mainfrom
fix/edge-otel-db-span-cardinality

Conversation

@dash0-dev

@dash0-dev dash0-dev Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Why

Edge DB spans were named after the full statement with filter values interpolated, so api emitted 511 distinct span names in one week. 287 of them carried full-text search terms (built from agents' tool-failure text, which the CLI keeps out of its own telemetry), 133 carried memory keys, and 4 carried user ids. This was recommendation #1 in docs/telemetry-quality-review.md.

What changed

  • DB CLIENT spans are named by db.query.summary (SELECT memories, CALL memory_write). db.query.text is parameterised ($n, LIMIT included) and renders a mutation's .select() as RETURNING.
  • error.message on every span has request URLs redacted, so a failed request no longer copies its URL and the filter values in it into telemetry. A postgrest-js fetch failure (status: 0) on a DB span records fetch failed.
  • DB spans add server.address (the hosted Supabase host; omitted for BYOD) and db.response.status_code. Root SERVER spans get http.route. Every recorded error gets a bounded error.type (an Error's name, a PostgREST code, or _OTHER), and an unexplained 5xx falls back to the status code.
  • Also fixes the Evals preflight. The latest Claude Code CLI ships default-on built-in plugins, which the harness's empty enabledPlugins couldn't switch off (packages/evals, one commit, can be cherry-picked out).

How to verify

cd packages/mcp-core && npx vitest run and node scripts/ci/deno-check-functions.mjs. Telemetry was verified on this PR's CI integration-smoke traces in Dash0: 57 span names per run, stable across runs, and no quoted literal in any statement.

Notes for reviewers

With http.route set, Dash0 names api operations by route (POST /mcp) instead of by function. Move the two dash0_operation_name="mcp" check rules to faas_name="mcp" before merging, or the absence rule will fire about 2h after deploy (see docs/otel.md). llms.txt and the user docs are unchanged because this is internal telemetry.

mthines and others added 4 commits October 2, 2026 12:17
… add server.address

DB CLIENT spans were named after the full statement with filter values
interpolated, so the api service emitted 511 distinct span names in a week
(full-text search terms, memory keys, user ids). Spans are now named by
db.query.summary (SELECT memories / CALL memory_write), db.query.text uses
$n placeholders, and server.address names the hosted Supabase host.

Root SERVER spans gain http.route (router-matched and single-route
functions) and every error()/clientError() call records a bounded
error.type.

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…he operation rename

The PR's own CI smoke trace showed two things the unit tests could not:
UPDATE ... .select('id,scope,key') rendered the returned columns as the SET
list, and Dash0 names root-span operations by http.route once it is present
(POST /mcp), not by faas.name. Fix the first; document the second and the
check rules that must move to faas_name before deploy.

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
@dash0-dev

dash0-dev Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

✅ No issues found

The delta since 1540895 is docs plus one comment, and it closes the last open thread: catch-block guidance now names errorTypeOf. A full deep re-pass found nothing above the bar; three low-confidence notes are listed as advisory.

3 advisory findings below the confidence bar — see Less certain below.

Checked: 28 of 29 changed files read · 5 possible issues → 0 confirmed → 0 posted

Less certain (3) — advisory, below the confidence bar
  • supabase/functions/_shared/telemetry/otel.ts:1087 — suggestion: Span.error coerces with String() because a throw would break the request, but this call does not. A non-2xx JSON body without message makes .replace throw and the span is lost. (confidence 76)
  • supabase/functions/_shared/telemetry/otel.ts:1103 — suggestion: A status-0 failure has no code, so error.type becomes PostgrestError though PostgREST never answered. The bounded TypeError: / AbortError: prefix of its message is dropped. (confidence 86)
  • supabase/functions/_shared/api/auth.ts:82 — suggestion: This span and mcp/auth.ts:178 keep the statement as their name and set no db.query.summary or server.address, so they fall outside summary-grouped views. (confidence 80)
Review details
Gate Status Details
Description vs. code ✅ Matches the diff: summary-named DB spans, $n statements, URL redaction, http.route + error.type, evals fix.
Prior review feedback ✅ Earlier review comments are resolved.
Documentation ✅ otel.md, CLAUDE.md, key-files and docblocks cover naming, placeholders, redaction, http.route, error.type.
Self-review signals ✅ No debug logs, leftover TODOs, or unreviewed stubs.
Code review ✅ The multi-lens review found no blocking issues.

Found

Quality — produced 5 → posted inline 0 · cleared 0 · carried forward 0 · deferred 0 · below-bar 3
Optimality — ran — 0 proposals; one redaction choke point in Span.error plus the pure, import-free span-semconv module is the optimal shape for the stated intent
Standards — ran — 7 governing docs, 137 normative bullets checked against all 29 files; 0 findings (1 candidate contradicted: webhook.ts catch only ever sees Error instances)

Run

full · 9 lines in delta · tier deep · depth checkout
⚠️ holistic, optimality, measurability and confidence lenses ran from the pipeline rule summaries, not their SKILL.md; span-semconv.spec.ts skimmed (index + 1 block) — ~970 of 1,399 changed lines read — no sub-agent dispatch available — the intent finder ran in-context with the other finders at effective thoroughness 0.95, instead of as its own sub-agent
Memories — 35 indexed · 7 used (4 knowledge · 1 hotspot · 2 lessons)

  • knowledge knowledge::createTracedClient@supabase/functions/_shared/telemetry/otel.ts — re-verified at a54351c: DB-arm naming, status-0 'fetch failed' and explicit code type unchanged
  • knowledge knowledge::errorTypeOf@supabase/functions/_shared/telemetry/span-semconv.ts — re-verified against postgrest-js 2.110.8: result.error is a plain JSON object, so the code fallback holds
  • knowledge knowledge::errorTypeFrom@supabase/functions/_shared/telemetry/span-semconv.ts — re-verified at a54351c: an absent type falls back to the Name: prefix, so a code-less DB error records PostgrestError
  • knowledge knowledge::redactUrls@supabase/functions/_shared/telemetry/span-semconv.ts — re-verified at a54351c: http(s)-only by design, prior scheme finding stays contradicted
  • hotspot hotspot::supabase/functions/_shared/telemetry/otel.ts — 6 confirmed here on this PR; error-path and missing-update checklist for the DB arm
  • lesson telemetry-privacy::anchor-a-credential-strip-on-any-scheme-not-https — checked redactUrls' literal scheme; edge I/O is https-only, so no finding
  • lesson otel::omit-the-attribute-rather-than-emit-a-placeholder-outside-the-semconv-enum — error.type _OTHER is the convention's own fallback value, not an invented placeholder

Nothing to report — measurability (17 paths classified), integrations (not activated), severity, 0 files skipped.

pr-reviewer · commit a54351c · full review · how these findings are produced · updated Oct 2, 2026 3:51pm UTC

mthines and others added 2 commits October 2, 2026 13:13
…interpolates filter values

Addresses the pr-reviewer advisory on #682: the token-lookup guards stay, but
their reason is now the rejection arm's error message, not the span name.

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…clean room

Evals preflight failed on three heads of this PR with "CONTAMINATED -
plugins-loaded: cc-plugin-agents-md, cc-plugin-plugin-authoring" on a
runner that has no ~/.claude. evals.yml installs @anthropic-ai/claude-code
unpinned; 2.1.287 ships built-in plugins, some on by default (2.1.280, the
latest at the last green preflight, ships none), and the harness's
`enabledPlugins: {}` override sets nothing, so they load.

Set `"<name>@Builtin": false` for each built-in instead. The
environment check is unchanged: any plugin that still loads fails preflight.
Reproduced locally with claude 2.1.287: the old override reports the same
two plugins, and the new one reports clean with the harness's SessionStart
hook still firing.

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
Comment thread supabase/functions/_shared/telemetry/otel.ts Outdated
postgrest-js resolves a failed fetch (status 0) with the request URL, filter
values included, in result.error.message, and the DB span copied it into
error.message on every traced query. Record 'fetch failed' for status 0 and
redact URLs from every other recorded message; correct the docs and comments
that blamed only the rejection arm.

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
Comment thread supabase/functions/CLAUDE.md Outdated
mthines and others added 2 commits October 2, 2026 14:07
Error messages are URL-redacted, not value-free: Postgres's own wording can
still echo a value. Say exactly that, and list redactUrls in key-files.

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…isely

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
Comment thread supabase/functions/_shared/telemetry/otel.ts Outdated
Comment thread supabase/functions/memories/index.ts Outdated
… error objects by code

Two pr-reviewer findings on #682: handler and root spans still recorded a
failed request's raw message (URL and search terms included), and a rethrown
PostgREST error object (no name) produced error.type 'undefined'. Span.error
and clientError now redact URLs and tolerate a non-string message; catch
blocks pass errorTypeOf(e), which falls back to the object's code.

Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
Comment thread supabase/functions/_shared/telemetry/otel.ts Outdated
Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
@dash0-dev
dash0-dev Bot marked this pull request as ready for review October 3, 2026 06:58
@dash0-dev
dash0-dev Bot requested a review from mthines as a code owner October 3, 2026 06:58
@dash0-dev
dash0-dev Bot merged commit 88c995e into main Oct 3, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant