fix: name a DB span for its operation, not "db query" - #14
Merged
Conversation
Core derives a DB span's semantic type by matching the verb inside the span
NAME (`classifyDBType` in internal/content/session.go) and never reads
`db_operation`, which this SDK already sends. Two of the three DB span sites
named every span "db query":
dbapi start_span("db query")
asyncpg start_span("db query")
sqlalchemy start_span(f"db {verb.lower()}") <- already correct
So every statement through sqlite3, psycopg2, mysql, pymysql or asyncpg was
stored as a generic `database_query`, and a SELECT was indistinguishable from a
DELETE in the record — while `db_statement` sat right there carrying the verb.
Only the SQLAlchemy path classified correctly, which is what made this look
like a driver quirk rather than a naming bug.
All three sites now share one `_db_span_name` helper, so the odd one out is the
convention rather than the exception. Core upper-cases the name before matching,
so the verb's case here does not matter; a statement with no readable verb keeps
"db query", the previous name.
Verified end to end against a local stack: five statements through sqlite3 now
store as database_query (CREATE), database_insert, database_select,
database_update and database_delete, where all five were database_query before.
390 tests pass. The 5 failures in tests/instrumentation are unrelated to this
change and reproduce on an unmodified tree — they need asyncpg and pymongo,
which are not installed.
Co-Authored-By: Claude Opus 5 (1M context) <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.
Why
Core derives a DB span's semantic type by matching the verb inside the span name (
classifyDBType,internal/content/session.go) and never readsdb_operation— which this SDK already computes and sends. Two of the three DB span sites hardcoded the name:So every statement through sqlite3, psycopg2, mysql, pymysql or asyncpg stored as a generic
database_query— a SELECT indistinguishable from a DELETE — whiledb_statementsat right there carrying the verb. Only the SQLAlchemy path classified correctly, which made it look like a driver quirk rather than a naming bug.What changed
One
_db_span_name(statement)helper, used at all three sites, so the convention is the rule rather than the exception. Core upper-cases the name before matching, so case here is irrelevant. A statement with no readable verb keeps"db query", the previous name — no behavior change for those.Verification
Five statements through sqlite3 against a local stack:
CREATE TABLE …database_querydatabase_queryINSERT INTO …database_querydatabase_insertSELECT … FROM …database_querydatabase_selectUPDATE … SET …database_querydatabase_updateDELETE FROM …database_querydatabase_delete390 tests pass. The 5 failures under
tests/instrumentationare unrelated and reproduce on an unmodified tree (git stashconfirmed) — they needasyncpgandpymongo, which aren't installed in this environment.Note for reviewers
Found while restoring span instrumentation in openbox-citadel-sdk-python (OpenBox-AI/openbox-citadel-sdk-python#1). Two related observations, not addressed here:
install_dbapipatches the DB-API governance seam but instruments no driver, so nothing constructs aCursorTracerand no SQL raises a span at all.install_redisandinstall_asyncpgpatch their drivers; the DB-API family has no equivalent step. Worked around SDK-side for now.url.full,http.request.method) and the status code never lands in attributes, so the dashboard renders no request link and no status badge — it readsattrs["http.url"]andattrs["http.response.status_code"]. Also worked around SDK-side. Both may belong here instead.🤖 Generated with Claude Code