From 74bdb1f2a88bec7979b0029bc37408446c4f57ea Mon Sep 17 00:00:00 2001 From: raphaelroshan <49832307+raphaelroshan@users.noreply.github.com> Date: Fri, 10 Jul 2026 13:46:20 +0800 Subject: [PATCH] fix: compare all LoggingOptions fields in PartialEq (#8449) * fix: compare all LoggingOptions fields in PartialEq Signed-off-by: raphaelroshan * test: set otlp_export_protocol in load_config_test expected configs The example configs set otlp_export_protocol = "http", but the expected LoggingOptions blocks for datanode/frontend/metasrv/standalone left it at the default None. Now that PartialEq compares all fields, add the field to match the parsed configs (as the flownode case already does). Signed-off-by: raphaelroshan --------- Signed-off-by: raphaelroshan --- src/cmd/tests/load_config_test.rs | 4 +++ src/common/telemetry/src/logging.rs | 39 ++++++++++++++++++----------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/cmd/tests/load_config_test.rs b/src/cmd/tests/load_config_test.rs index 74e4127de8..3570e88cc1 100644 --- a/src/cmd/tests/load_config_test.rs +++ b/src/cmd/tests/load_config_test.rs @@ -108,6 +108,7 @@ fn test_load_datanode_example_config() { level: Some("info".to_string()), dir: format!("{}/{}", DEFAULT_DATA_HOME, DEFAULT_LOGGING_DIR), otlp_endpoint: Some(DEFAULT_OTLP_HTTP_ENDPOINT.to_string()), + otlp_export_protocol: Some(common_telemetry::logging::OtlpExportProtocol::Http), tracing_sample_ratio: Some(Default::default()), ..Default::default() }, @@ -147,6 +148,7 @@ fn test_load_frontend_example_config() { level: Some("info".to_string()), dir: format!("{}/{}", DEFAULT_DATA_HOME, DEFAULT_LOGGING_DIR), otlp_endpoint: Some(DEFAULT_OTLP_HTTP_ENDPOINT.to_string()), + otlp_export_protocol: Some(common_telemetry::logging::OtlpExportProtocol::Http), tracing_sample_ratio: Some(Default::default()), ..Default::default() }, @@ -197,6 +199,7 @@ fn test_load_metasrv_example_config() { dir: format!("{}/{}", DEFAULT_DATA_HOME, DEFAULT_LOGGING_DIR), level: Some("info".to_string()), otlp_endpoint: Some(DEFAULT_OTLP_HTTP_ENDPOINT.to_string()), + otlp_export_protocol: Some(common_telemetry::logging::OtlpExportProtocol::Http), tracing_sample_ratio: Some(Default::default()), ..Default::default() }, @@ -314,6 +317,7 @@ fn test_load_standalone_example_config() { level: Some("info".to_string()), dir: format!("{}/{}", DEFAULT_DATA_HOME, DEFAULT_LOGGING_DIR), otlp_endpoint: Some(DEFAULT_OTLP_HTTP_ENDPOINT.to_string()), + otlp_export_protocol: Some(common_telemetry::logging::OtlpExportProtocol::Http), tracing_sample_ratio: Some(Default::default()), ..Default::default() }, diff --git a/src/common/telemetry/src/logging.rs b/src/common/telemetry/src/logging.rs index 953dbbc2df..5651353163 100644 --- a/src/common/telemetry/src/logging.rs +++ b/src/common/telemetry/src/logging.rs @@ -237,7 +237,7 @@ enum TraceState { } /// The logging options that used to initialize the logger. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(default)] pub struct LoggingOptions { /// The directory to store log files. If not set, logs will be written to stdout. @@ -341,20 +341,6 @@ pub enum LogFormat { Text, } -impl PartialEq for LoggingOptions { - fn eq(&self, other: &Self) -> bool { - self.dir == other.dir - && self.level == other.level - && self.enable_otlp_tracing == other.enable_otlp_tracing - && self.otlp_endpoint == other.otlp_endpoint - && self.tracing_sample_ratio == other.tracing_sample_ratio - && self.append_stdout == other.append_stdout - && self.enable_per_region_metrics == other.enable_per_region_metrics - } -} - -impl Eq for LoggingOptions {} - #[derive(Clone, Debug)] struct TraceContext { app_name: String, @@ -874,4 +860,27 @@ mod tests { let protocol: OtlpExportProtocol = serde_json::from_str(http_json).unwrap(); assert_eq!(protocol, OtlpExportProtocol::Http); } + + #[test] + fn test_logging_options_partial_eq_all_fields() { + let base = LoggingOptions::default(); + + let mut log_format = base.clone(); + log_format.log_format = LogFormat::Json; + assert_ne!(base, log_format); + + let mut max_log_files = base.clone(); + max_log_files.max_log_files += 1; + assert_ne!(base, max_log_files); + + let mut otlp_export_protocol = base.clone(); + otlp_export_protocol.otlp_export_protocol = Some(OtlpExportProtocol::Http); + assert_ne!(base, otlp_export_protocol); + + let mut otlp_headers = base.clone(); + otlp_headers + .otlp_headers + .insert("key".to_string(), "value".to_string()); + assert_ne!(base, otlp_headers); + } }