From e90fa34983e3c013b82f1eadad76f015242cfa8c Mon Sep 17 00:00:00 2001 From: Shivanshu Raj Shrivastava Date: Thu, 8 May 2025 19:35:11 +0530 Subject: [PATCH] fix: backend fixes Signed-off-by: Shivanshu Raj Shrivastava --- pkg/modules/tracefunnel/clickhouse_queries.go | 260 ++++++++++++---- pkg/modules/tracefunnel/query.go | 290 ++++++++---------- pkg/modules/tracefunnel/utils.go | 2 +- pkg/modules/tracefunnel/utils_test.go | 2 +- .../app/traces/v3/query_builder.go | 6 +- .../app/traces/v3/query_builder_test.go | 2 +- .../app/traces/v4/query_builder.go | 6 +- .../app/traces/v4/query_builder_test.go | 10 +- 8 files changed, 340 insertions(+), 238 deletions(-) diff --git a/pkg/modules/tracefunnel/clickhouse_queries.go b/pkg/modules/tracefunnel/clickhouse_queries.go index 0dadc390aa11..48f38fd584e7 100644 --- a/pkg/modules/tracefunnel/clickhouse_queries.go +++ b/pkg/modules/tracefunnel/clickhouse_queries.go @@ -9,7 +9,7 @@ func formatClause(clause string) string { if clause == "" { return "" } - return fmt.Sprintf("AND %s", clause) + return fmt.Sprintf("%s", clause) } func BuildTwoStepFunnelValidationQuery( @@ -47,8 +47,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t1 AND name = span_name_t1 - AND (contains_error_t1 = 0 OR has_error = true) - %[9]s + AND (contains_error_t1 = 0 OR has_error = true) %[9]s GROUP BY trace_id LIMIT 100000 ) @@ -63,8 +62,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t2 AND name = span_name_t2 - AND (contains_error_t2 = 0 OR has_error = true) - %[10]s + AND (contains_error_t2 = 0 OR has_error = true) %[10]s GROUP BY trace_id LIMIT 100000 ) @@ -144,8 +142,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t1 AND name = span_name_t1 - AND (contains_error_t1 = 0 OR has_error = true) - %[12]s + AND (contains_error_t1 = 0 OR has_error = true) %[12]s GROUP BY trace_id LIMIT 100000 ) @@ -160,8 +157,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t2 AND name = span_name_t2 - AND (contains_error_t2 = 0 OR has_error = true) - %[13]s + AND (contains_error_t2 = 0 OR has_error = true) %[13]s GROUP BY trace_id LIMIT 100000 ) @@ -176,8 +172,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t3 AND name = span_name_t3 - AND (contains_error_t3 = 0 OR has_error = true) - %[14]s + AND (contains_error_t3 = 0 OR has_error = true) %[14]s GROUP BY trace_id LIMIT 100000 ) @@ -260,8 +255,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t1 AND name = span_name_t1 - AND (contains_error_t1 = 0 OR has_error = true) - %[11]s + AND (contains_error_t1 = 0 OR has_error = true) %[11]s GROUP BY trace_id LIMIT 100000 ) @@ -277,8 +271,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t2 AND name = span_name_t2 - AND (contains_error_t2 = 0 OR has_error = true) - %[12]s + AND (contains_error_t2 = 0 OR has_error = true) %[12]s GROUP BY trace_id LIMIT 100000 ) @@ -296,10 +289,14 @@ WITH WHERE s2.first_time > s1.first_time ) +-- Error counts for each step +, errors_step1 AS (SELECT countIf(has_error_flag) AS errors FROM step1) +, errors_step2 AS (SELECT countIf(has_error_flag) AS errors FROM step2) + SELECT - round((count(DISTINCT trace_id) * 100.0) / (SELECT count(DISTINCT trace_id) FROM step1), 2) AS conversion_rate, - count(DISTINCT trace_id) / time_window_sec AS avg_rate, - greatest(countIf(s1_has_error), countIf(s2_has_error)) AS errors, + round((count(DISTINCT joined.trace_id) * 100.0) / (SELECT count(DISTINCT joined.trace_id) FROM step1), 2) AS conversion_rate, + count(DISTINCT joined.trace_id) / time_window_sec AS avg_rate, + greatest((SELECT errors FROM errors_step1), (SELECT errors FROM errors_step2)) AS errors, avg(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) AS avg_duration, quantile(0.99)(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) AS p99_latency FROM joined;` @@ -373,8 +370,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t1 AND name = span_name_t1 - AND (contains_error_t1 = 0 OR has_error = true) - %[15]s + AND (contains_error_t1 = 0 OR has_error = true) %[15]s GROUP BY trace_id LIMIT 100000 ) @@ -390,8 +386,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t2 AND name = span_name_t2 - AND (contains_error_t2 = 0 OR has_error = true) - %[16]s + AND (contains_error_t2 = 0 OR has_error = true) %[16]s GROUP BY trace_id LIMIT 100000 ) @@ -407,8 +402,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t3 AND name = span_name_t3 - AND (contains_error_t3 = 0 OR has_error = true) - %[17]s + AND (contains_error_t3 = 0 OR has_error = true) %[17]s GROUP BY trace_id LIMIT 100000 ) @@ -441,10 +435,15 @@ WITH WHERE s3.first_time > j2.t2_time ) +-- Error counts for each step +, errors_step1 AS (SELECT countIf(has_error_flag) AS errors FROM step1) +, errors_step2 AS (SELECT countIf(has_error_flag) AS errors FROM step2) +, errors_step3 AS (SELECT countIf(has_error_flag) AS errors FROM step3) + SELECT round((count(DISTINCT joined_t3.trace_id) * 100.0) / (SELECT count(DISTINCT trace_id) FROM step1), 2) AS conversion_rate, count(DISTINCT joined_t3.trace_id) / time_window_sec AS avg_rate, - greatest(countIf(s1_has_error), countIf(s2_has_error), countIf(s3_has_error)) AS errors, + greatest((SELECT errors FROM errors_step1), (SELECT errors FROM errors_step2), (SELECT errors FROM errors_step3)) AS errors, avg(abs(CAST(t3_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) AS avg_duration, quantile(0.99)(abs(CAST(t3_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) AS p99_latency FROM joined_t3;` @@ -472,7 +471,117 @@ FROM joined_t3;` return query } -func BuildFunnelStepFunnelOverviewQuery( +func BuildTwoStepFunnelStepOverviewQuery( + containsErrorT1 int, + containsErrorT2 int, + latencyPointerT1 string, + latencyPointerT2 string, + startTs int64, + endTs int64, + serviceNameT1 string, + spanNameT1 string, + serviceNameT2 string, + spanNameT2 string, + clauseStep1 string, + clauseStep2 string, + latencyTypeT2 string, +) string { + queryTemplate := ` +WITH + %[1]d AS contains_error_t1, + %[2]d AS contains_error_t2, + '%[3]s' AS latency_pointer_t1, + '%[4]s' AS latency_pointer_t2, + toDateTime64(%[5]d / 1000000000, 9) AS start_ts, + toDateTime64(%[6]d / 1000000000, 9) AS end_ts, + (%[6]d - %[5]d) / 1000000000 AS time_window_sec, + '%[7]s' AS service_name_t1, + '%[8]s' AS span_name_t1, + '%[9]s' AS service_name_t2, + '%[10]s' AS span_name_t2 + +-- Step 1 +, step1 AS ( + SELECT + trace_id, + argMin(timestamp, timestamp) AS first_time, + argMin(has_error, timestamp) AS has_error_flag + FROM signoz_traces.signoz_index_v3 + WHERE + timestamp BETWEEN start_ts AND end_ts + AND serviceName = service_name_t1 + AND name = span_name_t1 + AND (contains_error_t1 = 0 OR has_error = true) %[11]s + GROUP BY trace_id + LIMIT 100000 +) + +-- Step 2 +, step2 AS ( + SELECT + trace_id, + argMin(timestamp, timestamp) AS first_time, + argMin(has_error, timestamp) AS has_error_flag + FROM signoz_traces.signoz_index_v3 + WHERE + timestamp BETWEEN start_ts AND end_ts + AND serviceName = service_name_t2 + AND name = span_name_t2 + AND (contains_error_t2 = 0 OR has_error = true) %[12]s + GROUP BY trace_id + LIMIT 100000 +) + +-- Join steps +, joined AS ( + SELECT + s1.trace_id, + s1.first_time AS t1_time, + s2.first_time AS t2_time, + s1.has_error_flag AS s1_has_error, + s2.has_error_flag AS s2_has_error + FROM step1 s1 + INNER JOIN step2 s2 ON s1.trace_id = s2.trace_id + WHERE s2.first_time > s1.first_time +) + +-- Error counts for each step +, errors_step1 AS (SELECT countIf(has_error_flag) AS errors FROM step1) +, errors_step2 AS (SELECT countIf(has_error_flag) AS errors FROM step2) + +SELECT + round((count(DISTINCT joined.trace_id) * 100.0) / (SELECT count(DISTINCT joined.trace_id) FROM step1), 2) AS conversion_rate, + count(DISTINCT joined.trace_id) / time_window_sec AS avg_rate, + greatest((SELECT errors FROM errors_step1), (SELECT errors FROM errors_step2)) AS errors, + avg(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) AS avg_duration, + CASE + WHEN '%[13]s' = 'p99' THEN quantile(0.99)(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) + WHEN '%[13]s' = 'p95' THEN quantile(0.95)(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) + WHEN '%[13]s' = 'p90' THEN quantile(0.90)(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) + ELSE quantile(0.99)(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) + END AS latency +FROM joined;` + + query := fmt.Sprintf(queryTemplate, + containsErrorT1, + containsErrorT2, + latencyPointerT1, + latencyPointerT2, + startTs, + endTs, + serviceNameT1, + spanNameT1, + serviceNameT2, + spanNameT2, + formatClause(clauseStep1), + formatClause(clauseStep2), + latencyTypeT2, + ) + + return query +} + +func BuildThreeStepFunnelStepOverviewQuery( containsErrorT1 int, containsErrorT2 int, containsErrorT3 int, @@ -490,8 +599,13 @@ func BuildFunnelStepFunnelOverviewQuery( clauseStep1 string, clauseStep2 string, clauseStep3 string, + stepStart int64, + stepEnd int64, + latencyTypeT2 string, + latencyTypeT3 string, ) string { - queryTemplate := ` + // Common WITH and CTEs for all cases + baseQuery := ` WITH %[1]d AS contains_error_t1, %[2]d AS contains_error_t2, @@ -523,8 +637,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t1 AND name = span_name_t1 - AND (contains_error_t1 = 0 OR has_error = true) - %[15]s + AND (contains_error_t1 = 0 OR has_error = true) %[15]s GROUP BY trace_id LIMIT 100000 ) @@ -540,8 +653,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t2 AND name = span_name_t2 - AND (contains_error_t2 = 0 OR has_error = true) - %[16]s + AND (contains_error_t2 = 0 OR has_error = true) %[16]s GROUP BY trace_id LIMIT 100000 ) @@ -557,8 +669,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t3 AND name = span_name_t3 - AND (contains_error_t3 = 0 OR has_error = true) - %[17]s + AND (contains_error_t3 = 0 OR has_error = true) %[17]s GROUP BY trace_id LIMIT 100000 ) @@ -569,6 +680,7 @@ WITH s1.trace_id, s1.first_time AS t1_time, s2.first_time AS t2_time, + s1.has_error_flag AS s1_has_error, s2.has_error_flag AS s2_has_error FROM step1 s1 INNER JOIN step2 s2 ON s1.trace_id = s2.trace_id @@ -579,8 +691,10 @@ WITH , joined_t3 AS ( SELECT j2.trace_id, + j2.t1_time, j2.t2_time, s3.first_time AS t3_time, + j2.s1_has_error, j2.s2_has_error, s3.has_error_flag AS s3_has_error FROM joined_t2 j2 @@ -588,15 +702,50 @@ WITH WHERE s3.first_time > j2.t2_time ) -SELECT - round((count(DISTINCT joined_t3.trace_id) * 100.0) / (SELECT count(DISTINCT trace_id) FROM joined_t2), 2) AS conversion_rate, - count(DISTINCT joined_t3.trace_id) / time_window_sec AS avg_rate, - greatest(countIf(s2_has_error), countIf(s3_has_error)) AS errors, - avg(abs(CAST(t3_time AS Decimal(20, 9)) - CAST(t2_time AS Decimal(20, 9))) * 1000) AS avg_duration, - quantile(0.99)(abs(CAST(t3_time AS Decimal(20, 9)) - CAST(t2_time AS Decimal(20, 9))) * 1000) AS p99_latency -FROM joined_t3;` +-- Error counts for each step +, errors_step1 AS (SELECT countIf(has_error_flag) AS errors FROM step1) +, errors_step2 AS (SELECT countIf(has_error_flag) AS errors FROM step2) +, errors_step3 AS (SELECT countIf(has_error_flag) AS errors FROM step3) +` - query := fmt.Sprintf(queryTemplate, + var selectQuery string + // Dynamically select the correct SELECT statement based on stepStart and stepEnd + if stepStart == 1 && stepEnd == 2 { + // Metrics for step1 -> step2 (joined_t2) + selectQuery = ` +SELECT + round((count(DISTINCT trace_id) * 100.0) / (SELECT count(DISTINCT trace_id) FROM step1), 2) AS conversion_rate, + count(DISTINCT trace_id) / time_window_sec AS avg_rate, + greatest((SELECT errors FROM errors_step1), (SELECT errors FROM errors_step2)) AS errors, + avg(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) AS avg_duration, + CASE + WHEN '%[18]s' = 'p99' THEN quantile(0.99)(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) + WHEN '%[18]s' = 'p95' THEN quantile(0.95)(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) + WHEN '%[18]s' = 'p90' THEN quantile(0.90)(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) + ELSE quantile(0.99)(abs(CAST(t2_time AS Decimal(20, 9)) - CAST(t1_time AS Decimal(20, 9))) * 1000) + END AS latency +FROM joined_t2` + } else if stepStart == 2 && stepEnd == 3 { + // Metrics for step2 -> step3 (joined_t3) + selectQuery = ` +SELECT + round((count(DISTINCT trace_id) * 100.0) / (SELECT count(DISTINCT trace_id) FROM joined_t2), 2) AS conversion_rate, + count(DISTINCT trace_id) / time_window_sec AS avg_rate, + greatest((SELECT errors FROM errors_step2), (SELECT errors FROM errors_step3)) AS errors, + avg(abs(CAST(t3_time AS Decimal(20, 9)) - CAST(t2_time AS Decimal(20, 9))) * 1000) AS avg_duration, + CASE + WHEN '%[19]s' = 'p99' THEN quantile(0.99)(abs(CAST(t3_time AS Decimal(20, 9)) - CAST(t2_time AS Decimal(20, 9))) * 1000) + WHEN '%[19]s' = 'p95' THEN quantile(0.95)(abs(CAST(t3_time AS Decimal(20, 9)) - CAST(t2_time AS Decimal(20, 9))) * 1000) + WHEN '%[19]s' = 'p90' THEN quantile(0.90)(abs(CAST(t3_time AS Decimal(20, 9)) - CAST(t2_time AS Decimal(20, 9))) * 1000) + ELSE quantile(0.99)(abs(CAST(t3_time AS Decimal(20, 9)) - CAST(t2_time AS Decimal(20, 9))) * 1000) + END AS latency +FROM joined_t3;` + } else { + // Fallback: return empty result + selectQuery = `SELECT 0 AS conversion_rate, 0 AS avg_rate, 0 AS errors, 0 AS avg_duration, 0 AS latency;` + } + + query := fmt.Sprintf(baseQuery+selectQuery, containsErrorT1, containsErrorT2, containsErrorT3, @@ -614,6 +763,8 @@ FROM joined_t3;` formatClause(clauseStep1), formatClause(clauseStep2), formatClause(clauseStep3), + latencyTypeT2, + latencyTypeT3, ) return query @@ -655,8 +806,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t1 AND name = span_name_t1 - AND (contains_error_t1 = 0 OR has_error = true) - %[9]s + AND (contains_error_t1 = 0 OR has_error = true) %[9]s GROUP BY trace_id LIMIT 100000 ) @@ -672,8 +822,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t2 AND name = span_name_t2 - AND (contains_error_t2 = 0 OR has_error = true) - %[10]s + AND (contains_error_t2 = 0 OR has_error = true) %[10]s GROUP BY trace_id LIMIT 100000 ) @@ -758,8 +907,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t1 AND name = span_name_t1 - AND (contains_error_t1 = 0 OR has_error = true) - %[12]s + AND (contains_error_t1 = 0 OR has_error = true) %[12]s GROUP BY trace_id LIMIT 100000 ) @@ -775,8 +923,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t2 AND name = span_name_t2 - AND (contains_error_t2 = 0 OR has_error = true) - %[13]s + AND (contains_error_t2 = 0 OR has_error = true) %[13]s GROUP BY trace_id LIMIT 100000 ) @@ -792,8 +939,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t3 AND name = span_name_t3 - AND (contains_error_t3 = 0 OR has_error = true) - %[14]s + AND (contains_error_t3 = 0 OR has_error = true) %[14]s GROUP BY trace_id LIMIT 100000 ) @@ -889,8 +1035,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t1 AND name = span_name_t1 - AND (contains_error_t1 = 0 OR has_error = true) - %[9]s + AND (contains_error_t1 = 0 OR has_error = true) %[9]s GROUP BY trace_id LIMIT 100000 ), step1 AS ( @@ -907,8 +1052,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t2 AND name = span_name_t2 - AND (contains_error_t2 = 0 OR has_error = true) - %[10]s + AND (contains_error_t2 = 0 OR has_error = true) %[10]s GROUP BY trace_id LIMIT 100000 ), step2 AS ( @@ -1003,8 +1147,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t1 AND name = span_name_t1 - AND (contains_error_t1 = 0 OR has_error = true) - %[9]s + AND (contains_error_t1 = 0 OR has_error = true) %[9]s GROUP BY trace_id LIMIT 100000 ), step1 AS ( @@ -1021,8 +1164,7 @@ WITH timestamp BETWEEN start_ts AND end_ts AND serviceName = service_name_t2 AND name = span_name_t2 - AND (contains_error_t2 = 0 OR has_error = true) - %[10]s + AND (contains_error_t2 = 0 OR has_error = true) %[10]s GROUP BY trace_id LIMIT 100000 ), step2 AS ( diff --git a/pkg/modules/tracefunnel/query.go b/pkg/modules/tracefunnel/query.go index 2939514b5f2e..a517a5ccd7ed 100644 --- a/pkg/modules/tracefunnel/query.go +++ b/pkg/modules/tracefunnel/query.go @@ -2,109 +2,15 @@ package tracefunnel import ( "fmt" - "strings" - + tracev4 "github.com/SigNoz/signoz/pkg/query-service/app/traces/v4" v3 "github.com/SigNoz/signoz/pkg/query-service/model/v3" - "github.com/SigNoz/signoz/pkg/query-service/utils" - tracefunnel "github.com/SigNoz/signoz/pkg/types/tracefunnel" + "github.com/SigNoz/signoz/pkg/types/tracefunnel" + "strings" ) -func getColumnName(key v3.AttributeKey) string { - if key.IsColumn { - return key.Key - } - filterType, filterDataType := getClickhouseTracesColumnDataTypeAndType(key) - return fmt.Sprintf("%s%s['%s']", filterDataType, filterType, key.Key) -} - -func getClickhouseTracesColumnDataTypeAndType(key v3.AttributeKey) (v3.AttributeKeyType, string) { - filterType := key.Type - filterDataType := "string" - if key.DataType == v3.AttributeKeyDataTypeFloat64 || key.DataType == v3.AttributeKeyDataTypeInt64 { - filterDataType = "number" - } else if key.DataType == v3.AttributeKeyDataTypeBool { - filterDataType = "bool" - } - if filterType == v3.AttributeKeyTypeTag { - filterType = "TagMap" - } else { - filterType = "resourceTagsMap" - filterDataType = "" - } - return filterType, filterDataType -} - -// buildFilterClause converts a FilterSet into a SQL WHERE clause string -func buildFilterClause(filters *v3.FilterSet) string { - if filters == nil || len(filters.Items) == 0 { - return "" - } - - var conditions []string - for _, item := range filters.Items { - // Get the column name based on the key type - columnName := getColumnName(item.Key) - - // Convert operator to lowercase for consistency - op := strings.ToLower(string(item.Operator)) - - // Format the value based on its type - var valueStr string - var err error - if op != "exists" && op != "nexists" { - item.Value, err = utils.ValidateAndCastValue(item.Value, item.Key.DataType) - if err != nil { - continue // Skip invalid values - } - valueStr = utils.ClickHouseFormattedValue(item.Value) - } - - // Build the condition based on the operator - var condition string - switch op { - case "exists": - if item.Key.IsColumn { - condition = fmt.Sprintf("%s != ''", columnName) - } else { - columnType, columnDataType := getClickhouseTracesColumnDataTypeAndType(item.Key) - condition = fmt.Sprintf("has(%s%s, '%s')", columnDataType, columnType, item.Key.Key) - } - case "nexists": - if item.Key.IsColumn { - condition = fmt.Sprintf("%s = ''", columnName) - } else { - columnType, columnDataType := getClickhouseTracesColumnDataTypeAndType(item.Key) - condition = fmt.Sprintf("NOT has(%s%s, '%s')", columnDataType, columnType, item.Key.Key) - } - case "contains", "ncontains": - val := utils.QuoteEscapedString(fmt.Sprintf("%v", item.Value)) - operator := "ILIKE" - if op == "ncontains" { - operator = "NOT ILIKE" - } - condition = fmt.Sprintf("%s %s '%%%s%%'", columnName, operator, val) - case "regex", "nregex": - operator := "match" - if op == "nregex" { - operator = "NOT match" - } - condition = fmt.Sprintf("%s(%s, %s)", operator, columnName, valueStr) - default: - condition = fmt.Sprintf("%s %s %s", columnName, strings.ToUpper(op), valueStr) - } - conditions = append(conditions, condition) - } - - // Join conditions with the operator (AND/OR) - operator := strings.ToUpper(filters.Operator) - if operator == "" { - operator = "AND" // Default to AND if not specified - } - return strings.Join(conditions, fmt.Sprintf(" %s ", operator)) -} - func ValidateTraces(funnel *tracefunnel.Funnel, timeRange tracefunnel.TimeRange) (*v3.ClickHouseQuery, error) { var query string + var err error funnelSteps := funnel.Steps containsErrorT1 := 0 @@ -122,11 +28,20 @@ func ValidateTraces(funnel *tracefunnel.Funnel, timeRange tracefunnel.TimeRange) } // Build filter clauses for each step - clauseStep1 := buildFilterClause(funnelSteps[0].Filters) - clauseStep2 := buildFilterClause(funnelSteps[1].Filters) + clauseStep1, err := tracev4.BuildTracesFilterQuery(funnelSteps[0].Filters) + if err != nil { + return nil, err + } + clauseStep2, err := tracev4.BuildTracesFilterQuery(funnelSteps[1].Filters) + if err != nil { + return nil, err + } clauseStep3 := "" if len(funnel.Steps) > 2 { - clauseStep3 = buildFilterClause(funnelSteps[2].Filters) + clauseStep3, err = tracev4.BuildTracesFilterQuery(funnelSteps[2].Filters) + if err != nil { + return nil, err + } } if len(funnel.Steps) > 2 { @@ -168,14 +83,18 @@ func ValidateTraces(funnel *tracefunnel.Funnel, timeRange tracefunnel.TimeRange) func GetFunnelAnalytics(funnel *tracefunnel.Funnel, timeRange tracefunnel.TimeRange) (*v3.ClickHouseQuery, error) { var query string + var err error funnelSteps := funnel.Steps containsErrorT1 := 0 containsErrorT2 := 0 containsErrorT3 := 0 - latencyPointerT1 := "start" - latencyPointerT2 := "start" + latencyPointerT1 := funnelSteps[0].LatencyPointer + latencyPointerT2 := funnelSteps[1].LatencyPointer latencyPointerT3 := "start" + if len(funnel.Steps) > 2 { + latencyPointerT3 = funnelSteps[2].LatencyPointer + } if funnelSteps[0].HasErrors { containsErrorT1 = 1 @@ -187,22 +106,21 @@ func GetFunnelAnalytics(funnel *tracefunnel.Funnel, timeRange tracefunnel.TimeRa containsErrorT3 = 1 } - if funnelSteps[0].LatencyPointer != "" { - latencyPointerT1 = "end" - } - if funnelSteps[1].LatencyPointer != "" { - latencyPointerT2 = "end" - } - if len(funnel.Steps) > 2 && funnelSteps[2].LatencyPointer != "" { - latencyPointerT3 = "end" - } - // Build filter clauses for each step - clauseStep1 := buildFilterClause(funnelSteps[0].Filters) - clauseStep2 := buildFilterClause(funnelSteps[1].Filters) + clauseStep1, err := tracev4.BuildTracesFilterQuery(funnelSteps[0].Filters) + if err != nil { + return nil, err + } + clauseStep2, err := tracev4.BuildTracesFilterQuery(funnelSteps[1].Filters) + if err != nil { + return nil, err + } clauseStep3 := "" if len(funnel.Steps) > 2 { - clauseStep3 = buildFilterClause(funnelSteps[2].Filters) + clauseStep3, err = tracev4.BuildTracesFilterQuery(funnelSteps[2].Filters) + if err != nil { + return nil, err + } } if len(funnel.Steps) > 2 { @@ -246,79 +164,100 @@ func GetFunnelAnalytics(funnel *tracefunnel.Funnel, timeRange tracefunnel.TimeRa func GetFunnelStepAnalytics(funnel *tracefunnel.Funnel, timeRange tracefunnel.TimeRange, stepStart, stepEnd int64) (*v3.ClickHouseQuery, error) { var query string + var err error funnelSteps := funnel.Steps containsErrorT1 := 0 containsErrorT2 := 0 containsErrorT3 := 0 - latencyPointerT1 := "start" - latencyPointerT2 := "start" + latencyPointerT1 := funnelSteps[0].LatencyPointer + latencyPointerT2 := funnelSteps[1].LatencyPointer latencyPointerT3 := "start" - stepStartOrder := 0 - stepEndOrder := 1 + if len(funnel.Steps) > 2 { + latencyPointerT3 = funnelSteps[2].LatencyPointer + } + latencyTypeT2 := "p99" + latencyTypeT3 := "p99" - if stepStart != stepEnd { - stepStartOrder = int(stepStart) - 1 - stepEndOrder = int(stepEnd) - 1 - if funnelSteps[stepStartOrder].HasErrors { - containsErrorT1 = 1 - } - if funnelSteps[stepEndOrder].HasErrors { - containsErrorT2 = 1 - } - if funnelSteps[stepStartOrder].LatencyPointer != "" { - latencyPointerT1 = "end" - } - if funnelSteps[stepEndOrder].LatencyPointer != "" { - latencyPointerT2 = "end" - } + if stepStart == stepEnd { + return nil, fmt.Errorf("step start and end cannot be the same for /step/overview") } - // Build filter clauses for the steps - clauseStep1 := buildFilterClause(funnelSteps[stepStartOrder].Filters) - clauseStep2 := buildFilterClause(funnelSteps[stepEndOrder].Filters) + if funnelSteps[0].HasErrors { + containsErrorT1 = 1 + } + if funnelSteps[1].HasErrors { + containsErrorT2 = 1 + } + if len(funnel.Steps) > 2 && funnelSteps[2].HasErrors { + containsErrorT3 = 1 + } + + if funnelSteps[1].LatencyType != "" { + latencyTypeT2 = strings.ToLower(funnelSteps[1].LatencyType) + } + if len(funnel.Steps) > 2 && funnelSteps[2].LatencyType != "" { + latencyTypeT3 = strings.ToLower(funnelSteps[2].LatencyType) + } + + // Build filter clauses for each step + clauseStep1, err := tracev4.BuildTracesFilterQuery(funnelSteps[0].Filters) + if err != nil { + return nil, err + } + clauseStep2, err := tracev4.BuildTracesFilterQuery(funnelSteps[1].Filters) + if err != nil { + return nil, err + } clauseStep3 := "" if len(funnel.Steps) > 2 { - clauseStep3 = buildFilterClause(funnelSteps[2].Filters) + clauseStep3, err = tracev4.BuildTracesFilterQuery(funnelSteps[2].Filters) + if err != nil { + return nil, err + } } - if stepStart == 2 { - query = BuildFunnelStepFunnelOverviewQuery( + if len(funnel.Steps) > 2 { + query = BuildThreeStepFunnelStepOverviewQuery( containsErrorT1, // containsErrorT1 containsErrorT2, // containsErrorT2 - containsErrorT3, + containsErrorT3, // containsErrorT3 latencyPointerT1, latencyPointerT2, latencyPointerT3, - timeRange.StartTime, // startTs - timeRange.EndTime, // endTs - funnelSteps[0].ServiceName, - funnelSteps[0].SpanName, - funnelSteps[stepStartOrder].ServiceName, // serviceNameT1 - funnelSteps[stepStartOrder].SpanName, // spanNameT1 - funnelSteps[stepEndOrder].ServiceName, // serviceNameT1 - funnelSteps[stepEndOrder].SpanName, // spanNameT2 + timeRange.StartTime, // startTs + timeRange.EndTime, // endTs + funnelSteps[0].ServiceName, // serviceNameT1 + funnelSteps[0].SpanName, // spanNameT1 + funnelSteps[1].ServiceName, // serviceNameT1 + funnelSteps[1].SpanName, // spanNameT2 + funnelSteps[2].ServiceName, // serviceNameT1 + funnelSteps[2].SpanName, // spanNameT3 clauseStep1, clauseStep2, clauseStep3, + stepStart, + stepEnd, + latencyTypeT2, + latencyTypeT3, ) } else { - query = BuildTwoStepFunnelOverviewQuery( + query = BuildTwoStepFunnelStepOverviewQuery( containsErrorT1, // containsErrorT1 containsErrorT2, // containsErrorT2 latencyPointerT1, latencyPointerT2, - timeRange.StartTime, // startTs - timeRange.EndTime, // endTs - funnelSteps[stepStartOrder].ServiceName, // serviceNameT1 - funnelSteps[stepStartOrder].SpanName, // spanNameT1 - funnelSteps[stepEndOrder].ServiceName, // serviceNameT1 - funnelSteps[stepEndOrder].SpanName, // spanNameT2 + timeRange.StartTime, // startTs + timeRange.EndTime, // endTs + funnelSteps[0].ServiceName, // serviceNameT1 + funnelSteps[0].SpanName, // spanNameT1 + funnelSteps[1].ServiceName, // serviceNameT1 + funnelSteps[1].SpanName, // spanNameT2 clauseStep1, clauseStep2, + latencyTypeT2, ) } - return &v3.ClickHouseQuery{Query: query}, nil } @@ -341,11 +280,20 @@ func GetStepAnalytics(funnel *tracefunnel.Funnel, timeRange tracefunnel.TimeRang } // Build filter clauses for each step - clauseStep1 := buildFilterClause(funnelSteps[0].Filters) - clauseStep2 := buildFilterClause(funnelSteps[1].Filters) + clauseStep1, err := tracev4.BuildTracesFilterQuery(funnelSteps[0].Filters) + if err != nil { + return nil, err + } + clauseStep2, err := tracev4.BuildTracesFilterQuery(funnelSteps[1].Filters) + if err != nil { + return nil, err + } clauseStep3 := "" if len(funnel.Steps) > 2 { - clauseStep3 = buildFilterClause(funnelSteps[2].Filters) + clauseStep3, err = tracev4.BuildTracesFilterQuery(funnelSteps[2].Filters) + if err != nil { + return nil, err + } } if len(funnel.Steps) > 2 { @@ -404,8 +352,14 @@ func GetSlowestTraces(funnel *tracefunnel.Funnel, timeRange tracefunnel.TimeRang } // Build filter clauses for the steps - clauseStep1 := buildFilterClause(funnelSteps[stepStartOrder].Filters) - clauseStep2 := buildFilterClause(funnelSteps[stepEndOrder].Filters) + clauseStep1, err := tracev4.BuildTracesFilterQuery(funnelSteps[stepStartOrder].Filters) + if err != nil { + return nil, err + } + clauseStep2, err := tracev4.BuildTracesFilterQuery(funnelSteps[stepEndOrder].Filters) + if err != nil { + return nil, err + } query := BuildTwoStepFunnelTopSlowTracesQuery( containsErrorT1, // containsErrorT1 @@ -441,8 +395,14 @@ func GetErroredTraces(funnel *tracefunnel.Funnel, timeRange tracefunnel.TimeRang } // Build filter clauses for the steps - clauseStep1 := buildFilterClause(funnelSteps[stepStartOrder].Filters) - clauseStep2 := buildFilterClause(funnelSteps[stepEndOrder].Filters) + clauseStep1, err := tracev4.BuildTracesFilterQuery(funnelSteps[stepStartOrder].Filters) + if err != nil { + return nil, err + } + clauseStep2, err := tracev4.BuildTracesFilterQuery(funnelSteps[stepEndOrder].Filters) + if err != nil { + return nil, err + } query := BuildTwoStepFunnelTopSlowErrorTracesQuery( containsErrorT1, // containsErrorT1 diff --git a/pkg/modules/tracefunnel/utils.go b/pkg/modules/tracefunnel/utils.go index df14a6b8d488..e2ab16cd7980 100644 --- a/pkg/modules/tracefunnel/utils.go +++ b/pkg/modules/tracefunnel/utils.go @@ -8,7 +8,7 @@ import ( "github.com/SigNoz/signoz/pkg/errors" "github.com/SigNoz/signoz/pkg/types/authtypes" - tracefunnel "github.com/SigNoz/signoz/pkg/types/tracefunnel" + "github.com/SigNoz/signoz/pkg/types/tracefunnel" "github.com/SigNoz/signoz/pkg/valuer" ) diff --git a/pkg/modules/tracefunnel/utils_test.go b/pkg/modules/tracefunnel/utils_test.go index 17556b8a738f..023d0da4a4f0 100644 --- a/pkg/modules/tracefunnel/utils_test.go +++ b/pkg/modules/tracefunnel/utils_test.go @@ -8,7 +8,7 @@ import ( "github.com/SigNoz/signoz/pkg/types" "github.com/SigNoz/signoz/pkg/types/authtypes" - tracefunnel "github.com/SigNoz/signoz/pkg/types/tracefunnel" + "github.com/SigNoz/signoz/pkg/types/tracefunnel" "github.com/SigNoz/signoz/pkg/valuer" "github.com/stretchr/testify/assert" ) diff --git a/pkg/query-service/app/traces/v3/query_builder.go b/pkg/query-service/app/traces/v3/query_builder.go index a7e11c02bae5..ded13cdafbd6 100644 --- a/pkg/query-service/app/traces/v3/query_builder.go +++ b/pkg/query-service/app/traces/v3/query_builder.go @@ -144,7 +144,7 @@ func getZerosForEpochNano(epoch int64) int64 { return int64(math.Pow(10, float64(19-count))) } -func BuildTracesFilterQuery(fs *v3.FilterSet) (string, error) { +func buildTracesFilterQuery(fs *v3.FilterSet) (string, error) { var conditions []string if fs != nil && len(fs.Items) != 0 { @@ -228,14 +228,14 @@ func handleEmptyValuesInGroupBy(groupBy []v3.AttributeKey) (string, error) { Operator: "AND", Items: filterItems, } - return BuildTracesFilterQuery(&filterSet) + return buildTracesFilterQuery(&filterSet) } return "", nil } func buildTracesQuery(start, end, step int64, mq *v3.BuilderQuery, _ string, panelType v3.PanelType, options v3.QBOptions) (string, error) { - filterSubQuery, err := BuildTracesFilterQuery(mq.Filters) + filterSubQuery, err := buildTracesFilterQuery(mq.Filters) if err != nil { return "", err } diff --git a/pkg/query-service/app/traces/v3/query_builder_test.go b/pkg/query-service/app/traces/v3/query_builder_test.go index 424d57396cd8..e2d953720ab1 100644 --- a/pkg/query-service/app/traces/v3/query_builder_test.go +++ b/pkg/query-service/app/traces/v3/query_builder_test.go @@ -133,7 +133,7 @@ var buildFilterQueryData = []struct { func TestBuildTracesFilterQuery(t *testing.T) { for _, tt := range buildFilterQueryData { Convey("TestBuildTracesFilterQuery", t, func() { - query, err := BuildTracesFilterQuery(tt.FilterSet) + query, err := buildTracesFilterQuery(tt.FilterSet) So(err, ShouldBeNil) So(query, ShouldEqual, tt.ExpectedFilter) }) diff --git a/pkg/query-service/app/traces/v4/query_builder.go b/pkg/query-service/app/traces/v4/query_builder.go index 7b2befdd95a8..69a76de4d18e 100644 --- a/pkg/query-service/app/traces/v4/query_builder.go +++ b/pkg/query-service/app/traces/v4/query_builder.go @@ -87,7 +87,7 @@ func existsSubQueryForFixedColumn(key v3.AttributeKey, op v3.FilterOperator) (st } } -func buildTracesFilterQuery(fs *v3.FilterSet) (string, error) { +func BuildTracesFilterQuery(fs *v3.FilterSet) (string, error) { var conditions []string if fs != nil && len(fs.Items) != 0 { @@ -167,7 +167,7 @@ func handleEmptyValuesInGroupBy(groupBy []v3.AttributeKey) (string, error) { Operator: "AND", Items: filterItems, } - return buildTracesFilterQuery(&filterSet) + return BuildTracesFilterQuery(&filterSet) } return "", nil } @@ -248,7 +248,7 @@ func buildTracesQuery(start, end, step int64, mq *v3.BuilderQuery, panelType v3. timeFilter := fmt.Sprintf("(timestamp >= '%d' AND timestamp <= '%d') AND (ts_bucket_start >= %d AND ts_bucket_start <= %d)", tracesStart, tracesEnd, bucketStart, bucketEnd) - filterSubQuery, err := buildTracesFilterQuery(mq.Filters) + filterSubQuery, err := BuildTracesFilterQuery(mq.Filters) if err != nil { return "", err } diff --git a/pkg/query-service/app/traces/v4/query_builder_test.go b/pkg/query-service/app/traces/v4/query_builder_test.go index eff4070b5402..894308316255 100644 --- a/pkg/query-service/app/traces/v4/query_builder_test.go +++ b/pkg/query-service/app/traces/v4/query_builder_test.go @@ -211,7 +211,7 @@ func Test_buildTracesFilterQuery(t *testing.T) { want: "", }, { - name: "Test buildTracesFilterQuery in, nin", + name: "Test BuildTracesFilterQuery in, nin", args: args{ fs: &v3.FilterSet{Operator: "AND", Items: []v3.FilterItem{ {Key: v3.AttributeKey{Key: "method", DataType: v3.AttributeKeyDataTypeString, Type: v3.AttributeKeyTypeTag}, Value: []interface{}{"GET", "POST"}, Operator: v3.FilterOperatorIn}, @@ -226,7 +226,7 @@ func Test_buildTracesFilterQuery(t *testing.T) { wantErr: false, }, { - name: "Test buildTracesFilterQuery not eq, neq, gt, lt, gte, lte", + name: "Test BuildTracesFilterQuery not eq, neq, gt, lt, gte, lte", args: args{ fs: &v3.FilterSet{Operator: "AND", Items: []v3.FilterItem{ {Key: v3.AttributeKey{Key: "duration", DataType: v3.AttributeKeyDataTypeInt64, Type: v3.AttributeKeyTypeTag}, Value: 102, Operator: v3.FilterOperatorEqual}, @@ -274,13 +274,13 @@ func Test_buildTracesFilterQuery(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := buildTracesFilterQuery(tt.args.fs) + got, err := BuildTracesFilterQuery(tt.args.fs) if (err != nil) != tt.wantErr { - t.Errorf("buildTracesFilterQuery() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("BuildTracesFilterQuery() error = %v, wantErr %v", err, tt.wantErr) return } if got != tt.want { - t.Errorf("buildTracesFilterQuery() = %v, want %v", got, tt.want) + t.Errorf("BuildTracesFilterQuery() = %v, want %v", got, tt.want) } }) }