Compare commits

..

8 Commits

Author SHA1 Message Date
Naman Verma
404976b725 fix: wrong number changed earlier 2026-02-26 14:19:09 +05:30
Naman Verma
21e31bb157 test: cumulative histogram unit test should show start ts decrease by step interval 2026-02-26 14:10:10 +05:30
Naman Verma
40a3c7b919 test: pass time aggregation in histogram unit tests 2026-02-26 14:05:50 +05:30
Naman Verma
f80cdb71e3 Merge branch 'main' into nv/4074 2026-02-26 13:52:23 +05:30
Naman Verma
e6e484accf Merge branch 'main' into nv/4074 2026-02-26 13:40:15 +05:30
Naman Verma
fe3dfa5821 chore: check for orig time and space agg for histograms 2026-02-26 13:36:12 +05:30
Naman Verma
16e8245a1f chore: better error messages based on metric type 2026-02-26 13:34:54 +05:30
Naman Verma
22169487b1 fix: add validity check for spatial aggregation 2026-02-26 13:07:21 +05:30
4 changed files with 53 additions and 6 deletions

View File

@@ -5419,7 +5419,7 @@ func (r *ClickHouseReader) ListSummaryMetrics(ctx context.Context, orgID valuer.
t.metric_name AS metric_name,
ANY_VALUE(t.description) AS description,
ANY_VALUE(t.type) AS metric_type,
argMax(t.unit, unix_milli) AS metric_unit,
ANY_VALUE(t.unit) AS metric_unit,
uniq(t.fingerprint) AS timeseries,
uniq(metric_name) OVER() AS total
FROM %s.%s AS t

View File

@@ -159,6 +159,23 @@ func (b *MetricQueryStatementBuilder) buildPipelineStatement(
query.Aggregations[0].TimeAggregation = metrictypes.TimeAggregationIncrease
}
query.Aggregations[0].SpaceAggregation = metrictypes.SpaceAggregationSum
// check for origTimeAgg's and origSpaceAgg's validity
if origTimeAgg.IsZero() || !origTimeAgg.IsValid() {
return nil, errors.Newf(
errors.TypeInvalidInput,
errors.CodeInvalidInput,
"invalid time aggregation, should be one of the following: [`rate`, `increase`]",
)
}
if origSpaceAgg.IsZero() || !origSpaceAgg.IsValid() {
return nil, errors.Newf(
errors.TypeInvalidInput,
errors.CodeInvalidInput,
"invalid space aggregation, should be one of the following: [`count`, `p50`, `p75`, `p90`, `p95`, `p99`]",
)
}
}
var timeSeriesCTE string
@@ -523,11 +540,26 @@ func (b *MetricQueryStatementBuilder) buildSpatialAggregationCTE(
query qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation],
_ map[string][]*telemetrytypes.TelemetryFieldKey,
) (string, []any, error) {
if query.Aggregations[0].SpaceAggregation.IsZero() {
if query.Aggregations[0].SpaceAggregation.IsZero() || !query.Aggregations[0].SpaceAggregation.IsValid() {
if query.Aggregations[0].Type.IsPercentileSpaceAggregationAllowed() {
return "", nil, errors.Newf(
errors.TypeInvalidInput,
errors.CodeInvalidInput,
"invalid space aggregation, should be one of the following: [`sum`, `avg`, `min`, `max`, `count`, `p50`, `p75`, `p90`, `p95`, `p99`]",
)
} else {
return "", nil, errors.Newf(
errors.TypeInvalidInput,
errors.CodeInvalidInput,
"invalid space aggregation, should be one of the following: [`sum`, `avg`, `min`, `max`, `count`]",
)
}
}
if query.Aggregations[0].SpaceAggregation.IsPercentile() && !query.Aggregations[0].Type.IsPercentileSpaceAggregationAllowed() {
return "", nil, errors.Newf(
errors.TypeInvalidInput,
errors.CodeInvalidInput,
"invalid space aggregation, should be one of the following: [`sum`, `avg`, `min`, `max`, `count`, `p50`, `p75`, `p90`, `p95`, `p99`]",
"percentile based aggregations are invalid for this metric, should be one of the following: [`sum`, `avg`, `min`, `max`, `count`]",
)
}
sb := sqlbuilder.NewSelectBuilder()

View File

@@ -122,7 +122,7 @@ func TestStatementBuilder(t *testing.T) {
expectedErr: nil,
},
{
name: "test_histogram_percentile",
name: "test_histogram_percentile1",
requestType: qbtypes.RequestTypeTimeSeries,
query: qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]{
Signal: telemetrytypes.SignalMetrics,
@@ -132,6 +132,7 @@ func TestStatementBuilder(t *testing.T) {
MetricName: "signoz_latency",
Type: metrictypes.HistogramType,
Temporality: metrictypes.Delta,
TimeAggregation: metrictypes.TimeAggregationRate,
SpaceAggregation: metrictypes.SpaceAggregationPercentile95,
},
},
@@ -187,7 +188,7 @@ func TestStatementBuilder(t *testing.T) {
expectedErr: nil,
},
{
name: "test_histogram_percentile",
name: "test_histogram_percentile2",
requestType: qbtypes.RequestTypeTimeSeries,
query: qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation]{
Signal: telemetrytypes.SignalMetrics,
@@ -197,6 +198,7 @@ func TestStatementBuilder(t *testing.T) {
MetricName: "http_server_duration_bucket",
Type: metrictypes.HistogramType,
Temporality: metrictypes.Cumulative,
TimeAggregation: metrictypes.TimeAggregationRate,
SpaceAggregation: metrictypes.SpaceAggregationPercentile95,
},
},
@@ -211,7 +213,7 @@ func TestStatementBuilder(t *testing.T) {
},
expected: qbtypes.Statement{
Query: "WITH __temporal_aggregation_cte AS (SELECT ts, `service.name`, `le`, multiIf(row_number() OVER rate_window = 1, nan, (per_series_value - lagInFrame(per_series_value, 1) OVER rate_window) < 0, per_series_value / (ts - lagInFrame(ts, 1) OVER rate_window), (per_series_value - lagInFrame(per_series_value, 1) OVER rate_window) / (ts - lagInFrame(ts, 1) OVER rate_window)) AS per_series_value FROM (SELECT fingerprint, toStartOfInterval(toDateTime(intDiv(unix_milli, 1000)), toIntervalSecond(30)) AS ts, `service.name`, `le`, max(value) AS per_series_value FROM signoz_metrics.distributed_samples_v4 AS points INNER JOIN (SELECT fingerprint, JSONExtractString(labels, 'service.name') AS `service.name`, JSONExtractString(labels, 'le') AS `le` FROM signoz_metrics.time_series_v4_6hrs WHERE metric_name IN (?) AND unix_milli >= ? AND unix_milli <= ? AND LOWER(temporality) LIKE LOWER(?) AND __normalized = ? GROUP BY fingerprint, `service.name`, `le`) AS filtered_time_series ON points.fingerprint = filtered_time_series.fingerprint WHERE metric_name IN (?) AND unix_milli >= ? AND unix_milli < ? GROUP BY fingerprint, ts, `service.name`, `le` ORDER BY fingerprint, ts) WINDOW rate_window AS (PARTITION BY fingerprint ORDER BY fingerprint, ts)), __spatial_aggregation_cte AS (SELECT ts, `service.name`, `le`, sum(per_series_value) AS value FROM __temporal_aggregation_cte WHERE isNaN(per_series_value) = ? GROUP BY ts, `service.name`, `le`) SELECT ts, `service.name`, histogramQuantile(arrayMap(x -> toFloat64(x), groupArray(le)), groupArray(value), 0.950) AS value FROM __spatial_aggregation_cte GROUP BY `service.name`, ts ORDER BY `service.name`, ts",
Args: []any{"http_server_duration_bucket", uint64(1747936800000), uint64(1747983420000), "cumulative", false, "http_server_duration_bucket", uint64(1747947390000), uint64(1747983420000), 0},
Args: []any{"http_server_duration_bucket", uint64(1747936800000), uint64(1747983420000), "cumulative", false, "http_server_duration_bucket", uint64(1747947360000), uint64(1747983420000), 0},
},
expectedErr: nil,
},

View File

@@ -3,6 +3,7 @@ package metrictypes
import (
"database/sql/driver"
"fmt"
"slices"
"strings"
"github.com/SigNoz/signoz/pkg/errors"
@@ -135,6 +136,10 @@ func (t *Type) Scan(src interface{}) error {
return nil
}
func (t Type) IsPercentileSpaceAggregationAllowed() bool {
return t == HistogramType || t == ExpHistogramType || t == SummaryType
}
var (
GaugeType = Type{valuer.NewString("gauge")}
SumType = Type{valuer.NewString("sum")}
@@ -185,6 +190,10 @@ func (TimeAggregation) Enum() []any {
}
}
func (t TimeAggregation) IsValid() bool {
return slices.ContainsFunc(t.Enum(), func(v any) bool { return v == t })
}
type SpaceAggregation struct {
valuer.String
}
@@ -218,6 +227,10 @@ func (SpaceAggregation) Enum() []any {
}
}
func (s SpaceAggregation) IsValid() bool {
return slices.ContainsFunc(s.Enum(), func(v any) bool { return v == s })
}
func (s SpaceAggregation) IsPercentile() bool {
return s == SpaceAggregationPercentile50 ||
s == SpaceAggregationPercentile75 ||