Skip to content

Commit

Permalink
Handle empty series correctly (#86)
Browse files Browse the repository at this point in the history
Behaviour of this changed in thanos-io/thanos#3010 -- this now generates an error, as it probably should, but we sent back a series with no datapoints in it previously.
  • Loading branch information
dgl committed Nov 30, 2020
1 parent 87be6a0 commit e9388db
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 14 deletions.
17 changes: 13 additions & 4 deletions pkg/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,11 @@ func (store *OpenTSDBStore) Series(
if err != nil {
return err
}
if res == nil {
// OpenTSDB can return a series with no datapoints, in Prometheus this
// is just not returned.
continue
}
if err := server.Send(res); err != nil {
return err
}
Expand Down Expand Up @@ -650,10 +655,14 @@ func (store *OpenTSDBStore) convertOpenTSDBResultsToSeriesResponse(respI *opents
}
chunks = append(chunks, aggrChunk)
}
return storepb.NewSeriesResponse(&storepb.Series{
Labels: seriesLabels,
Chunks: chunks,
}), len(dps), nil
var response *storepb.SeriesResponse
if len(chunks) > 0 {
response = storepb.NewSeriesResponse(&storepb.Series{
Labels: seriesLabels,
Chunks: chunks,
})
}
return response, len(dps), nil
}

func convertPromQLMatcherToFilter(matcher storepb.LabelMatcher) (opentsdb.Filter, error) {
Expand Down
27 changes: 17 additions & 10 deletions pkg/store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -979,23 +979,21 @@ func TestConvertOpenTSDBResultsToSeriesResponse(t *testing.T) {
Tags: map[string]string{},
Dps: newDps(map[string]interface{}{}),
},
expectedOutput: storepb.NewSeriesResponse(&storepb.Series{
Labels: []storepb.Label{{Name: "__name__", Value: "metric"}},
Chunks: []storepb.AggrChunk{},
}),
expectedChunkTypes: []storepb.Aggr{},
expectedOutput: nil,
},
{
input: opentsdb.QueryRespItem{
Metric: "metric.with.dot.and-dash",
Tags: map[string]string{},
Dps: newDps(map[string]interface{}{}),
Dps: newDps(map[string]interface{}{
"1": 1.0,
}),
},
expectedOutput: storepb.NewSeriesResponse(&storepb.Series{
Labels: []storepb.Label{{Name: "__name__", Value: "metric:with:dot:and_dash"}},
Chunks: []storepb.AggrChunk{},
Chunks: []storepb.AggrChunk{{MinTime: 1, MaxTime: 1}},
}),
expectedChunkTypes: []storepb.Aggr{},
expectedChunkTypes: []storepb.Aggr{storepb.Aggr_RAW},
},
{
input: opentsdb.QueryRespItem{
Expand Down Expand Up @@ -1187,13 +1185,22 @@ func TestConvertOpenTSDBResultsToSeriesResponse(t *testing.T) {
expectedChunkTypes: []storepb.Aggr{storepb.Aggr_RAW},
},
}
for _, test := range testCases {
for i, test := range testCases {
store := NewOpenTSDBStore(
log.NewJSONLogger(&testLogger{t}), nil, nil, time.Duration(0), 1*time.Minute, test.storeLabels, nil, nil, false, true, "foo")
converted, _, err := store.convertOpenTSDBResultsToSeriesResponse(&test.input)
if err != nil {
t.Errorf("unexpected error: %s", err.Error())
}
if converted == nil && test.expectedOutput == nil {
continue // Expected nothing, got nothing.
}
if test.expectedOutput == nil {
t.Errorf("expected nil, got %v", converted)
}
if converted == nil {
t.Errorf("%d: expected response, got nil", i)
}
if len(converted.GetSeries().Labels) == len(test.expectedOutput.GetSeries().Labels) {
// Labels must be alphabetically sorted
for i, label := range converted.GetSeries().Labels {
Expand All @@ -1206,7 +1213,7 @@ func TestConvertOpenTSDBResultsToSeriesResponse(t *testing.T) {
t.Errorf("number of tags does not match")
}
if len(test.expectedOutput.GetSeries().Chunks) != len(converted.GetSeries().Chunks) {
t.Error("number of chunks does not match")
t.Errorf("%d: number of chunks does not match", i)
}
for ci, chunk := range test.expectedOutput.GetSeries().Chunks {
convertedChunk := converted.GetSeries().Chunks[ci]
Expand Down

0 comments on commit e9388db

Please sign in to comment.