From 41e856eb9e56214450dc290d4d77fdb40daca815 Mon Sep 17 00:00:00 2001
From: Ning Sun <sunng@protonmail.com>
Date: Tue, 11 Jul 2023 20:49:05 +0800
Subject: [PATCH] refactor: change logging level for mysql error log (#1938)

* refactor: change logging level for mysql error log

* Update src/common/telemetry/Cargo.toml

Co-authored-by: LFC <bayinamine@gmail.com>

---------

Co-authored-by: LFC <bayinamine@gmail.com>
---
 src/common/telemetry/Cargo.toml    |  2 +-
 src/common/telemetry/src/macros.rs | 44 ++++++++++++++++++++++++++++++
 src/servers/src/metrics.rs         |  4 +++
 src/servers/src/mysql/writer.rs    | 10 +++++--
 4 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/src/common/telemetry/Cargo.toml b/src/common/telemetry/Cargo.toml
index acb916cfb1..2009befc50 100644
--- a/src/common/telemetry/Cargo.toml
+++ b/src/common/telemetry/Cargo.toml
@@ -24,7 +24,7 @@ opentelemetry-jaeger = { version = "0.16", features = ["rt-tokio"] }
 parking_lot = { version = "0.12", features = [
     "deadlock_detection",
 ], optional = true }
-serde = "1.0"
+serde.workspace = true
 tokio.workspace = true
 tracing = "0.1"
 tracing-appender = "0.2"
diff --git a/src/common/telemetry/src/macros.rs b/src/common/telemetry/src/macros.rs
index 3ad473b344..202032cd5f 100644
--- a/src/common/telemetry/src/macros.rs
+++ b/src/common/telemetry/src/macros.rs
@@ -140,6 +140,50 @@ macro_rules! warn {
         $crate::log!(target: $target, $crate::logging::Level::WARN, $($arg)+)
     };
 
+    // warn!(e; "a {} event", "log")
+    ($e:expr; $($arg:tt)+) => ({
+        use std::error::Error;
+        use $crate::common_error::ext::ErrorExt;
+        match ($e.source(), $e.location_opt()) {
+            (Some(source), Some(location)) => {
+                $crate::log!(
+                    $crate::logging::Level::WARN,
+                    err.msg = %$e,
+                    err.code = %$e.status_code(),
+                    err.source = source,
+                    err.location = %location,
+                    $($arg)+
+                )
+            },
+            (Some(source), None) => {
+                $crate::log!(
+                    $crate::logging::Level::WARN,
+                    err.msg = %$e,
+                    err.code = %$e.status_code(),
+                    err.source = source,
+                    $($arg)+
+                )
+            },
+            (None, Some(location)) => {
+                $crate::log!(
+                    $crate::logging::Level::WARN,
+                    err.msg = %$e,
+                    err.code = %$e.status_code(),
+                    err.location = %location,
+                    $($arg)+
+                )
+            },
+            (None, None) => {
+                $crate::log!(
+                    $crate::logging::Level::WARN,
+                    err.msg = %$e,
+                    err.code = %$e.status_code(),
+                    $($arg)+
+                )
+            }
+        }
+    });
+
     // warn!("a {} event", "log")
     ($($arg:tt)+) => {
         $crate::log!($crate::logging::Level::WARN, $($arg)+)
diff --git a/src/servers/src/metrics.rs b/src/servers/src/metrics.rs
index aca8c628bb..848871bf34 100644
--- a/src/servers/src/metrics.rs
+++ b/src/servers/src/metrics.rs
@@ -31,6 +31,10 @@ use crate::error::UpdateJemallocMetricsSnafu;
 pub(crate) const METRIC_DB_LABEL: &str = "db";
 pub(crate) const METRIC_CODE_LABEL: &str = "code";
 pub(crate) const METRIC_TYPE_LABEL: &str = "type";
+pub(crate) const METRIC_PROTOCOL_LABEL: &str = "protocol";
+
+pub(crate) const METRIC_ERROR_COUNTER: &str = "servers.error";
+pub(crate) const METRIC_ERROR_COUNTER_LABEL_MYSQL: &str = "mysql";
 
 pub(crate) const METRIC_HTTP_SQL_ELAPSED: &str = "servers.http_sql_elapsed";
 pub(crate) const METRIC_HTTP_PROMQL_ELAPSED: &str = "servers.http_promql_elapsed";
diff --git a/src/servers/src/mysql/writer.rs b/src/servers/src/mysql/writer.rs
index 4249b82780..92e8fa6a34 100644
--- a/src/servers/src/mysql/writer.rs
+++ b/src/servers/src/mysql/writer.rs
@@ -16,9 +16,10 @@ use std::ops::Deref;
 
 use common_query::Output;
 use common_recordbatch::{util, RecordBatch};
-use common_telemetry::error;
+use common_telemetry::warn;
 use datatypes::prelude::{ConcreteDataType, Value};
 use datatypes::schema::SchemaRef;
+use metrics::increment_counter;
 use opensrv_mysql::{
     Column, ColumnFlags, ColumnType, ErrorKind, OkResponse, QueryResultWriter, RowWriter,
 };
@@ -27,6 +28,7 @@ use snafu::prelude::*;
 use tokio::io::AsyncWrite;
 
 use crate::error::{self, Error, Result};
+use crate::metrics::*;
 
 /// Try to write multiple output to the writer if possible.
 pub async fn write_output<'a, W: AsyncWrite + Send + Sync + Unpin>(
@@ -200,7 +202,11 @@ impl<'a, W: AsyncWrite + Unpin> MysqlResultWriter<'a, W> {
         error: Error,
         w: QueryResultWriter<'a, W>,
     ) -> Result<()> {
-        error!(error; "Failed to execute query '{}'", query);
+        warn!(error; "Failed to execute query '{}'", query);
+        increment_counter!(
+            METRIC_ERROR_COUNTER,
+            &[(METRIC_PROTOCOL_LABEL, METRIC_ERROR_COUNTER_LABEL_MYSQL)]
+        );
 
         let kind = ErrorKind::ER_INTERNAL_ERROR;
         w.error(kind, error.to_string().as_bytes()).await?;