From 4bff0be32e46ec8a817faac6739d90bc89d2c5cb Mon Sep 17 00:00:00 2001 From: Taiming Liu <90490946+taimingl@users.noreply.github.com> Date: Sat, 6 Jul 2024 07:16:55 -0700 Subject: [PATCH] fix: trace's span name (#3916) --- src/common/utils/http.rs | 29 ++++------ src/handler/http/request/search/mod.rs | 54 +++++++++++++------ .../http/request/search/multi_streams.rs | 40 ++++++++++---- src/handler/http/request/traces/mod.rs | 18 ++++--- src/service/search/sql.rs | 13 +++-- 5 files changed, 95 insertions(+), 59 deletions(-) diff --git a/src/common/utils/http.rs b/src/common/utils/http.rs index df791af49c..a6fb373c06 100644 --- a/src/common/utils/http.rs +++ b/src/common/utils/http.rs @@ -96,10 +96,7 @@ pub(crate) fn get_folder(query: &Query>) -> String { } #[inline(always)] -pub(crate) fn get_or_create_trace_id_and_span( - headers: &HeaderMap, - ep: String, -) -> (String, Option) { +pub(crate) fn get_or_create_trace_id(headers: &HeaderMap, span: &Option) -> String { let cfg = config::get_config(); if let Some(traceparent) = headers.get("traceparent") { if cfg.common.tracing_enabled || cfg.common.tracing_search_enabled { @@ -108,16 +105,10 @@ pub(crate) fn get_or_create_trace_id_and_span( propagator.extract(&RequestHeaderExtractor::new(headers)) }); let trace_id = ctx.span().span_context().trace_id().to_string(); - - let span = if cfg.common.tracing_search_enabled { - // Create the span and set parent context - let span = tracing::info_span!("{ep}", ep = ep); + if let Some(span) = span { span.set_parent(ctx); - Some(span) - } else { - None - }; - (trace_id, span) + } + trace_id } else { // manually parse trace_id if let Ok(traceparent_str) = traceparent.to_str() { @@ -128,20 +119,18 @@ pub(crate) fn get_or_create_trace_id_and_span( // characters or all zeros), vendors MUST ignore the traceparent. // https://www.w3.org/TR/trace-context/#traceparent-header if trace_id.len() == 32 && !trace_id.chars().all(|c| c == '0') { - return (trace_id, None); + return trace_id; } } } // If parsing fails or trace_id is invalid, generate a new one log::warn!("Failed to parse valid trace_id from received [Traceparent] header"); - (config::ider::uuid(), None) + config::ider::uuid() } - } else if cfg.common.tracing_search_enabled { - let span = tracing::info_span!("{ep}", ep = ep); - let trace_id = span.context().span().span_context().trace_id().to_string(); - (trace_id, Some(span)) + } else if let Some(span) = span { + span.context().span().span_context().trace_id().to_string() } else { - (config::ider::uuid(), None) + config::ider::uuid() } } diff --git a/src/handler/http/request/search/mod.rs b/src/handler/http/request/search/mod.rs index 2e912ff3d6..473706a1f3 100644 --- a/src/handler/http/request/search/mod.rs +++ b/src/handler/http/request/search/mod.rs @@ -45,7 +45,7 @@ use crate::{ utils::{ functions, http::{ - get_or_create_trace_id_and_span, get_search_type_from_request, + get_or_create_trace_id, get_search_type_from_request, get_stream_type_from_request, get_use_cache_from_request, }, }, @@ -142,8 +142,15 @@ pub async fn search( let org_id = org_id.into_inner(); let mut range_error = String::new(); let cfg = get_config(); - let (trace_id, http_span) = - get_or_create_trace_id_and_span(in_req.headers(), format!("api/{org_id}/_search")); + let http_span = if cfg.common.tracing_search_enabled { + Some(tracing::info_span!( + "/api/{org_id}/_search", + org_id = org_id.clone() + )) + } else { + None + }; + let trace_id = get_or_create_trace_id(in_req.headers(), &http_span); let query = web::Query::>::from_query(in_req.query_string()).unwrap(); let stream_type = match get_stream_type_from_request(&query) { @@ -548,10 +555,16 @@ pub async fn around( let started_at = Utc::now().timestamp_micros(); let (org_id, stream_name) = path.into_inner(); let cfg = get_config(); - let (trace_id, http_span) = get_or_create_trace_id_and_span( - in_req.headers(), - format!("/api/{org_id}/{stream_name}/_around"), - ); + let http_span = if cfg.common.tracing_search_enabled { + Some(tracing::info_span!( + "/api/{org_id}/{stream_name}/_around", + org_id = org_id.clone(), + stream_name = stream_name.clone() + )) + } else { + None + }; + let trace_id = get_or_create_trace_id(in_req.headers(), &http_span); let mut uses_fn = false; let query = web::Query::>::from_query(in_req.query_string()).unwrap(); @@ -885,10 +898,16 @@ pub async fn values( Some(v) => v.to_str().unwrap(), None => "", }; - let (trace_id, http_span) = get_or_create_trace_id_and_span( - in_req.headers(), - format!("/api/{org_id}/{stream_name}/_values"), - ); + let http_span = if config::get_config().common.tracing_search_enabled { + Some(tracing::info_span!( + "/api/{org_id}/{stream_name}/_values", + org_id = org_id.clone(), + stream_name = stream_name.clone() + )) + } else { + None + }; + let trace_id = get_or_create_trace_id(in_req.headers(), &http_span); if fields.len() == 1 && DISTINCT_FIELDS.contains(&fields[0]) @@ -1471,10 +1490,15 @@ pub async fn search_partition( ) -> Result { let start = std::time::Instant::now(); let cfg = get_config(); - let (trace_id, http_span) = get_or_create_trace_id_and_span( - in_req.headers(), - format!("/api/{org_id}/_search_partition"), - ); + let http_span = if cfg.common.tracing_search_enabled { + Some(tracing::info_span!( + "/api/{org_id}/_search_partition", + org_id = org_id.clone(), + )) + } else { + None + }; + let trace_id = get_or_create_trace_id(in_req.headers(), &http_span); let org_id = org_id.into_inner(); let query = web::Query::>::from_query(in_req.query_string()).unwrap(); diff --git a/src/handler/http/request/search/multi_streams.rs b/src/handler/http/request/search/multi_streams.rs index 074faa8890..e31e210001 100644 --- a/src/handler/http/request/search/multi_streams.rs +++ b/src/handler/http/request/search/multi_streams.rs @@ -36,7 +36,7 @@ use crate::{ utils::{ functions, http::{ - get_or_create_trace_id_and_span, get_search_type_from_request, + get_or_create_trace_id, get_search_type_from_request, get_stream_type_from_request, }, }, @@ -119,8 +119,15 @@ pub async fn search_multi( let org_id = org_id.into_inner(); let cfg = get_config(); let started_at = Utc::now().timestamp_micros(); - let (trace_id, http_span) = - get_or_create_trace_id_and_span(in_req.headers(), format!("/api/{org_id}/_search_multi")); + let http_span = if cfg.common.tracing_search_enabled { + Some(tracing::info_span!( + "/api/{org_id}/_search_multi", + org_id = org_id.clone() + )) + } else { + None + }; + let trace_id = get_or_create_trace_id(in_req.headers(), &http_span); let query = web::Query::>::from_query(in_req.query_string()).unwrap(); let stream_type = match get_stream_type_from_request(&query) { @@ -419,10 +426,15 @@ pub async fn _search_partition_multi( ) -> Result { let start = std::time::Instant::now(); let cfg = get_config(); - let (trace_id, http_span) = get_or_create_trace_id_and_span( - in_req.headers(), - format!("/api/{org_id}/_search_partition_multi"), - ); + let http_span = if cfg.common.tracing_search_enabled { + Some(tracing::info_span!( + "/api/{org_id}/_search_partition_multi", + org_id = org_id.clone() + )) + } else { + None + }; + let trace_id = get_or_create_trace_id(in_req.headers(), &http_span); let org_id = org_id.into_inner(); let query = web::Query::>::from_query(in_req.query_string()).unwrap(); @@ -555,10 +567,16 @@ pub async fn around_multi( let (org_id, stream_names) = path.into_inner(); let cfg = get_config(); - let (trace_id, http_span) = get_or_create_trace_id_and_span( - in_req.headers(), - format!("/api/{org_id}/{stream_names}/_around_multi"), - ); + let http_span = if cfg.common.tracing_search_enabled { + Some(tracing::info_span!( + "/api/{org_id}/{stream_names}/_around_multi", + org_id = org_id.clone(), + stream_names = stream_names.clone() + )) + } else { + None + }; + let trace_id = get_or_create_trace_id(in_req.headers(), &http_span); let stream_names = base64::decode_url(&stream_names)?; let mut uses_fn = false; diff --git a/src/handler/http/request/traces/mod.rs b/src/handler/http/request/traces/mod.rs index 71a2e56b63..534367cd02 100644 --- a/src/handler/http/request/traces/mod.rs +++ b/src/handler/http/request/traces/mod.rs @@ -24,7 +24,7 @@ use tracing::Instrument; use crate::{ common::{ meta::{self, http::HttpResponse as MetaHttpResponse}, - utils::http::get_or_create_trace_id_and_span, + utils::http::get_or_create_trace_id, }, handler::http::request::{CONTENT_TYPE_JSON, CONTENT_TYPE_PROTO}, service::{search as SearchService, traces::otlp_http}, @@ -129,12 +129,19 @@ pub async fn get_latest_traces( path: web::Path<(String, String)>, in_req: HttpRequest, ) -> Result { + let cfg = get_config(); let start = std::time::Instant::now(); let (org_id, stream_name) = path.into_inner(); - let (trace_id, http_span) = get_or_create_trace_id_and_span( - in_req.headers(), - format!("/api/{org_id}/{stream_name}/traces/latest"), - ); + let http_span = if cfg.common.tracing_search_enabled { + Some(tracing::info_span!( + "/api/{org_id}/{stream_name}/traces/latest", + org_id = org_id.clone(), + stream_name = stream_name.clone() + )) + } else { + None + }; + let trace_id = get_or_create_trace_id(in_req.headers(), &http_span); let query = web::Query::>::from_query(in_req.query_string()).unwrap(); @@ -204,7 +211,6 @@ pub async fn get_latest_traces( metrics::QUERY_PENDING_NUMS .with_label_values(&[&org_id]) .inc(); - let cfg = get_config(); // get a local search queue lock #[cfg(not(feature = "enterprise"))] let locker = SearchService::QUEUE_LOCKER.clone(); diff --git a/src/service/search/sql.rs b/src/service/search/sql.rs index 5d7e45692a..25606d0236 100644 --- a/src/service/search/sql.rs +++ b/src/service/search/sql.rs @@ -62,12 +62,11 @@ static RE_ONLY_FROM: Lazy = Lazy::new(|| Regex::new(r"(?i) from[ ]+query" pub static RE_HISTOGRAM: Lazy = Lazy::new(|| Regex::new(r"(?i)histogram\(([^\)]*)\)").unwrap()); -static RE_MATCH_ALL: Lazy = +static RE_MATCH_ALL_RAW: Lazy = Lazy::new(|| Regex::new(r"(?i)match_all_raw\('([^']*)'\)").unwrap()); -static RE_MATCH_ALL_IGNORE_CASE: Lazy = +static RE_MATCH_ALL_RAW_IGNORE_CASE: Lazy = Lazy::new(|| Regex::new(r"(?i)match_all_raw_ignore_case\('([^']*)'\)").unwrap()); -static RE_MATCH_ALL_INDEXED: Lazy = - Lazy::new(|| Regex::new(r"(?i)match_all\('([^']*)'\)").unwrap()); +static RE_MATCH_ALL: Lazy = Lazy::new(|| Regex::new(r"(?i)match_all\('([^']*)'\)").unwrap()); pub static _TS_WITH_ALIAS: Lazy = Lazy::new(|| Regex::new(r"\s*\(\s*_timestamp\s*\)?\s*").unwrap()); @@ -494,13 +493,13 @@ impl Sql { if !token.to_lowercase().starts_with("match_all") { continue; } - for cap in RE_MATCH_ALL.captures_iter(token) { + for cap in RE_MATCH_ALL_RAW.captures_iter(token) { fulltext.push((cap[0].to_string(), cap[1].to_string())); } - for cap in RE_MATCH_ALL_IGNORE_CASE.captures_iter(token) { + for cap in RE_MATCH_ALL_RAW_IGNORE_CASE.captures_iter(token) { fulltext.push((cap[0].to_string(), cap[1].to_lowercase())); } - for cap in RE_MATCH_ALL_INDEXED.captures_iter(token) { + for cap in RE_MATCH_ALL.captures_iter(token) { indexed_text.push((cap[0].to_string(), cap[1].to_lowercase())); // since `terms` are indexed in lowercase } }