From b554aecd9f53d242430a1c6fff9116f9e0b7892f Mon Sep 17 00:00:00 2001 From: garren smith Date: Wed, 14 Sep 2022 14:49:02 +0200 Subject: [PATCH 1/3] only add trace comment to sql if we are tracing --- .../sql-query-connector/src/query_ext.rs | 4 +--- .../sql-query-connector/src/sql_trace.rs | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/query-engine/connectors/sql-query-connector/src/query_ext.rs b/query-engine/connectors/sql-query-connector/src/query_ext.rs index 2ed1ce32b583..3d52e22af2e7 100644 --- a/query-engine/connectors/sql-query-connector/src/query_ext.rs +++ b/query-engine/connectors/sql-query-connector/src/query_ext.rs @@ -50,9 +50,7 @@ pub trait QueryExt: Queryable + Send + Sync { Query::Select(Box::from(x.comment(trace_parent_to_string(span_ctx)))) } // This is part of the required changes to pass a traceid - (Query::Select(x), Some(traceparent)) => { - Query::Select(Box::from(x.comment(format!("traceparent={}", traceparent)))) - } + (Query::Select(x), trace_id) => Query::Select(Box::from(x.add_trace_id(trace_id))), (q, _) => q, }; diff --git a/query-engine/connectors/sql-query-connector/src/sql_trace.rs b/query-engine/connectors/sql-query-connector/src/sql_trace.rs index c3d67b46fc9e..17965ab21ee2 100644 --- a/query-engine/connectors/sql-query-connector/src/sql_trace.rs +++ b/query-engine/connectors/sql-query-connector/src/sql_trace.rs @@ -33,7 +33,12 @@ macro_rules! sql_trace { // Temporary method to pass the traceid in an operation fn add_trace_id(self, trace_id: Option) -> Self { if let Some(traceparent) = trace_id { - self.comment(format!("traceparent={}", traceparent)) + let out = should_sample(&traceparent); + if should_sample(&traceparent) { + self.comment(format!("traceparent={}", traceparent)) + } else { + self + } } else { self } @@ -42,6 +47,16 @@ macro_rules! sql_trace { }; } +fn should_sample(traceparent: &str) -> bool { + let trace_info: Vec<&str> = traceparent.split("-").collect(); + + if trace_info.len() != 4 { + false + } else { + trace_info.last() == Some(&"01") + } +} + sql_trace!(Insert<'_>); sql_trace!(Update<'_>); From 53907adbbee311ded24c9107bfd6e9a429dd04d2 Mon Sep 17 00:00:00 2001 From: garren smith Date: Wed, 14 Sep 2022 15:29:02 +0200 Subject: [PATCH 2/3] clippy --- query-engine/connectors/sql-query-connector/src/sql_trace.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/query-engine/connectors/sql-query-connector/src/sql_trace.rs b/query-engine/connectors/sql-query-connector/src/sql_trace.rs index 17965ab21ee2..b5e0643b2127 100644 --- a/query-engine/connectors/sql-query-connector/src/sql_trace.rs +++ b/query-engine/connectors/sql-query-connector/src/sql_trace.rs @@ -33,7 +33,6 @@ macro_rules! sql_trace { // Temporary method to pass the traceid in an operation fn add_trace_id(self, trace_id: Option) -> Self { if let Some(traceparent) = trace_id { - let out = should_sample(&traceparent); if should_sample(&traceparent) { self.comment(format!("traceparent={}", traceparent)) } else { @@ -48,7 +47,7 @@ macro_rules! sql_trace { } fn should_sample(traceparent: &str) -> bool { - let trace_info: Vec<&str> = traceparent.split("-").collect(); + let trace_info: Vec<&str> = traceparent.split('-').collect(); if trace_info.len() != 4 { false From 6f559dc48745bb5ca0d1b95d39d4e3301efd7bd2 Mon Sep 17 00:00:00 2001 From: garren smith Date: Thu, 15 Sep 2022 07:43:49 +0200 Subject: [PATCH 3/3] simplify trace check --- .../connectors/sql-query-connector/src/sql_trace.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/query-engine/connectors/sql-query-connector/src/sql_trace.rs b/query-engine/connectors/sql-query-connector/src/sql_trace.rs index b5e0643b2127..29cab8198215 100644 --- a/query-engine/connectors/sql-query-connector/src/sql_trace.rs +++ b/query-engine/connectors/sql-query-connector/src/sql_trace.rs @@ -47,13 +47,7 @@ macro_rules! sql_trace { } fn should_sample(traceparent: &str) -> bool { - let trace_info: Vec<&str> = traceparent.split('-').collect(); - - if trace_info.len() != 4 { - false - } else { - trace_info.last() == Some(&"01") - } + traceparent.split('-').count() == 4 && traceparent.ends_with("-01") } sql_trace!(Insert<'_>);