Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ See [otel.md](./otel.md) for the full setup. Every layer emits telemetry to Dash

| Signal source | What's emitted |
|---------------|----------------|
| Edge Function (Deno) | `lorekit.memory.*` spans, `lorekit.webhook.github` spans, DB child spans named by SQL statement |
| Edge Function (Deno) | `lorekit.memory.*` spans, `lorekit.webhook.github` spans, DB child spans named by `db.query.summary` (`SELECT memories`) with a parameterised `db.query.text` |
| Next.js server | HTTP server spans via `@vercel/otel` |
| Browser (RUM) | Page loads, navigation, fetch traces, errors via `@dash0/sdk-web` |

Expand Down
1 change: 1 addition & 0 deletions docs/key-files.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ else. Grouped loosely by subsystem in migration/PR order.
| `supabase/functions/mcp/index.ts` | Self-contained Deno MCP server (production) |
| `supabase/functions/_shared/telemetry/otel.ts` | Reusable OTel for Edge Functions: `traceRequest()`, `createTracedClient()`, and the SINGLE source of the OTLP resource attributes (`buildResourceAttributes`), endpoint/dataset resolution (`getOtlpConfig`) and attribute encoding (`toOtlpValue`) that BOTH the span and metric exporters use — a second copy of any of them puts a metric on a different resource than the spans beside it, and the two silently stop correlating. Also stamps the self-time attribution (`lorekit.io.wait_ms` / `lorekit.io.calls` / `lorekit.self_time_ms`) on every root request span |
| `supabase/functions/_shared/telemetry/otlp-metrics.ts` | The metric half of the edge exporter — `buildOtlpMetricsPayload` + `exportMetrics`, POSTing `resourceMetrics` to `/v1/metrics`. Cumulative monotonic sums ONLY, because that is the shape `pg_stat_statements` counters already have: handing them over cumulative (with `stats_reset` as `startTimeUnixNano`) lets Dash0 own both `rate()` and reset detection, where computing deltas here would mean persisting a snapshot and re-implementing reset detection. Unlike `ExportBatch.flush()` it AWAITS and REPORTS the outcome — a swallowed failure here would be a cron job answering `200` while nothing reaches Dash0 |
| `supabase/functions/_shared/telemetry/span-semconv.ts` | Pure, import-free semconv rendering for edge spans: `querySummary` (the DB span name), `renderStatement` / `conditionSql` / `postgrestLogicToSql` (a parameterised `db.query.text` — no filter value reaches the span name or the statement; an unparseable `.or()` string collapses to `(?)`), `redactUrls` (strips request URLs, and the filter values in their query strings, from a recorded `error.message`; Postgres's own wording can still echo a value), `resolveServerAddress` (never names a BYOD host), `errorTypeFrom` / `errorTypeOf` (the bounded `error.type` — catch blocks pass `errorTypeOf(e)`, which falls back to a PostgREST error object's `code`), `httpRouteFor`. Executed by `packages/mcp-core/src/telemetry/span-semconv.spec.ts` |
| `packages/mcp-core/src/telemetry/io-ledger.ts` | Pure `mergeBusyMs` / `attributeIoTime` — the self-time split behind `lorekit.self_time_ms`. The closest thing to a CPU profile a managed Deno isolate can produce (no host for Dash0's eBPF profiler, no userland V8 profiler, and OTLP profiles have no JS exporter). Intervals are MERGED, never summed: handlers issue concurrent queries, so adding two overlapping 40 ms calls claims 80 ms of wait in a request that waited 40 and drives self time negative on exactly the requests worth profiling. Mirrored into `supabase/functions/_shared/telemetry/io-ledger.ts`, guarded by `edge-parity.spec.ts` |
| `packages/mcp-core/src/telemetry/db-query-metrics.ts` | Pure `buildDbQueryMetrics` — `pg_stat_statements` rows → the three cumulative sums (`lorekit.db.query.time` in **seconds**, `.calls`, `.rows`). Holds the decisions that are silent when wrong: the ms→s conversion, the EPOCH (not `now`) fallback for a never-reset counter — `startTimeMs === timeMs` is a zero-length series a backend divides by zero over — `queryid` kept as a STRING (int64), and the bounded attribute set with deliberately NO tenant dimension. Mirrored into `supabase/functions/_shared/telemetry/db-query-metrics.ts`, guarded by `edge-parity.spec.ts` |
| `supabase/functions/profiling/index.ts` | Query-level profiling export: read `lorekit_db_query_stats()` → map → POST as OTLP metrics. SERVICE-ROLE only (the counters are the whole cluster's, so an `lk_ro_*` tenant token is refused too) and deliberately NOT behind `createRouter` — a cron poke is not a tenant request and must not land in `usage_events`. Distinguishes `503` (nothing to send / nowhere to send it) from `502` (Dash0 rejected it), because a `200` on either is the silent failure the endpoint exists to prevent |
Expand Down
75 changes: 69 additions & 6 deletions docs/otel.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LoreKit emits traces, metrics, and logs to Dash0 from every layer of the stack.

| Layer | SDK | Signals |
|-------|-----|---------|
| Edge Function (Deno) | Lightweight OTLP/JSON via `fetch()` | Traces per tool call + webhook; DB child spans named by SQL statement; self-time attribution on every root span; Postgres query-cost metrics (opt-in) |
| Edge Function (Deno) | Lightweight OTLP/JSON via `fetch()` | Traces per tool call + webhook; DB child spans named by `db.query.summary` (`SELECT memories`) with a parameterised `db.query.text`; `http.route` + `error.type` on root spans; self-time attribution on every root span; Postgres query-cost metrics (opt-in) |
| Next.js server | `@vercel/otel` | HTTP server spans, Supabase query spans, custom INTERNAL spans for every mutating server action |
| Browser (RUM) | `@dash0/sdk-web` | Page loads, navigation, Web Vitals, fetch tracing, errors, sessions |
| CLI (`@lorekit/cli`) | Lightweight OTLP/JSON via `fetch()` (zero-dep, no SDK) | One span + one counter point per human-facing command (`install` / `uninstall` / `doctor` / `list` / `search` / `show` / `stats` / `scopes` / `diff` / `tree` / `lint` / `dedupe` / `obligations` / `link` / `migrate`) |
Expand All @@ -33,7 +33,7 @@ Every `tools/call` invocation produces a trace tree:

```
lorekit.memory.write (INTERNAL — tool dispatch)
└── UPSERT INTO memories WHERE ... (CLIENT — Postgres, db.query.text set)
└── CALL memory_write (CLIENT — Postgres; parameterised db.query.text)
```

Attributes on `lorekit.memory.*` spans:
Expand Down Expand Up @@ -585,15 +585,78 @@ Users can read their own rows (self-service "my usage" view via RLS).

---

DB child spans carry OTel database semconv:
DB child spans carry OTel database semconv. The span is NAMED by
`db.query.summary`; no filter value reaches the name or the statement, and a
failure's `error.message` has URLs redacted (see "Error messages" below):

| Attribute | Example |
|-----------|---------|
| span name = `db.query.summary` | `SELECT memories` / `CALL memory_write` — bounded: one per operation × table/function |
| `db.system` | `postgresql` |
| `db.operation.name` | `SELECT` / `INSERT` |
| `db.collection.name` | `memories` |
| `db.query.text` | `SELECT key,value FROM memories WHERE scope = '...' LIMIT 50` |
| `db.operation.name` | `SELECT` / `INSERT` / `RPC` |
| `db.collection.name` | `memories` (the function name for an RPC) |
| `db.query.text` | `SELECT key,value FROM memories WHERE scope = $1 AND archived_at IS NULL LIMIT $2` — values are `$n` placeholders, `LIMIT` included |
| `server.address` | `pqokxlhvnosogizsjztg.supabase.co` — omitted for a BYOD client, whose host is the user's own project |
| `db.response.rows` | `7` |
| `db.response.status_code` | `23505` — the SQLSTATE / PostgREST code whenever PostgREST returns an error object, including `PGRST116` (a `.single()` with no row, which is not marked as an error); the span's `error.type` when it is one |

**Why the values are gone.** The span used to be named after the statement with
every filter value interpolated (`… WHERE key = 'reviewer-lessons::…'`,
`… to_tsquery('zsh glob no matches …')`, `… user_id = '6e05…'`). In the week
before the change that produced **511 distinct span names** on `api` — 287
carrying full-text search terms (which the CLI hook distils from an agent's
tool-failure text and keeps out of its own telemetry), 133 memory keys, 4 user
ids. A span name is a grouping key; and `db.query.text` is defined as the
sanitised statement. The rendering lives in the pure
`_shared/telemetry/span-semconv.ts`: identifiers from our code are kept, every
value is a placeholder, and a `.or()` logic string it cannot parse collapses to
`(?)` rather than being guessed at. `span-semconv.spec.ts` runs a real
`createTracedClient` chain with sentinel values and asserts none reaches the
exported span.

**Error messages.** Every span's `error()` / `clientError()` runs its message
through `redactUrls`, so a handler or root span that records a failed request's
message cannot carry its URL either. A failed DB span records PostgREST's error
text as `error.message`. A failed *request* is the dangerous case: postgrest-js does not
reject on a network failure, it **resolves** with `status: 0` and the request URL
— every filter value in its query string — in the message. That case records
`PostgrestError: fetch failed`; every other message (including a thrown error on
the rejection arm) is kept with URLs replaced by `<url>` (`redactUrls`). What is
left is Postgres's own wording, which can still echo a value (e.g. `invalid input
syntax for type uuid: "…"`, or the slug in `unknown_org: <slug>`). That residue
is why the token lookups in `mcp/auth.ts` / `_shared/api/auth.ts` stay off
`createTracedClient` as defence in depth (`mcp-auth-tracing.spec.ts`,
`rest-auth-tracing.spec.ts`).

### Root request spans: `http.route` and `error.type`

| Attribute | Example | Set by |
|-----------|---------|--------|
| `http.route` | `/memories/:id`, `/mcp`, `/health` | `createRouter` once a route MATCHES (so a 403 carries it; a 404/405 does not), and each single-route function at the top of its `traceRequest` callback. Same shape as `url.path`, template not value. The router's handler child span keeps its function-relative `http.route` (`/:id`) |
| `error.type` | `UserInputError`, `MethodNotFound`, `missing_token`, `TypeError`, `23505`, `503` | Every `span.error()` / `span.clientError()` — an explicit type, else the `Name:` prefix of the message (`errorTypeFrom`), else `_OTHER`. Catch blocks pass `errorTypeOf(e)`: an `Error`'s name, else a PostgREST error object's `code` (those objects have no `name`, so `` `${e.name}: …` `` would read `undefined`). A 5xx response with nothing more specific records the status code. Never the message itself — that stays on `error.message`, with URLs redacted on every span |

**`http.route` renames the Dash0 operation.** With it present, Dash0 names a
root span's operation `{method} {route}` — `POST /mcp`, `GET /memories/:id` —
where it used to be the function (`mcp`, `memories`, from `faas.name`). That was
observed on this change's own CI smoke trace, against what the operation-rule
docs' table order suggests. A request that matches no route (a 404, a discovery
probe) keeps the function-named operation.

So: per-endpoint RED now comes for free in Dash0's operation views, and anything
that must address a whole **function** filters on `faas_name`, which is the same
under both namings — never on `dash0_operation_name="mcp"`:

```promql
sum by (faas_name, http_route) (increase({otel_metric_name="dash0.spans", service_name="api", service_namespace="lorekit", otel_span_kind="SERVER"}[1h]))
```

The two check rules that filtered `dash0_operation_name="mcp"` ("API — mcp
operation has stopped sending spans", "API — mcp operation elevated error rate")
must move to `faas_name="mcp", otel_span_kind="SERVER"` **before** this deploys,
or the absence rule fires two hours after the deploy and the error-rate rule
silently stops matching. The `api-health-red` and `error-hotspot-triage`
dashboards group by `dash0_operation_name` and will show routes instead of
functions.

---

Expand Down
11 changes: 8 additions & 3 deletions docs/telemetry-quality-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,12 @@ resource attributes only.

---

## Recommendations (not applied — design/judgment calls)
## Recommendations (design/judgment calls — status noted per item)

1. **`db.query.text` and DB span names inline literal filter values**
1. **`db.query.text` and DB span names inline literal filter values** —
**applied** (2026-10): spans are named by `db.query.summary`, the statement
is parameterised, see [otel.md](./otel.md) → DB child spans. Original
finding kept below for the record.
(`_shared/telemetry/otel.ts` `buildSql`). Span names like
`SELECT ... FROM memories WHERE scope = 'repo::acme/x' AND key = '...'` are
**high-cardinality** (an anti-pattern for span names, which should group) and
Expand All @@ -155,7 +158,9 @@ resource attributes only.
the span. This is a deliberate design change to all edge DB spans, so it is
left for maintainer sign-off rather than auto-applied.

2. **`error.message` is a non-registry attribute.** `Span.error()` /
2. **`error.message` is a non-registry attribute.** — **partly applied**
(2026-10): every `error()` / `clientError()` now also records a bounded
`error.type`; `error.message` is kept so existing queries keep working. `Span.error()` /
`clientError()` set `error.message`; the conventions use `exception.message`
(with `exception.type`) or `error.type`. Also, PostgREST error strings placed
there can carry incidental detail. Recommend migrating to `error.type` +
Expand Down
7 changes: 6 additions & 1 deletion packages/evals/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,12 @@ Two mechanisms now stand against that, and only the second is trusted:

1. **Ask.** `--disable-slash-commands` drops skills and commands,
`--strict-mcp-config` admits only the harness's own server, and a
session-scoped `--settings` file sets `enabledPlugins: {}`.
session-scoped `--settings` file sets `"<name>@builtin": false` in
`enabledPlugins` for each of Claude Code's built-in plugins
(`BUILTIN_PLUGINS` in `src/harness/arm.mjs`). An empty `enabledPlugins: {}`
is not enough: it sets nothing, so a default-on built-in still loads —
on a clean CI runner that was `cc-plugin-agents-md` and
`cc-plugin-plugin-authoring`.
2. **Verify.** `src/grading/environment.mjs` reads the run's own
`{"type":"system","subtype":"init"}` event and its `hook_started` events, and
reports what _actually_ loaded. A rep whose environment is dirty is
Expand Down
41 changes: 37 additions & 4 deletions packages/evals/src/harness/arm.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,44 @@ export async function prepareArm(
};
}

/**
* Claude Code's BUILT-IN plugins, by the name their `<name>@builtin` id uses.
*
* WHY AN EMPTY `enabledPlugins` STOPPED BEING ENOUGH.
* A built-in ships inside the `claude` binary rather than being installed, and
* some are on by default: `{}` sets nothing, so a default-on built-in still
* loads. The unpinned `npm install -g` in evals.yml picked up a release that
* does exactly that (reproduced on 2.1.287; 2.1.280 ships no built-ins), and
* on a GitHub runner with no `~/.claude` at all the session loaded
* `cc-plugin-agents-md` and `cc-plugin-plugin-authoring`, so preflight rightly
* reported the clean room CONTAMINATED. Only an explicit
* `"<name>@builtin": false` switches one off.
*
* The list is every built-in 2.1.287 ships except `sec-default`, which is
* enabled from managed policy only, so a session setting cannot switch it
* either way. It is a request, like every flag here, so a built-in a later
* release adds is not silently trusted: preflight names it in a
* `plugins-loaded` finding, and the fix is to add it below.
*/
export const BUILTIN_PLUGINS = Object.freeze([
"agents-md",
"claude-test",
"diff",
"mermaid",
"mods-guide",
"plugin-authoring",
"responsive-mode",
"telemetry",
"tips",
"you-should-know",
]);

/**
* A session-scoped settings override that switches OFF the developer's enabled
* plugins. Plugins are auto-discovered from user settings and carry skills,
* agents and hooks of their own, so they are a second route by which the
* machine's configuration leaks into a measurement.
* machine's configuration leaks into a measurement. Claude Code's own
* built-ins (`BUILTIN_PLUGINS` above) are a third, and are switched off by name.
*
* `--disable-slash-commands` covers skills and commands; this covers plugins.
* Neither is trusted — `environment.mjs` verifies the result from the run's own
Expand All @@ -243,10 +276,10 @@ export async function prepareArm(
*/
export async function writeSettingsOverride(sandbox) {
const file = path.join(sandbox.root, "claude-settings.json");
await fsp.writeFile(
file,
JSON.stringify({ enabledPlugins: {} }, null, 2) + "\n",
const enabledPlugins = Object.fromEntries(
BUILTIN_PLUGINS.map((name) => [`${name}@builtin`, false]),
);
await fsp.writeFile(file, JSON.stringify({ enabledPlugins }, null, 2) + "\n");
return file;
}

Expand Down
10 changes: 9 additions & 1 deletion packages/evals/test/grading/environment.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,15 @@ test("a prepared arm carries its own launch options", async () => {
assert.equal(arm.settingsPath.startsWith(sandbox.cwd), false);

const settings = JSON.parse(await fsp.readFile(arm.settingsPath, "utf8"));
assert.deepEqual(settings.enabledPlugins, {});
// Every entry switches a plugin OFF; nothing is switched on.
assert.ok(Object.keys(settings.enabledPlugins).length > 0);
assert.ok(
Object.values(settings.enabledPlugins).every((on) => on === false),
);
// The two built-ins that loaded on a clean CI runner when the override was
// `{}`, which sets nothing and so left every default-on built-in running.
assert.equal(settings.enabledPlugins["agents-md@builtin"], false);
assert.equal(settings.enabledPlugins["plugin-authoring@builtin"], false);
// Crucially NOT `hooks: {}` — `--settings` overrides every scope, so that
// would switch off the harness's own SessionStart hook and silently turn
// arm B into arm A.
Expand Down
7 changes: 4 additions & 3 deletions packages/mcp-core/src/auth/rest-auth-tracing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,10 @@ describe('REST auth resolution telemetry (supabase/functions/_shared/api/auth.ts
});

it('never routes the token lookup through createTracedClient', () => {
// createTracedClient interpolates `eq()` values into the span name and
// db.query.text. The value here is the token hash, so tracing this query
// that way would publish a credential into telemetry.
// createTracedClient keeps `eq()` values out of the span name and
// db.query.text, and redacts URLs from a failed request's message — but it
// records DB error messages, which can echo a value in another form. The
// value here is the token hash, so this guard stays as defence in depth.
expect(executable).not.toContain('createTracedClient');
});

Expand Down
Loading
Loading