Repository navigation
chore: prepare SQLx 0.9 integration on OpenTelemetry 0.32 - #1
Merged
Merged
Conversation
This fixes an issue where the shutdown flag gets permanently flipped to true (which signals that the polling task should be stopped) any time a cloned pool is dropped. Because all the pool clones share the same `Arc<AtomicBool>`, the first clone to be dropped incorrectly flips the flag, which is not what the documentation implies. This also adds a regression test to make sure the metric task surives a cloned pool being dropped
Cuts the release covering the opentelemetry 0.32 upgrade and the pool-metrics shutdown fix. Adds a per-release compatibility matrix and writes down the versioning policy the crate has followed since 0.1.0. Both sqlx and opentelemetry have to be upgraded in lockstep with this crate, and they fail differently when they are not: sqlx types are in the public signatures, so a mismatch is a compile error, whereas opentelemetry is coupled only through its `global` provider registry, so a mismatch compiles fine and silently emits nothing. Neither constraint was documented anywhere. Corrects a CHANGELOG slip: v0.3.0 was tagged and published on 2026-05-05 but its notes were still sitting under `## [Unreleased]`.
Upgrades to sqlx 0.9, which raises the minimum Rust version to 1.94. sqlx 0.9 dropped the `'q` lifetime from statements and arguments. That changed the signatures of `prepare`, `prepare_with` and `describe`, and made `Execute::sql` consume the query rather than borrow it. The wrapper needs the query text for `db.query.text` and still has to pass the query on to the inner executor, which a consuming `sql` rules out, so queries are now taken apart and reassembled first. See `src/rebuilt_query.rs`. `sqlx-core` and the per-backend driver crates become direct dependencies so their `offline` feature can be enabled. Without it `Executor::describe` is absent from the trait; with it, implementing it is mandatory.
`db.client.operation.duration` and `db.client.connection.use_time` inherited the OpenTelemetry SDK's default bucket boundaries (`0, 5, 10, 25, ... 10000`), which are shaped for milliseconds. Semconv requires both be recorded in seconds, so every observation below five seconds collapsed into a single bucket and any quantile derived from it was an interpolation artefact rather than a measurement. `wait_time` already carried explicit boundaries; hoist that set into a shared `LATENCY_BUCKETS_SECONDS` constant and apply it to all three. The row-count histograms keep the SDK defaults, which already suit counts. The boundaries are instrument advice, so an application that registers a `View` with a compatible explicit aggregation still overrides them. Document that in the README with a worked example, and pin it with a test that observes the same workload with and without a view — asserting the crate's advice applies when no view matches, so the override is demonstrably an override rather than a value filling a vacuum. Also close gaps left by the acquisition-path work: record why `PoolExecutor` owns its pool rather than borrowing, and why it implements only four `Executor` methods — sqlx's defaults funnel into those, which is what holds acquisition to once per query, and an added override would double every acquisition metric with no compile error. Cover the error-after-acquisition, closed-pool, multi-statement-stream, and `prepare` paths.
Covers both unreleased pull requests. chmodas#40 routed every acquisition path through the instrumented `Pool::acquire()`, fixed the pending-requests leak on cancellation, and gave `wait_time` explicit bucket boundaries; chmodas#42 extended those boundaries to `operation.duration` and `use_time`, which had the same millisecond-shaped defaults. Entries are attributed per pull request rather than per behaviour, so the bucket change names both: bisecting a shifted dashboard should land on the commit that moved the histogram in question.
Adopt upstream 83ee877, including accepted owned transactions, acquisition metrics and seconds-scale latency buckets. Retain automatic query summaries from upstream PR41 and the fork error classification. Preserve both fork and upstream history; no SQLx driver or core source is changed here. Validated unit/SQLite/Tokio pool metrics (349 passes), async-std pool metrics (21 passes), restored implicit-query polling, minimal-driver feature checks, formatting and all-target/all-feature Clippy. Independent Opus5.5 xhigh review completed. Consumer migration remains subject to separate SQLx qualification.
torchello
marked this pull request as ready for review
November 2, 2026 07:44
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.
This updates the SQLx 0.8 integration branch to SQLx 0.9 while keeping OpenTelemetry 0.32 and the fork's telemetry contracts.
The integration adopts upstream
83ee8779a5faa699a13013e1fd0eb2d36dd2c810, including the merged owned-transaction, acquisition-metrics, and seconds-scale histogram fixes (chmodas#39, chmodas#40, chmodas#42 in chmodas/sqlx-otel). The remaining delta is automatic query summaries from the existing upstream PR chmodas#41 and the existing SQLSTATE/canonical error classification. No SQLx driver or core source is patched here.Validation with Rust 1.97.1:
This is a dependency preparation PR, not approval to switch a consumer to SQLx 0.9. The published 0.9.0 still lacks the PostgreSQL outer-BEGIN fix from transact-rs/sqlx#4394. A separate deterministic PostgreSQL 17 qualification of both the official fix commit and current main
7908fe79fabc816ce172ac1ac59f6b4bd548ca39passed outer BEGIN cancellation but failed nested SAVEPOINT cancellation and rejected nested custom BEGIN. A consumer that also requires a deadline for asynchronous pool return must qualify that protection separately; the upstream BEGIN fix does not provide it.