Repository navigation
feat(database): make the statement span the only span a statement gets - #126
Confidence 6/10 · 2 issues to address
🟡 Confidence 6/10 · needs attention. Held at 6 because a correctness warning is open.
2 warnings −2 · tests covered · risk high −1
Makes the Database statement span the only span a statement gets: it now carries db.query.text and the SQLSTATE, and plain SQL is named from its text. Turning the driver span off by default also drops the peer attributes the ClickHouse client relied on, and the new table heuristic misreads common SQL.
driverSpansopts back into the driver span, which now runs untraced by default- Statement span carries
db.query.textanddb.response.status_code - Plain SQL spans are named from their leading operation and first table
Findings
🟠 Warning · F1 · Statement spans no longer name the peer now the driver span is off
observability · MAP-01 · src/database/database.ts:220
driver() switches the tracer off, so the span that carried the peer is gone: src/clickhouse-http/client.ts:279 puts db.namespace, server.address, server.port and peer.service on that driver span only. Statement spans keep db.system.name alone, so the service-map edge promised in docs/running-queries.md:110 disappears for every Db caller.
Carry the ClickHouse client's `spanAttributes` onto the statement span — an option on `fromSqlClient`, or an `annotateCurrentSpan` from the client's `execute` — so `peer.service`, `server.address` and `db.namespace` survive.
🟠 Warning · F2 · FIRST_TABLE reads a column or a subquery as the table
correctness · src/database/tracing.ts:46-55
FIRST_TABLE takes the first from|into|update|join|table plus a word anywhere, so Db.sqlSELECT extract(epoch from created_at) FROM events`` records db.collection.name: created_at and is named `SELECT created_at`, and `FROM (SELECT id FROM events) t` records `SELECT` — though `docs/database.md:315` says collection is absent over a subquery.
Skip keywords at parenthesis depth above 0 (`extract(epoch from x)`) and a `FROM` followed by `(`, so a column or a subquery never becomes the collection.
What was checked
- Ran
FIRST_TABLE/STRING_LITERALon 16 statements and a 500 KB literal: masking is linear, no backtracking - Postgres
db.query.textkeeps$nplaceholders, not values (src/database/database.test.ts:649) observeannotations land on the statement span sinceguardruns insidetraceStatement(src/database/database.ts:261)
89050ff · Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one. Check ids refer to Maple's instrumentation audit.
Annotations
Check warning on line 220 in src/database/database.ts
maple-review-bot / Maple / review
observability · MAP-01: Statement spans no longer name the peer now the driver span is off
`driver()` switches the tracer off, so the span that carried the peer is gone: `src/clickhouse-http/client.ts:279` puts `db.namespace`, `server.address`, `server.port` and `peer.service` on that driver span only. Statement spans keep `db.system.name` alone, so the service-map edge promised in `docs/running-queries.md:110` disappears for every `Db` caller.
Check warning on line 55 in src/database/tracing.ts
maple-review-bot / Maple / review
correctness: `FIRST_TABLE` reads a column or a subquery as the table
`FIRST_TABLE` takes the first `from|into|update|join|table` plus a word anywhere, so `Db.sql`SELECT extract(epoch from created_at) FROM events`` records `db.collection.name: created_at` and is named `SELECT created_at`, and `FROM (SELECT id FROM events) t` records `SELECT` — though `docs/database.md:315` says collection is absent over a subquery.