From 3572baa5eb3f33fa800d02ee920f3cb79ae099ad Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 29 Dec 2023 12:35:22 +0530 Subject: [PATCH] fix: adjust the start and end more accurately (#4263) * fix: adjust the start and end more accurately Part of https://github.com/SigNoz/signoz/issues/1327 * chore: cache friendly timestamps --- .../app/metrics/v3/query_builder.go | 13 +++++--- .../app/metrics/v3/query_builder_test.go | 32 +++++++++++-------- pkg/query-service/app/querier/querier_test.go | 6 ++-- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/pkg/query-service/app/metrics/v3/query_builder.go b/pkg/query-service/app/metrics/v3/query_builder.go index ccb4bc0d30..01b860c968 100644 --- a/pkg/query-service/app/metrics/v3/query_builder.go +++ b/pkg/query-service/app/metrics/v3/query_builder.go @@ -2,6 +2,7 @@ package v3 import ( "fmt" + "math" "strings" "time" @@ -172,7 +173,7 @@ func buildMetricQuery(start, end, step int64, mq *v3.BuilderQuery, tableName str return "", err } - samplesTableTimeFilter := fmt.Sprintf("metric_name = %s AND timestamp_ms >= %d AND timestamp_ms <= %d", utils.ClickHouseFormattedValue(mq.AggregateAttribute.Key), start, end) + samplesTableTimeFilter := fmt.Sprintf("metric_name = %s AND timestamp_ms >= %d AND timestamp_ms < %d", utils.ClickHouseFormattedValue(mq.AggregateAttribute.Key), start, end) // Select the aggregate value for interval queryTmpl := @@ -447,10 +448,14 @@ func reduceQuery(query string, reduceTo v3.ReduceToOperator, aggregateOperator v // start and end are in milliseconds // step is in seconds func PrepareMetricQuery(start, end int64, queryType v3.QueryType, panelType v3.PanelType, mq *v3.BuilderQuery, options Options) (string, error) { - - // adjust the start and end time to be aligned with the step interval start = start - (start % (mq.StepInterval * 1000)) - end = end - (end % (mq.StepInterval * 1000)) + // if the query is a rate query, we adjust the start time by one more step + // so that we can calculate the rate for the first data point + if mq.AggregateOperator.IsRateOperator() && mq.Temporality != v3.Delta { + start -= mq.StepInterval * 1000 + } + adjustStep := int64(math.Min(float64(mq.StepInterval), 60)) + end = end - (end % (adjustStep * 1000)) var query string var err error diff --git a/pkg/query-service/app/metrics/v3/query_builder_test.go b/pkg/query-service/app/metrics/v3/query_builder_test.go index c7f9ae47b3..2ad6013de6 100644 --- a/pkg/query-service/app/metrics/v3/query_builder_test.go +++ b/pkg/query-service/app/metrics/v3/query_builder_test.go @@ -245,7 +245,7 @@ func TestBuildQueryOperators(t *testing.T) { func TestBuildQueryXRate(t *testing.T) { t.Run("TestBuildQueryXRate", func(t *testing.T) { - tmpl := `SELECT ts, %s(rate_value) as value FROM (SELECT ts, If((value - lagInFrame(value, 1, 0) OVER rate_window) < 0, nan, If((ts - lagInFrame(ts, 1, toDate('1970-01-01')) OVER rate_window) >= 86400, nan, (value - lagInFrame(value, 1, 0) OVER rate_window) / (ts - lagInFrame(ts, 1, toDate('1970-01-01')) OVER rate_window))) as rate_value FROM(SELECT fingerprint, toStartOfInterval(toDateTime(intDiv(timestamp_ms, 1000)), INTERVAL 60 SECOND) as ts, max(value) as value FROM signoz_metrics.distributed_samples_v2 INNER JOIN (SELECT fingerprint FROM signoz_metrics.time_series_v2 WHERE metric_name = 'name' AND temporality IN ['Cumulative', 'Unspecified']) as filtered_time_series USING fingerprint WHERE metric_name = 'name' AND timestamp_ms >= 1650991980000 AND timestamp_ms <= 1651078380000 GROUP BY fingerprint, ts ORDER BY fingerprint, ts) WINDOW rate_window as (PARTITION BY fingerprint ORDER BY fingerprint, ts) ) WHERE isNaN(rate_value) = 0 GROUP BY ts ORDER BY ts` + tmpl := `SELECT ts, %s(rate_value) as value FROM (SELECT ts, If((value - lagInFrame(value, 1, 0) OVER rate_window) < 0, nan, If((ts - lagInFrame(ts, 1, toDate('1970-01-01')) OVER rate_window) >= 86400, nan, (value - lagInFrame(value, 1, 0) OVER rate_window) / (ts - lagInFrame(ts, 1, toDate('1970-01-01')) OVER rate_window))) as rate_value FROM(SELECT fingerprint, toStartOfInterval(toDateTime(intDiv(timestamp_ms, 1000)), INTERVAL 60 SECOND) as ts, max(value) as value FROM signoz_metrics.distributed_samples_v2 INNER JOIN (SELECT fingerprint FROM signoz_metrics.time_series_v2 WHERE metric_name = 'name' AND temporality IN ['Cumulative', 'Unspecified']) as filtered_time_series USING fingerprint WHERE metric_name = 'name' AND timestamp_ms >= 1650991920000 AND timestamp_ms < 1651078380000 GROUP BY fingerprint, ts ORDER BY fingerprint, ts) WINDOW rate_window as (PARTITION BY fingerprint ORDER BY fingerprint, ts) ) WHERE isNaN(rate_value) = 0 GROUP BY ts ORDER BY ts` cases := []struct { aggregateOperator v3.AggregateOperator @@ -298,7 +298,7 @@ func TestBuildQueryXRate(t *testing.T) { func TestBuildQueryRPM(t *testing.T) { t.Run("TestBuildQueryXRate", func(t *testing.T) { - tmpl := `SELECT ts, ceil(value * 60) as value FROM (SELECT ts, %s(rate_value) as value FROM (SELECT ts, If((value - lagInFrame(value, 1, 0) OVER rate_window) < 0, nan, If((ts - lagInFrame(ts, 1, toDate('1970-01-01')) OVER rate_window) >= 86400, nan, (value - lagInFrame(value, 1, 0) OVER rate_window) / (ts - lagInFrame(ts, 1, toDate('1970-01-01')) OVER rate_window))) as rate_value FROM(SELECT fingerprint, toStartOfInterval(toDateTime(intDiv(timestamp_ms, 1000)), INTERVAL 60 SECOND) as ts, max(value) as value FROM signoz_metrics.distributed_samples_v2 INNER JOIN (SELECT fingerprint FROM signoz_metrics.time_series_v2 WHERE metric_name = 'name' AND temporality IN ['Cumulative', 'Unspecified']) as filtered_time_series USING fingerprint WHERE metric_name = 'name' AND timestamp_ms >= 1650991980000 AND timestamp_ms <= 1651078380000 GROUP BY fingerprint, ts ORDER BY fingerprint, ts) WINDOW rate_window as (PARTITION BY fingerprint ORDER BY fingerprint, ts) ) WHERE isNaN(rate_value) = 0 GROUP BY ts ORDER BY ts)` + tmpl := `SELECT ts, ceil(value * 60) as value FROM (SELECT ts, %s(rate_value) as value FROM (SELECT ts, If((value - lagInFrame(value, 1, 0) OVER rate_window) < 0, nan, If((ts - lagInFrame(ts, 1, toDate('1970-01-01')) OVER rate_window) >= 86400, nan, (value - lagInFrame(value, 1, 0) OVER rate_window) / (ts - lagInFrame(ts, 1, toDate('1970-01-01')) OVER rate_window))) as rate_value FROM(SELECT fingerprint, toStartOfInterval(toDateTime(intDiv(timestamp_ms, 1000)), INTERVAL 60 SECOND) as ts, max(value) as value FROM signoz_metrics.distributed_samples_v2 INNER JOIN (SELECT fingerprint FROM signoz_metrics.time_series_v2 WHERE metric_name = 'name' AND temporality IN ['Cumulative', 'Unspecified']) as filtered_time_series USING fingerprint WHERE metric_name = 'name' AND timestamp_ms >= 1650991920000 AND timestamp_ms < 1651078380000 GROUP BY fingerprint, ts ORDER BY fingerprint, ts) WINDOW rate_window as (PARTITION BY fingerprint ORDER BY fingerprint, ts) ) WHERE isNaN(rate_value) = 0 GROUP BY ts ORDER BY ts)` cases := []struct { aggregateOperator v3.AggregateOperator @@ -376,8 +376,8 @@ func TestBuildQueryAdjustedTimes(t *testing.T) { }, }, }, - // 20:11:00 - 20:41:00 - expected: "timestamp_ms >= 1686082260000 AND timestamp_ms <= 1686084060000", + // 20:10:00 - 20:41:00 + expected: "timestamp_ms >= 1686082200000 AND timestamp_ms < 1686084060000", }, { name: "TestBuildQueryAdjustedTimes start close to 50 seconds", @@ -401,8 +401,8 @@ func TestBuildQueryAdjustedTimes(t *testing.T) { }, }, }, - // 20:11:00 - 20:41:00 - expected: "timestamp_ms >= 1686082260000 AND timestamp_ms <= 1686084060000", + // 20:10:00 - 20:41:00 + expected: "timestamp_ms >= 1686082200000 AND timestamp_ms < 1686084060000", }, { name: "TestBuildQueryAdjustedTimes start close to 42 seconds with step 30 seconds", @@ -426,8 +426,8 @@ func TestBuildQueryAdjustedTimes(t *testing.T) { }, }, }, - // 20:11:30 - 20:41:00 - expected: "timestamp_ms >= 1686082290000 AND timestamp_ms <= 1686084060000", + // 20:11:00 - 20:41:00 + expected: "timestamp_ms >= 1686082260000 AND timestamp_ms < 1686084060000", }, { name: "TestBuildQueryAdjustedTimes start close to 42 seconds with step 30 seconds and end close to 30 seconds", @@ -451,8 +451,8 @@ func TestBuildQueryAdjustedTimes(t *testing.T) { }, }, }, - // 20:11:30 - 20:41:00 - expected: "timestamp_ms >= 1686082290000 AND timestamp_ms <= 1686084060000", + // 20:11:00 - 20:41:00 + expected: "timestamp_ms >= 1686082260000 AND timestamp_ms < 1686084060000", }, { name: "TestBuildQueryAdjustedTimes start close to 42 seconds with step 300 seconds and end close to 30 seconds", @@ -476,8 +476,10 @@ func TestBuildQueryAdjustedTimes(t *testing.T) { }, }, }, - // 20:10:00 - 20:40:00 - expected: "timestamp_ms >= 1686082200000 AND timestamp_ms <= 1686084000000", + // 20:05:00 - 20:41:00 + // 20:10:00 is the nearest 5 minute interval, but we round down to 20:05:00 + // as this is a rate query and we want to include the previous value for the first interval + expected: "timestamp_ms >= 1686081900000 AND timestamp_ms < 1686084060000", }, { name: "TestBuildQueryAdjustedTimes start close to 42 seconds with step 180 seconds and end close to 30 seconds", @@ -501,8 +503,10 @@ func TestBuildQueryAdjustedTimes(t *testing.T) { }, }, }, - // 20:09:00 - 20:39:00 - expected: "timestamp_ms >= 1686082140000 AND timestamp_ms <= 1686083940000", + // 20:06:00 - 20:39:00 + // 20:09:00 is the nearest 3 minute interval, but we round down to 20:06:00 + // as this is a rate query and we want to include the previous value for the first interval + expected: "timestamp_ms >= 1686081960000 AND timestamp_ms < 1686084060000", }, } diff --git a/pkg/query-service/app/querier/querier_test.go b/pkg/query-service/app/querier/querier_test.go index f4427a739a..f08ae82dcd 100644 --- a/pkg/query-service/app/querier/querier_test.go +++ b/pkg/query-service/app/querier/querier_test.go @@ -558,8 +558,8 @@ func TestQueryRange(t *testing.T) { } q := NewQuerier(opts) expectedTimeRangeInQueryString := []string{ - fmt.Sprintf("timestamp_ms >= %d AND timestamp_ms <= %d", 1675115580000, 1675115580000+120*60*1000), - fmt.Sprintf("timestamp_ms >= %d AND timestamp_ms <= %d", 1675115580000+120*60*1000, 1675115580000+180*60*1000), + fmt.Sprintf("timestamp_ms >= %d AND timestamp_ms < %d", 1675115520000, 1675115580000+120*60*1000), + fmt.Sprintf("timestamp_ms >= %d AND timestamp_ms < %d", 1675115520000+120*60*1000, 1675115580000+180*60*1000), fmt.Sprintf("timestamp >= '%d' AND timestamp <= '%d'", 1675115580000*1000000, (1675115580000+120*60*1000)*int64(1000000)), fmt.Sprintf("timestamp >= '%d' AND timestamp <= '%d'", (1675115580000+60*60*1000)*int64(1000000), (1675115580000+180*60*1000)*int64(1000000)), } @@ -669,7 +669,7 @@ func TestQueryRangeValueType(t *testing.T) { q := NewQuerier(opts) // No caching expectedTimeRangeInQueryString := []string{ - fmt.Sprintf("timestamp_ms >= %d AND timestamp_ms <= %d", 1675115580000, 1675115580000+120*60*1000), + fmt.Sprintf("timestamp_ms >= %d AND timestamp_ms < %d", 1675115520000, 1675115580000+120*60*1000), fmt.Sprintf("timestamp >= '%d' AND timestamp <= '%d'", (1675115580000+60*60*1000)*int64(1000000), (1675115580000+180*60*1000)*int64(1000000)), }