feat(source/bigquery): attach SQLCommenter attributes as BigQuery job labels - #3843
feat(source/bigquery): attach SQLCommenter attributes as BigQuery job labels#3843spark2ignite wants to merge 3 commits into
Conversation
BigQuery does not retain SQL-text comments in job metadata, so instead of prepending a comment the SQLCommenter attributes (tool.name, client, client.model, client.user.id, client.agent.id, traceparent, server, db.system.name) are attached as native job labels on queries executed through RunSQL. Labels surface in INFORMATION_SCHEMA.JOBS, audit logs, and billing exports without query text parsing. Keys and values are sanitized to BigQuery label constraints (lowercase [a-z0-9_-], max 63 chars, keys start with a letter); explicit tool-supplied labels win on collisions. Gated exactly like other sources: the global --sql-commenter flag plus an optional per-source sqlCommenter override.
There was a problem hiding this comment.
Code Review
This pull request introduces SQL Commenter support for the BigQuery source. Because BigQuery does not retain SQL-text comments in its logs, SQL Commenter attributes are attached as native job labels instead. The implementation includes sanitization logic to satisfy BigQuery's label constraints, a helper to merge explicit and commenter-derived labels, configuration updates, and comprehensive unit tests and documentation. No review comments were provided, so there is no additional feedback.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for SQL Commenter in BigQuery by attaching telemetry attributes as native job labels instead of SQL-text comments, updating the configuration, documentation, and adding sanitization logic for BigQuery label constraints. The review feedback highlights a potential issue where sanitized label values starting with an underscore or dash would be rejected by BigQuery, and suggests prefixing them with 'x' along with adding corresponding test cases.
Label keys must begin with a lowercase letter, but values carry no leading-character requirement — BigQuery accepts values starting with an underscore, dash, or digit. Assert that the sanitizer preserves such values rather than prefixing them.
Description
Enabling
--sql-commentercurrently has no effect on the BigQuery source:the commenter is only wired into postgres/mysql-family sources, and BigQuery
would not retain SQL-text comments in job metadata anyway. As noted on the
issue, today exactly one attribute reaches the job — the automatic
mcp-toolbox-toollabel added in v1.5.0 (#1975), which carries the tooltype (e.g.
bigquery-execute-sql). The seven remaining SQLCommenterattributes — per-user, per-agent, per-model attribution and the trace
linkage — never reach the job. This PR adds them. The SQLCommenter
tool.nameattribute (the MCP tool name, e.g.execute_sql) is distinctfrom the existing type label; both are kept, and explicit tool-supplied
labels always win on key collisions.
Following the direction discussed in the linked issue, this PR attaches the
SQLCommenter attributes as native job labels instead of prepending a
comment, so they surface first-class in
INFORMATION_SCHEMA.JOBS, auditlogs, and billing exports — no query text parsing needed to recover them.
sqlcommenter.Labels(...)returns the exact attribute setPrependCommentuses (tool.name,client,client.model,client.user.id,client.agent.id,traceparent,server,db.system.name), sanitized to BigQuery label constraints: lowercase[a-z0-9_-], max 63 chars, keys start with a letter, dots mapped tounderscores (
tool.name→tool_name).Source.RunSQL, so bothbigquery-execute-sqlandbigquery-sqlare covered with no per-tool changes; ML/AI tools that don'tflow through
RunSQLare unaffected, matching the scope note on the issue.Explicit tool-supplied labels (e.g.
mcp-toolbox-tool) win on collisions.--sql-commenterflag plus anoptional per-source
sqlCommenteroverride in tools.yaml, default off.page (with an example
INFORMATION_SCHEMA.JOBSattribution query) and asqlCommenterrow in the BigQuery source reference.Tests: table-driven unit tests for key/value sanitization (uppercase,
illegal characters, truncation, leading non-letter keys), enable/override
precedence mirroring the existing
PrependCommentcases, label mergeprecedence, and YAML config parsing.
go test -race ./cmd/... ./internal/...and
golangci-lint runpass.Also verified end-to-end against a live BigQuery project: with
sqlCommenter: true, a query issued through the MCP protocol (JSON-RPCtools/callwith_meta["dev.mcp-toolbox/telemetry"]set) produced a jobcarrying all eight attributes as labels alongside the existing
mcp-toolbox-tool:With the flag off, the job carries only
mcp-toolbox-tool— byte-identicalto current behavior. The labels are queryable in
INFORMATION_SCHEMA.JOBS_BY_PROJECT(grouping bytes billed bytool_nameand
clientworks as expected).PR Checklist
CONTRIBUTING.md
bug/issue
before writing your code! That way we can discuss the change, evaluate
designs, and agree on the general idea
review
!if this involve a breaking change🛠️ Fixes #3286