Add sqlcommenter feature for traceparent SQL comments#2
Open
Diggsey wants to merge 4 commits into
Open
Conversation
Opt-in feature that appends `/*traceparent='00-{trace}-{span}-{flags}'*/`
to outgoing SQL on PostgreSQL and MySQL so server-side observability
tools (Cloud SQL Insights, AlloyDB Insights, pg_stat_statements
consumers preserving comments) can link query stats back to upstream
OTel traces.
`sqlx_core::sqlcommenter::comment_for_span` reads the OTel trace
context by briefly entering the `QueryLogger`'s span and consulting
`opentelemetry::Context::current()` — that resolves to the span's OTel
context as long as the user has `tracing-opentelemetry`'s `Layer`
installed. Without a layer or a valid trace context, returns `None`
and the query goes out unmodified.
`maybe_append_comment` returns `Cow::Borrowed` on the no-context /
feature-off path, so the hot path allocates nothing. The MySQL
executor's `try_stream!` body needs to capture by move, so it goes a
step further: builds an `Option<String>` outside the generator and
falls back to `logger.sql().as_str()` inside, keeping the no-comment
fast path allocation-free.
Cost when the feature kicks in: the comment is part of the SQL string
used as the prepared-statement cache key, so each call with a fresh
trace id re-prepares the statement. That's the price of correlating
prepared queries with traces; it's why the feature is opt-in.
SQLite is intentionally skipped — there's no managed observability
consumer that would parse the comments.
Optional dep: `opentelemetry = "0.27"` (trace feature only), gated
behind the `sqlcommenter` feature on `sqlx-core` and passed through
from `sqlx-postgres`, `sqlx-mysql`, and the top-level `sqlx` crate.
Diggsey
force-pushed
the
db-tracing-spans-sqlcommenter
branch
from
0f03bd1 to
b2d011c
Compare
…ontext::current The previous implementation entered the QueryLogger span and read `opentelemetry::Context::current()`, on the assumption that tracing-opentelemetry's Layer syncs the OTel current context with the entered tracing span. It doesn't — `Layer::on_enter` only updates timing data; nothing ever calls `Context::attach()`. So `current()` returned the default (empty) context for every query, the carrier stayed empty, `comment_for_span` returned `None`, and no comment was ever appended. The downstream `persistent` override in the executors therefore never triggered, leaving every query going out as a cached named prepared statement with no traceparent — exactly the "no trace correlation in Cloud SQL Insights" symptom. Switch to `tracing_opentelemetry::OpenTelemetrySpanExt::context()`, which downcasts the subscriber to find the OTel layer's `WithContext` and builds an `opentelemetry::Context` from the `OtelData` extension stashed on the span at `on_new_span` time. That's the documented way to extract the OTel context from a tracing span, and it works whether or not the span is currently entered. If no OTel layer is installed, `context()` falls through to `Context::default()` and the existing empty-carrier check still produces `None`, so the no-context fast path is unchanged. Pulls in tracing-opentelemetry as a feature-gated optional dep (no extra cost for users not opting into sqlcommenter).
Forcing `persistent = false` made `prepare()` send `Parse(UNNAMED)`, but `resolve_statement_metadata` and `apply_patches` may issue `raw_sql(...).fetch_all` (a simple Query) to resolve custom types, which destroys the unnamed statement before Bind runs and fails with "unnamed prepared statement does not exist". Split the old `persistent` flag in `get_or_prepare` into separate `named` and `cache` axes. The sqlcommenter path now uses a named statement (immune to simple-query side trips) without caching, and `Close::Statement`s it after Execute so Postgres doesn't accumulate one-shot statements.
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
1 participant
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.
Opt-in feature that appends
/*traceparent='00-{trace}-{span}-{flags}'*/to outgoing SQL on PostgreSQL and MySQL so server-side observability tools (Cloud SQL Insights, AlloyDB Insights, pg_stat_statements consumers preserving comments) can link query stats back to upstream OTel traces.sqlx_core::sqlcommenter::comment_for_spanreads the OTel trace context by briefly entering theQueryLogger's span and consultingopentelemetry::Context::current()— that resolves to the span's OTel context as long as the user hastracing-opentelemetry'sLayerinstalled. Without a layer or a valid trace context, returnsNoneand the query goes out unmodified.maybe_append_commentreturnsCow::Borrowedon the no-context / feature-off path, so the hot path allocates nothing. The MySQL executor'stry_stream!body needs to capture by move, so it goes a step further: builds anOption<String>outside the generator and falls back tologger.sql().as_str()inside, keeping the no-comment fast path allocation-free.Cost when the feature kicks in: the comment is part of the SQL string used as the prepared-statement cache key, so each call with a fresh trace id re-prepares the statement. That's the price of correlating prepared queries with traces; it's why the feature is opt-in.
SQLite is intentionally skipped — there's no managed observability consumer that would parse the comments.
Optional dep:
opentelemetry = "0.27"(trace feature only), gated behind thesqlcommenterfeature onsqlx-coreand passed through fromsqlx-postgres,sqlx-mysql, and the top-levelsqlxcrate.Does your PR solve an issue?
Delete this text and add "fixes #(issue number)".
Do not just list issue numbers here as they will not be automatically closed on merging this pull request unless prefixed with "fixes" or "closes".
Is this a breaking change?
Delete this text and answer yes/no and explain.
If yes, this pull request will need to wait for the next major release (
0.{x + 1}.0)Behavior changes can be breaking if significant enough.
Consider Hyrum's Law: