Skip to content

feat: add opt-in automatic query summaries - #41

Draft
torchello wants to merge 3 commits into
chmodas:trunkfrom
torchello:codex/upstream-query-summary
Draft

torchello wants to merge 3 commits into
chmodas:trunkfrom
torchello:codex/upstream-query-summary

Conversation

@torchello

@torchello torchello commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Unannotated queries currently keep generic database span names. This draft adds opt-in operation/target summaries such as SELECT users, so callers can group query spans and operation metrics without annotating every query.

PoolBuilder::with_query_summary_mode(QuerySummaryMode::Auto) enables inference. The default remains Off. This is a proposal to discuss extraction scope and supported SQL forms.

Behavior:

  • Generate db.query.summary for the span name and operation metrics; do not infer db.operation.name or db.collection.name.
  • Any explicit query annotations suppress inference, including annotations without a summary and empty annotations through the query-side API.
  • Work independently of QueryTextMode, including Off, using the existing literal obfuscator and a lightweight tokenizer with no parser dependency.
  • Resolve common read CTEs with bounded recursion; preserve physical write targets and operation casing. Anchor operation detection at the outer statement, rather than inferring an inner operation from unsupported commands.
  • Ignore MySQL # comments without treating PostgreSQL # operators as comments. Limit generated summaries to 255 bytes, falling back to the operation rather than truncating an identifier.

This is best-effort extraction, not a full dialect parser or a general SQL redaction boundary. Unsupported forms can be inaccurate or fall back to an operation-only/generic name. Recognized literals are scrubbed, but table/schema names still affect cardinality and can be sensitive. Keep automatic summaries off or use explicit annotations for sensitive SQL or dynamic object names; disabling query text alone does not disable summaries. The shared scrubber does not model PostgreSQL standard_conforming_strings, SQLite literal backslashes, or MySQL NO_BACKSLASH_ESCAPES/default double-quoted strings; in those forms literal text can surface as the summary target. Use explicit annotations or keep inference off for queries relying on those semantics.

Updated onto official trunk at e6c3de535843183d57a2ba084dc24be619743726 (SQLx 0.9, OpenTelemetry 0.33). The existing PR history is preserved. This contains only automatic-summary functionality and its docs/tests; it does not carry downstream error classification or SQLx core/driver patches.

Validation on this updated base (Rust 1.97.1):

  • Unit tests, all features: 225 passed.
  • SQLite integration tests: 119 passed, including actual pool/query-side annotation precedence.
  • All-target/all-feature Clippy with warnings denied, rustfmt, and cargo doc --no-deps --all-features: passed.
  • Regression/property coverage: supported literal shapes, Unicode/no-panic and length bounds, CTE/write targets, unsupported outer commands, MySQL comments, PostgreSQL operators, and opt-in span/metric grouping.
  • Targeted local LLVM coverage (--lib --all-features, macOS/Rust 1.97.1): src/summary.rs 682/682 lines and 67/67 functions, with no missing lines. Added CTE depth/scope/materialization, malformed-input, DDL/modifier, comment/identifier, UTF-8 boundary and structured-input property tests; production code is unchanged by this coverage follow-up.
  • Independent Claude Opus 5.5 xhigh CLI review: completed; required corrections addressed and final code delta approved.

The earlier PostgreSQL/MySQL full-suite results in the PR history predate this refresh; those services were not rerun for this update.

@torchello

Copy link
Copy Markdown
Contributor Author

@chmodas Any thoughts on this one?

Refresh existing PR chmodas#41 onto SQLx 0.9 / OpenTelemetry 0.33 without rewriting its history. Keep inference opt-in and explicit annotations authoritative. Anchor outer operations, handle MySQL comments conservatively, and cover literal/annotation edge cases.

Validated with Rust 1.97.1: 214 unit tests, 119 SQLite tests, all-target/all-feature Clippy, rustdoc and formatting.
@codecov-commenter

codecov-commenter commented Nov 2, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.88%. Comparing base (e6c3de5) to head (b18d469).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##            trunk      #41      +/-   ##
==========================================
+ Coverage   99.85%   99.88%   +0.03%     
==========================================
  Files          15       16       +1     
  Lines        2760     3497     +737     
==========================================
+ Hits         2756     3493     +737     
  Misses          4        4              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Add CTE depth and scope, malformed input, DDL, comment, quoted identifier,
UTF-8 limit and structured property tests without changing production code.

Validated with 225 unit tests, Clippy, rustfmt, and local LLVM coverage
(682/682 summary.rs lines). Independently reviewed with Claude Opus 5.5.
torchello added a commit to torchello/sqlx-otel that referenced this pull request Nov 2, 2026
Port reviewed query-summary corrections and coverage from upstream PR chmodas#41,
including backend-aware comment handling and explicit inference limits.
Preserve OTel 0.32, SQLSTATE error classification and pool metrics.

Validation: 225 unit, 119 SQLite, 21 pool-metrics and 13 doc tests;
Clippy and rustfmt pass. Independent Claude Opus 5.5 xhigh review approved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants