Skip to content

Commit

Permalink
Don't emit INFO spans for administrative queries
Browse files Browse the repository at this point in the history
This prevents internal queries like
`SELECT @@max_allowed_packet` from showing up in user
tracing unless tracing level TRACE is allowed.
  • Loading branch information
cloneable committed Feb 8, 2023
1 parent 0e8d22e commit 0fd5853
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 14 deletions.
6 changes: 3 additions & 3 deletions src/conn/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,7 @@ impl Conn {
/// Do nothing if socket address is already in [`Opts`] or if `prefer_socket` is `false`.
async fn read_socket(&mut self) -> Result<()> {
if self.inner.opts.prefer_socket() && self.inner.socket.is_none() {
let row_opt = self.query_first("SELECT @@socket").await?;
let row_opt = self.query_internal("SELECT @@socket").await?;
self.inner.socket = row_opt.unwrap_or((None,)).0;
}
Ok(())
Expand All @@ -898,7 +898,7 @@ impl Conn {
let max_allowed_packet = if let Some(value) = self.opts().max_allowed_packet() {
Some(value)
} else {
self.query_first("SELECT @@max_allowed_packet").await?
self.query_internal("SELECT @@max_allowed_packet").await?
};
if let Some(stream) = self.inner.stream.as_mut() {
stream.set_max_allowed_packet(max_allowed_packet.unwrap_or(DEFAULT_MAX_ALLOWED_PACKET));
Expand All @@ -911,7 +911,7 @@ impl Conn {
let wait_timeout = if let Some(value) = self.opts().wait_timeout() {
Some(value)
} else {
self.query_first("SELECT @@wait_timeout").await?
self.query_internal("SELECT @@wait_timeout").await?
};
self.inner.wait_timeout = Duration::from_secs(wait_timeout.unwrap_or(28800) as u64);
Ok(())
Expand Down
29 changes: 20 additions & 9 deletions src/conn/routines/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use futures_core::future::BoxFuture;
use futures_util::FutureExt;
use mysql_common::constants::Command;
#[cfg(feature = "tracing")]
use tracing::{field, info_span, Instrument, Level};
use tracing::{field, info_span, span_enabled, trace_span, Instrument, Level};

use crate::{Conn, TextProtocol};

Expand All @@ -12,24 +12,35 @@ use super::Routine;
#[derive(Debug, Copy, Clone)]
pub struct QueryRoutine<'a> {
data: &'a [u8],
#[cfg_attr(not(feature = "tracing"), allow(dead_code))]
internal: bool,
}

impl<'a> QueryRoutine<'a> {
pub fn new(data: &'a [u8]) -> Self {
Self { data }
pub fn new(data: &'a [u8], internal: bool) -> Self {
Self { data, internal }
}
}

impl Routine<()> for QueryRoutine<'_> {
fn call<'a>(&'a mut self, conn: &'a mut Conn) -> BoxFuture<'a, crate::Result<()>> {
#[cfg(feature = "tracing")]
let span = info_span!(
"mysql_async::query",
mysql_async.connection.id = conn.id(),
mysql_async.query.sql = field::Empty
);
let span = if self.internal {
trace_span!(
"mysql_async::query",
mysql_async.connection.id = conn.id(),
mysql_async.query.sql = field::Empty
)
} else {
info_span!(
"mysql_async::query",
mysql_async.connection.id = conn.id(),
mysql_async.query.sql = field::Empty
)
};

#[cfg(feature = "tracing")]
if tracing::span_enabled!(Level::DEBUG) {
if span_enabled!(Level::DEBUG) {
// The statement may contain sensitive data. Restrict to DEBUG.
span.record(
"mysql_async.query.sql",
Expand Down
28 changes: 26 additions & 2 deletions src/queryable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,33 @@ impl Conn {
where
Q: AsQuery + 'a,
{
self.routine(QueryRoutine::new(query.as_query().as_ref()))
self.routine(QueryRoutine::new(query.as_query().as_ref(), false))
.await
}

/// Used for internal querying of connection settings,
/// bypassing instrumentation meant for user queries.
// This is a merge of `Queryable::query_first` and `Conn::query_iter`.
// TODO: find a cleaner way without duplicating code.
pub(crate) fn query_internal<'a, T, Q>(&'a mut self, query: Q) -> BoxFuture<'a, Option<T>>
where
Q: AsQuery + 'a,
T: FromRow + Send + 'static,
{
async move {
self.routine(QueryRoutine::new(query.as_query().as_ref(), true))
.await?;
let mut result: QueryResult<'a, 'static, TextProtocol> = QueryResult::new(self);
let output = if result.is_empty() {
None
} else {
result.next().await?.map(crate::from_row)
};
result.drop_result().await?;
Ok(output)
}
.boxed()
}
}

/// Methods of this trait are used to execute database queries.
Expand Down Expand Up @@ -456,7 +480,7 @@ impl Queryable for Conn {
Q: AsQuery + 'a,
{
async move {
self.routine(QueryRoutine::new(query.as_query().as_ref()))
self.routine(QueryRoutine::new(query.as_query().as_ref(), false))
.await?;
Ok(QueryResult::new(self))
}
Expand Down

0 comments on commit 0fd5853

Please sign in to comment.