Repository navigation
feat(server-utils): Set db.operation.name on SQL spans - #25850
Merged
Merged
Conversation
Contributor
size-limit report 📦Compared: 90450a6 (PR test merge using develop at 2f5a2f9) Against: 2f5a2f9 (develop) · base run #64409 ✅ Baseline matches: this diff contains only this PR's changes.
|
Contributor
Optional E2E testsSome optional checks were not successful. They do not block the merge. 5 not successful, 47 successful. See the run.
Successful checks
|
mydea
marked this pull request as ready for review
October 29, 2026 08:07
mydea
requested review from
andreiborza,
chargome,
isaacs and
nicohrubec
and removed request for
a team
October 29, 2026 08:07
nicohrubec
reviewed
Oct 29, 2026
andreiborza
reviewed
Oct 29, 2026
| }; | ||
| } | ||
|
|
||
| // OTel names a batch `BATCH <op>` when every statement shares one operation, plain `BATCH` otherwise. |
Member
There was a problem hiding this comment.
q: Should we add that as an example/rule to the conventions?
Member
Author
There was a problem hiding this comment.
mydea
force-pushed
the
feat/db-operation-name
branch
from
October 30, 2026 08:43
ef3ed82 to
6030ddd
Compare
Member
Author
|
@nicohrubec I took your feedback and actually put this into a dedicated PR stacked on top: #25927 as this would otherwise make it a bit tricky to see what changes where/why. That renames the utility to a more sensible name after the changes, and actually streamlines this a bit so we can mostly spread these query attributes in and have this consistent everywhere! |
mydea
force-pushed
the
feat/db-operation-name
branch
from
October 30, 2026 09:32
6030ddd to
9b71099
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f2129aa. Configure here.
Open
2 tasks done
pg, mysql, mysql2, tedious, neon, Prisma and the Nuxt db0 integration now derive `db.operation.name` from the sanitized query, matching what postgres.js, knex and the document stores already report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… T-SQL's ;WITH The operation of a WITH query is its main statement, not the first DML keyword inside a CTE. Leading semicolons are now stripped by the sanitizer, so T-SQL's `;WITH` queries get an operation name too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The leading keyword skips parentheses that wrap the whole query, so the CTE scan now starts after `WITH` instead of counting them too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mydea
force-pushed
the
feat/db-operation-name
branch
from
October 30, 2026 12:45
5718dfe to
1e9b545
Compare
mydea
added a commit
that referenced
this pull request
Oct 30, 2026
…nsistency (#25927) **Why:** `sanitizeSqlQueryWithSummary` grew a third value with `db.operation.name`, and every SQL integration copied the result into the same three attributes by hand. `getSqlQueryAttributes` returns them keyed by attribute name, so integrations spread it and the keys can't drift. The mysql2 diagnostics-channel subscriber and postgres.js drop their own operation regexes for the shared helper, so their `db.operation.name` keeps the statement's case and resolves CTEs like the other integrations (postgres.js still prefers the server-reported command once a query succeeds). postgres.js also no longer reports the sanitizer's `Unknown SQL Query` placeholder as `db.query.text` when there is no query; its public `requestHook` gets `''` in that case. **How to review:** Start with `utils/sql.ts`, then the Prisma tracing helper. Prisma keeps its own `db.query.text`/`db.operation.name` and only falls back to ours; `db.query.summary` is always ours, so the streamed span name stays dialect-aware across Prisma versions. Prisma 5 only reports `db.statement`, so its spans now also get a sanitized `db.query.text` (see the v5 test). Tedious clears the summary and operation name for procedure calls, as before. **Unsure about:** `@sentry/server-utils` exports the renamed helper, but the package is internal (no SemVer guarantee) and Nuxt pins it exactly, so I didn't keep an alias. Stacked on #25850. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
I noticed this when bumping junior from v10 to v11, that we are not setting these in some of our db instrumentation.
Why: pg, mysql, mysql2, tedious, neon, Prisma and Nuxt's db0 spans had
db.query.summarybut nodb.operation.name, while postgres.js, knex, MongoDB, Redis etc. already set it. That made grouping/alerting by operation inconsistent across DB integrations. The operation is parsed from the sanitized query by a shared helper next to the summary logic, keeping the original case as OTel recommends.How to review: Start with
getSqlOperationNameinutils/sql.ts; the integration changes are one-liners. Neon batches follow the OTel rule (BATCH <op>if all statements share one operation, elseBATCH).Unsure about: For
WITHqueries I report the main statement's keyword (the first one outside the parenthesized CTE bodies) rather thanWITH. Depth counting is safe because it runs on the sanitized query, which has no literals or comments left. tediouscallProcedureis left without an operation. postgres.js still uppercases its value; I didn't migrate it to avoid a behavior change here.