From c0fe800e793cc9a70dcc91f8039386ceef767cf6 Mon Sep 17 00:00:00 2001 From: dennis zhuang Date: Thu, 14 Aug 2025 11:43:13 +0800 Subject: [PATCH] feat: improve slow queries options deserialization (#6734) * feat: improve slow queries options deserialization Signed-off-by: Dennis Zhuang * refactor: use serde default for struct Signed-off-by: Dennis Zhuang --------- Signed-off-by: Dennis Zhuang --- Cargo.lock | 1 + src/common/base/src/lib.rs | 1 + src/common/base/src/serde.rs | 31 ++++++++ src/common/telemetry/Cargo.toml | 1 + src/common/telemetry/src/logging.rs | 108 +++++++++++++++++++++++++++- 5 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 src/common/base/src/serde.rs diff --git a/Cargo.lock b/Cargo.lock index 14cbe863a8..4e146ae096 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2784,6 +2784,7 @@ name = "common-telemetry" version = "0.17.0" dependencies = [ "backtrace", + "common-base", "common-error", "common-version", "console-subscriber", diff --git a/src/common/base/src/lib.rs b/src/common/base/src/lib.rs index 9fec936a13..cc5acdbf47 100644 --- a/src/common/base/src/lib.rs +++ b/src/common/base/src/lib.rs @@ -20,6 +20,7 @@ pub mod range_read; #[allow(clippy::all)] pub mod readable_size; pub mod secrets; +pub mod serde; pub type AffectedRows = usize; diff --git a/src/common/base/src/serde.rs b/src/common/base/src/serde.rs new file mode 100644 index 0000000000..cf099c2160 --- /dev/null +++ b/src/common/base/src/serde.rs @@ -0,0 +1,31 @@ +// Copyright 2023 Greptime Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use serde::{Deserialize, Deserializer}; + +/// Deserialize an empty string as the default value. +pub fn empty_string_as_default<'de, D, T>(deserializer: D) -> Result +where + D: Deserializer<'de>, + T: Default + Deserialize<'de>, +{ + let s = String::deserialize(deserializer)?; + + if s.is_empty() { + Ok(T::default()) + } else { + T::deserialize(serde::de::value::StringDeserializer::::new(s)) + .map_err(serde::de::Error::custom) + } +} diff --git a/src/common/telemetry/Cargo.toml b/src/common/telemetry/Cargo.toml index dd471c4f09..b5450826bc 100644 --- a/src/common/telemetry/Cargo.toml +++ b/src/common/telemetry/Cargo.toml @@ -13,6 +13,7 @@ workspace = true [dependencies] backtrace = "0.3" +common-base.workspace = true common-error.workspace = true common-version.workspace = true console-subscriber = { version = "0.1", optional = true } diff --git a/src/common/telemetry/src/logging.rs b/src/common/telemetry/src/logging.rs index 72e994ad91..7da4e93529 100644 --- a/src/common/telemetry/src/logging.rs +++ b/src/common/telemetry/src/logging.rs @@ -18,6 +18,7 @@ use std::io::IsTerminal; use std::sync::{Arc, Mutex, Once}; use std::time::Duration; +use common_base::serde::empty_string_as_default; use once_cell::sync::{Lazy, OnceCell}; use opentelemetry::{global, KeyValue}; use opentelemetry_otlp::{Protocol, SpanExporterBuilder, WithExportConfig}; @@ -60,6 +61,7 @@ pub struct LoggingOptions { pub level: Option, /// The log format that can be one of "json" or "text". Default is "text". + #[serde(default, deserialize_with = "empty_string_as_default")] pub log_format: LogFormat, /// The maximum number of log files set by default. @@ -94,11 +96,13 @@ pub enum OtlpExportProtocol { /// The options of slow query. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[serde(default)] pub struct SlowQueryOptions { /// Whether to enable slow query log. pub enable: bool, /// The record type of slow queries. + #[serde(deserialize_with = "empty_string_as_default")] pub record_type: SlowQueriesRecordType, /// The threshold of slow queries. @@ -126,19 +130,21 @@ impl Default for SlowQueryOptions { } } -#[derive(Clone, Debug, Serialize, Deserialize, Copy, PartialEq)] +#[derive(Clone, Debug, Serialize, Deserialize, Copy, PartialEq, Default)] #[serde(rename_all = "snake_case")] pub enum SlowQueriesRecordType { /// Record the slow query in the system table. + #[default] SystemTable, /// Record the slow query in a specific logs file. Log, } -#[derive(Clone, Debug, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Debug, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(rename_all = "snake_case")] pub enum LogFormat { Json, + #[default] Text, } @@ -529,3 +535,101 @@ where None } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_logging_options_deserialization_default() { + let json = r#"{}"#; + let opts: LoggingOptions = serde_json::from_str(json).unwrap(); + + assert_eq!(opts.log_format, LogFormat::Text); + assert_eq!(opts.dir, ""); + assert_eq!(opts.level, None); + assert!(opts.append_stdout); + } + + #[test] + fn test_logging_options_deserialization_empty_log_format() { + let json = r#"{"log_format": ""}"#; + let opts: LoggingOptions = serde_json::from_str(json).unwrap(); + + // Empty string should use default (Text) + assert_eq!(opts.log_format, LogFormat::Text); + } + + #[test] + fn test_logging_options_deserialization_valid_log_format() { + let json_format = r#"{"log_format": "json"}"#; + let opts: LoggingOptions = serde_json::from_str(json_format).unwrap(); + assert_eq!(opts.log_format, LogFormat::Json); + + let text_format = r#"{"log_format": "text"}"#; + let opts: LoggingOptions = serde_json::from_str(text_format).unwrap(); + assert_eq!(opts.log_format, LogFormat::Text); + } + + #[test] + fn test_logging_options_deserialization_missing_log_format() { + let json = r#"{"dir": "/tmp/logs"}"#; + let opts: LoggingOptions = serde_json::from_str(json).unwrap(); + + // Missing log_format should use default (Text) + assert_eq!(opts.log_format, LogFormat::Text); + assert_eq!(opts.dir, "/tmp/logs"); + } + + #[test] + fn test_slow_query_options_deserialization_default() { + let json = r#"{"enable": true, "threshold": "30s"}"#; + let opts: SlowQueryOptions = serde_json::from_str(json).unwrap(); + + assert_eq!(opts.record_type, SlowQueriesRecordType::SystemTable); + assert!(opts.enable); + } + + #[test] + fn test_slow_query_options_deserialization_empty_record_type() { + let json = r#"{"enable": true, "record_type": "", "threshold": "30s"}"#; + let opts: SlowQueryOptions = serde_json::from_str(json).unwrap(); + + // Empty string should use default (SystemTable) + assert_eq!(opts.record_type, SlowQueriesRecordType::SystemTable); + assert!(opts.enable); + } + + #[test] + fn test_slow_query_options_deserialization_valid_record_type() { + let system_table_json = + r#"{"enable": true, "record_type": "system_table", "threshold": "30s"}"#; + let opts: SlowQueryOptions = serde_json::from_str(system_table_json).unwrap(); + assert_eq!(opts.record_type, SlowQueriesRecordType::SystemTable); + + let log_json = r#"{"enable": true, "record_type": "log", "threshold": "30s"}"#; + let opts: SlowQueryOptions = serde_json::from_str(log_json).unwrap(); + assert_eq!(opts.record_type, SlowQueriesRecordType::Log); + } + + #[test] + fn test_slow_query_options_deserialization_missing_record_type() { + let json = r#"{"enable": false, "threshold": "30s"}"#; + let opts: SlowQueryOptions = serde_json::from_str(json).unwrap(); + + // Missing record_type should use default (SystemTable) + assert_eq!(opts.record_type, SlowQueriesRecordType::SystemTable); + assert!(!opts.enable); + } + + #[test] + fn test_otlp_export_protocol_deserialization_valid_values() { + let grpc_json = r#""grpc""#; + let protocol: OtlpExportProtocol = serde_json::from_str(grpc_json).unwrap(); + assert_eq!(protocol, OtlpExportProtocol::Grpc); + + let http_json = r#""http""#; + let protocol: OtlpExportProtocol = serde_json::from_str(http_json).unwrap(); + assert_eq!(protocol, OtlpExportProtocol::Http); + } +}