Skip to content

Commit 4fc3f7e

Browse files
authored
feat: add validation for exemplar types in timeseries queries (#4631)
1 parent 9e07b6b commit 4fc3f7e

File tree

2 files changed

+90
-1
lines changed

2 files changed

+90
-1
lines changed

pkg/querybackend/query_time_series.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77

88
"github.com/grafana/dskit/runutil"
99
"github.com/parquet-go/parquet-go"
10+
"google.golang.org/grpc/codes"
11+
"google.golang.org/grpc/status"
1012

1113
queryv1 "github.com/grafana/pyroscope/api/gen/proto/go/query/v1"
1214
typesv1 "github.com/grafana/pyroscope/api/gen/proto/go/types/v1"
@@ -31,11 +33,15 @@ func init() {
3133
}
3234

3335
func queryTimeSeries(q *queryContext, query *queryv1.Query) (r *queryv1.Report, err error) {
36+
includeExemplars, err := validateExemplarType(query.TimeSeries.ExemplarType)
37+
if err != nil {
38+
return nil, err
39+
}
40+
3441
opts := []profileIteratorOption{
3542
withFetchPartition(false), // Partition data not needed, as we don't access stacktraces at all
3643
}
3744

38-
includeExemplars := query.TimeSeries.ExemplarType == typesv1.ExemplarType_EXEMPLAR_TYPE_INDIVIDUAL
3945
if includeExemplars {
4046
opts = append(opts,
4147
withAllLabels(),
@@ -166,3 +172,18 @@ func (a *timeSeriesAggregator) build() *queryv1.Report {
166172
},
167173
}
168174
}
175+
176+
// validateExemplarType validates the exemplar type and returns whether to include exemplars.
177+
func validateExemplarType(exemplarType typesv1.ExemplarType) (bool, error) {
178+
switch exemplarType {
179+
case typesv1.ExemplarType_EXEMPLAR_TYPE_UNSPECIFIED,
180+
typesv1.ExemplarType_EXEMPLAR_TYPE_NONE:
181+
return false, nil
182+
case typesv1.ExemplarType_EXEMPLAR_TYPE_INDIVIDUAL:
183+
return true, nil
184+
case typesv1.ExemplarType_EXEMPLAR_TYPE_SPAN:
185+
return false, status.Error(codes.Unimplemented, "exemplar type span is not implemented")
186+
default:
187+
return false, status.Errorf(codes.InvalidArgument, "unknown exemplar type: %v", exemplarType)
188+
}
189+
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package querybackend
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
"google.golang.org/grpc/codes"
9+
"google.golang.org/grpc/status"
10+
11+
typesv1 "github.com/grafana/pyroscope/api/gen/proto/go/types/v1"
12+
)
13+
14+
func TestValidateExemplarType(t *testing.T) {
15+
tests := []struct {
16+
name string
17+
exemplarType typesv1.ExemplarType
18+
expectedInclude bool
19+
expectedErrorMsg string
20+
expectedCode codes.Code
21+
}{
22+
{
23+
name: "UNSPECIFIED returns false, no error",
24+
exemplarType: typesv1.ExemplarType_EXEMPLAR_TYPE_UNSPECIFIED,
25+
expectedInclude: false,
26+
},
27+
{
28+
name: "NONE returns false, no error",
29+
exemplarType: typesv1.ExemplarType_EXEMPLAR_TYPE_NONE,
30+
expectedInclude: false,
31+
},
32+
{
33+
name: "INDIVIDUAL returns true, no error",
34+
exemplarType: typesv1.ExemplarType_EXEMPLAR_TYPE_INDIVIDUAL,
35+
expectedInclude: true,
36+
},
37+
{
38+
name: "SPAN returns error with Unimplemented code",
39+
exemplarType: typesv1.ExemplarType_EXEMPLAR_TYPE_SPAN,
40+
expectedInclude: false,
41+
expectedErrorMsg: "exemplar type span is not implemented",
42+
expectedCode: codes.Unimplemented,
43+
},
44+
{
45+
name: "Unknown type returns error with InvalidArgument code",
46+
exemplarType: typesv1.ExemplarType(999),
47+
expectedInclude: false,
48+
expectedErrorMsg: "unknown exemplar type",
49+
expectedCode: codes.InvalidArgument,
50+
},
51+
}
52+
53+
for _, tt := range tests {
54+
t.Run(tt.name, func(t *testing.T) {
55+
include, err := validateExemplarType(tt.exemplarType)
56+
if tt.expectedErrorMsg != "" {
57+
require.Error(t, err)
58+
assert.Contains(t, err.Error(), tt.expectedErrorMsg)
59+
st, ok := status.FromError(err)
60+
require.True(t, ok)
61+
assert.Equal(t, tt.expectedCode, st.Code())
62+
} else {
63+
require.NoError(t, err)
64+
assert.Equal(t, tt.expectedInclude, include)
65+
}
66+
})
67+
}
68+
}

0 commit comments

Comments
 (0)