refactor: log statement on error by Display (#3803)

* refactor: log statement on error by Display

Signed-off-by: tison <wander4096@gmail.com>

* Apply suggestions from code review

Co-authored-by: Yingwen <realevenyag@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>

---------

Signed-off-by: tison <wander4096@gmail.com>
Co-authored-by: Yingwen <realevenyag@gmail.com>
This commit is contained in:
tison
2024-04-26 12:05:13 +08:00
committed by GitHub
parent 1ec595134d
commit 4eadd9f8a8
5 changed files with 8 additions and 68 deletions

View File

@@ -315,20 +315,18 @@ impl SqlQueryHandler for Instance {
break;
}
match self.query_statement(stmt, query_ctx.clone()).await {
match self.query_statement(stmt.clone(), query_ctx.clone()).await {
Ok(output) => {
let output_result =
query_interceptor.post_execute(output, query_ctx.clone());
results.push(output_result);
}
Err(e) => {
let redacted = sql::util::redact_sql_secrets(query.as_ref());
if e.status_code().should_log_error() {
error!(e; "Failed to execute query: {redacted}");
error!(e; "Failed to execute query: {stmt}");
} else {
debug!("Failed to execute query: {redacted}, {e}");
debug!("Failed to execute query: {stmt}, {e}");
}
results.push(Err(e));
break;
}
@@ -448,9 +446,7 @@ impl PrometheusHandler for Instance {
.execute_stmt(stmt, query_ctx.clone())
.await
.map_err(BoxedError::new)
.with_context(|_| ExecuteQuerySnafu {
query: format!("{query:?}"),
})?;
.context(ExecuteQuerySnafu)?;
Ok(interceptor.post_execute(output, query_ctx)?)
}

View File

@@ -149,9 +149,7 @@ impl Instance {
.handle_remote_query(&ctx, catalog_name, schema_name, &table_name, query)
.await
.map_err(BoxedError::new)
.with_context(|_| error::ExecuteQuerySnafu {
query: format!("{query:#?}"),
})?;
.context(error::ExecuteQuerySnafu)?;
results.push((table_name, output));
}

View File

@@ -97,9 +97,8 @@ pub enum Error {
error: Box<dyn std::error::Error + Send + Sync>,
},
#[snafu(display("Failed to execute query, query: {}", query))]
#[snafu(display("Failed to execute query"))]
ExecuteQuery {
query: String,
location: Location,
source: BoxedError,
},

View File

@@ -84,10 +84,7 @@ where
.do_query(query, query_ctx)
.await
.into_iter()
.map(|x| {
x.map_err(BoxedError::new)
.context(error::ExecuteQuerySnafu { query })
})
.map(|x| x.map_err(BoxedError::new).context(error::ExecuteQuerySnafu))
.collect()
}
@@ -108,14 +105,7 @@ where
.do_promql_query(query, query_ctx)
.await
.into_iter()
.map(|x| {
x.map_err(BoxedError::new).with_context(|_| {
let query_literal = format!("{query:?}");
error::ExecuteQuerySnafu {
query: query_literal,
}
})
})
.map(|x| x.map_err(BoxedError::new).context(error::ExecuteQuerySnafu))
.collect()
}

View File

@@ -13,20 +13,11 @@
// limitations under the License.
use std::fmt::{Display, Formatter};
use std::sync::LazyLock;
use regex::Regex;
use sqlparser::ast::{Expr, ObjectName, SqlOption, Value};
use crate::error::{InvalidTableOptionValueSnafu, Result};
static SQL_SECRET_PATTERNS: LazyLock<Vec<Regex>> = LazyLock::new(|| {
vec![
Regex::new(r#"(?i)access_key_id=["']([^"']*)["'].*"#).unwrap(),
Regex::new(r#"(?i)secret_access_key=["']([^"']*)["'].*"#).unwrap(),
]
});
/// Format an [ObjectName] without any quote of its idents.
pub fn format_raw_object_name(name: &ObjectName) -> String {
struct Inner<'a> {
@@ -59,37 +50,3 @@ pub fn parse_option_string(option: SqlOption) -> Result<(String, String)> {
let k = key.value.to_lowercase();
Ok((k, v))
}
/// Use regex to match and replace common seen secret values in SQL.
pub fn redact_sql_secrets(sql: &str) -> String {
let mut s = sql.to_string();
for p in SQL_SECRET_PATTERNS.iter() {
if let Some(captures) = p.captures(&s) {
if let Some(m) = captures.get(1) {
s = s.replace(m.as_str(), "******");
}
}
}
s
}
#[cfg(test)]
mod test {
use super::*;
#[test]
fn test_redact_sql_secrets() {
assert_eq!(
redact_sql_secrets(
r#"COPY 'my_table' FROM '/test.orc' WITH (FORMAT = 'orc') CONNECTION(ENDPOINT = 's3.storage.site', REGION = 'hz', ACCESS_KEY_ID='my_key_id', SECRET_ACCESS_KEY="my_access_key");"#
),
r#"COPY 'my_table' FROM '/test.orc' WITH (FORMAT = 'orc') CONNECTION(ENDPOINT = 's3.storage.site', REGION = 'hz', ACCESS_KEY_ID='******', SECRET_ACCESS_KEY="******");"#
);
assert_eq!(
redact_sql_secrets(
r#"COPY 'my_table' FROM '/test.orc' WITH (FORMAT = 'orc') CONNECTION(ENDPOINT = 's3.storage.site', REGION = 'hz', ACCESS_KEY_ID='@scoped/key_id', SECRET_ACCESS_KEY="@scoped/access_key");"#
),
r#"COPY 'my_table' FROM '/test.orc' WITH (FORMAT = 'orc') CONNECTION(ENDPOINT = 's3.storage.site', REGION = 'hz', ACCESS_KEY_ID='******', SECRET_ACCESS_KEY="******");"#
);
}
}