Skip to content

Commit 14e1002

Browse files
justinjung04justinjung04
committed
Add more tests
1 parent cb24add commit 14e1002

File tree

4 files changed

+90
-51
lines changed

4 files changed

+90
-51
lines changed

‎pkg/distributor/distributor.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -1307,7 +1307,7 @@ func (d *Distributor) LabelNamesCommon(ctx context.Context, from, to model.Time,
13071307
return r, nil
13081308
}
13091309

1310-
func (d *Distributor) LabelNamesStream(ctx context.Context, from model.Time, to model.Time, hints *storage.LabelHints, partialDataEnabled bool, matchers ...*labels.Matcher) ([]string, error) {
1310+
func (d *Distributor) LabelNamesStream(ctx context.Context, from, to model.Time, hints *storage.LabelHints, partialDataEnabled bool, matchers ...*labels.Matcher) ([]string, error) {
13111311
return d.LabelNamesCommon(ctx, from, to, hints, func(ctx context.Context, rs ring.ReplicationSet, req *ingester_client.LabelNamesRequest) ([]interface{}, error) {
13121312
return d.ForReplicationSet(ctx, rs, d.cfg.ZoneResultsQuorumMetadata, partialDataEnabled, func(ctx context.Context, client ingester_client.IngesterClient) (interface{}, error) {
13131313
stream, err := client.LabelNamesStream(ctx, req)
@@ -1333,7 +1333,7 @@ func (d *Distributor) LabelNamesStream(ctx context.Context, from model.Time, to
13331333
}
13341334

13351335
// LabelNames returns all the label names.
1336-
func (d *Distributor) LabelNames(ctx context.Context, from model.Time, to model.Time, hint *storage.LabelHints, partialDataEnabled bool, matchers ...*labels.Matcher) ([]string, error) {
1336+
func (d *Distributor) LabelNames(ctx context.Context, from, to model.Time, hint *storage.LabelHints, partialDataEnabled bool, matchers ...*labels.Matcher) ([]string, error) {
13371337
return d.LabelNamesCommon(ctx, from, to, hint, func(ctx context.Context, rs ring.ReplicationSet, req *ingester_client.LabelNamesRequest) ([]interface{}, error) {
13381338
return d.ForReplicationSet(ctx, rs, d.cfg.ZoneResultsQuorumMetadata, partialDataEnabled, func(ctx context.Context, client ingester_client.IngesterClient) (interface{}, error) {
13391339
resp, err := client.LabelNames(ctx, req)
@@ -1348,7 +1348,7 @@ func (d *Distributor) LabelNames(ctx context.Context, from model.Time, to model.
13481348
// MetricsForLabelMatchers gets the metrics that match said matchers
13491349
func (d *Distributor) MetricsForLabelMatchers(ctx context.Context, from, through model.Time, hint *storage.SelectHints, partialDataEnabled bool, matchers ...*labels.Matcher) ([]model.Metric, error) {
13501350
return d.metricsForLabelMatchersCommon(ctx, from, through, hint, func(ctx context.Context, rs ring.ReplicationSet, req *ingester_client.MetricsForLabelMatchersRequest, metrics *map[model.Fingerprint]model.Metric, mutex *sync.Mutex, queryLimiter *limiter.QueryLimiter) error {
1351-
_, err := d.ForReplicationSet(ctx, rs, false, false, func(ctx context.Context, client ingester_client.IngesterClient) (interface{}, error) {
1351+
_, err := d.ForReplicationSet(ctx, rs, false, partialDataEnabled, func(ctx context.Context, client ingester_client.IngesterClient) (interface{}, error) {
13521352
resp, err := client.MetricsForLabelMatchers(ctx, req)
13531353
if err != nil {
13541354
return nil, err
@@ -1377,7 +1377,7 @@ func (d *Distributor) MetricsForLabelMatchers(ctx context.Context, from, through
13771377

13781378
func (d *Distributor) MetricsForLabelMatchersStream(ctx context.Context, from, through model.Time, hint *storage.SelectHints, partialDataEnabled bool, matchers ...*labels.Matcher) ([]model.Metric, error) {
13791379
return d.metricsForLabelMatchersCommon(ctx, from, through, hint, func(ctx context.Context, rs ring.ReplicationSet, req *ingester_client.MetricsForLabelMatchersRequest, metrics *map[model.Fingerprint]model.Metric, mutex *sync.Mutex, queryLimiter *limiter.QueryLimiter) error {
1380-
_, err := d.ForReplicationSet(ctx, rs, false, false, func(ctx context.Context, client ingester_client.IngesterClient) (interface{}, error) {
1380+
_, err := d.ForReplicationSet(ctx, rs, false, partialDataEnabled, func(ctx context.Context, client ingester_client.IngesterClient) (interface{}, error) {
13811381
stream, err := client.MetricsForLabelMatchersStream(ctx, req)
13821382
if err != nil {
13831383
return nil, err

‎pkg/querier/distributor_queryable.go

+27-4
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,17 @@ func (q *distributorQuerier) Select(ctx context.Context, sortSeries bool, sp *st
132132
ms, err = q.distributor.MetricsForLabelMatchers(ctx, model.Time(minT), model.Time(maxT), sp, partialDataEnabled, matchers...)
133133
}
134134

135-
if err != nil {
135+
if err != nil && !partialdata.IsPartialDataError(err) {
136136
return storage.ErrSeriesSet(err)
137137
}
138+
139+
seriesSet := series.MetricsToSeriesSet(ctx, sortSeries, ms)
140+
141+
if partialdata.IsPartialDataError(err) {
142+
warning := seriesSet.Warnings()
143+
return series.NewSeriesSetWithWarnings(seriesSet, warning.Add(err))
144+
}
145+
138146
return series.MetricsToSeriesSet(ctx, sortSeries, ms)
139147
}
140148

@@ -177,8 +185,8 @@ func (q *distributorQuerier) streamingSelect(ctx context.Context, sortSeries, pa
177185
seriesSet := series.NewConcreteSeriesSet(sortSeries, serieses)
178186

179187
if partialdata.IsPartialDataError(err) {
180-
warning := seriesSet.Warnings()
181-
return series.NewSeriesSetWithWarnings(seriesSet, warning.Add(err))
188+
warnings := seriesSet.Warnings()
189+
return series.NewSeriesSetWithWarnings(seriesSet, warnings.Add(err))
182190
}
183191

184192
return seriesSet
@@ -198,6 +206,11 @@ func (q *distributorQuerier) LabelValues(ctx context.Context, name string, hints
198206
lvs, err = q.distributor.LabelValuesForLabelName(ctx, model.Time(q.mint), model.Time(q.maxt), model.LabelName(name), hints, partialDataEnabled, matchers...)
199207
}
200208

209+
if partialdata.IsPartialDataError(err) {
210+
warnings := annotations.Annotations(nil)
211+
return lvs, warnings.Add(err), nil
212+
}
213+
201214
return lvs, nil, err
202215
}
203216

@@ -222,6 +235,11 @@ func (q *distributorQuerier) LabelNames(ctx context.Context, hints *storage.Labe
222235
ln, err = q.distributor.LabelNames(ctx, model.Time(q.mint), model.Time(q.maxt), hints, partialDataEnabled, matchers...)
223236
}
224237

238+
if partialdata.IsPartialDataError(err) {
239+
warnings := annotations.Annotations(nil)
240+
return ln, warnings.Add(err), nil
241+
}
242+
225243
return ln, nil, err
226244
}
227245

@@ -241,7 +259,7 @@ func (q *distributorQuerier) labelNamesWithMatchers(ctx context.Context, hints *
241259
ms, err = q.distributor.MetricsForLabelMatchers(ctx, model.Time(q.mint), model.Time(q.maxt), labelHintsToSelectHints(hints), partialDataEnabled, matchers...)
242260
}
243261

244-
if err != nil {
262+
if err != nil && !partialdata.IsPartialDataError(err) {
245263
return nil, nil, err
246264
}
247265
namesMap := make(map[string]struct{})
@@ -258,6 +276,11 @@ func (q *distributorQuerier) labelNamesWithMatchers(ctx context.Context, hints *
258276
}
259277
sort.Strings(names)
260278

279+
if partialdata.IsPartialDataError(err) {
280+
warnings := annotations.Annotations(nil)
281+
return names, warnings.Add(err), nil
282+
}
283+
261284
return names, nil, nil
262285
}
263286

‎pkg/querier/distributor_queryable_test.go

+50-38
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,13 @@ func TestIngesterStreaming(t *testing.T) {
171171
},
172172
},
173173
}
174-
d.On("QueryStream", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(queryResponse, nil)
175-
176-
ctx := user.InjectOrgID(context.Background(), "0")
174+
var partialDataErr error
177175
if partialDataEnabled {
178-
d = &MockDistributor{}
179-
d.On("QueryStream", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(queryResponse, partialdata.Error{})
176+
partialDataErr = partialdata.Error{}
180177
}
178+
d.On("QueryStream", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(queryResponse, partialDataErr)
179+
180+
ctx := user.InjectOrgID(context.Background(), "0")
181181

182182
queryable := newDistributorQueryable(d, true, true, batch.NewChunkMergeIterator, 0, func(string) bool {
183183
return partialDataEnabled
@@ -204,7 +204,7 @@ func TestIngesterStreaming(t *testing.T) {
204204
require.NoError(t, seriesSet.Err())
205205

206206
if partialDataEnabled {
207-
require.NotEmpty(t, seriesSet.Warnings())
207+
require.Contains(t, seriesSet.Warnings(), partialdata.ErrorMsg)
208208
}
209209
}
210210
}
@@ -218,40 +218,52 @@ func TestDistributorQuerier_LabelNames(t *testing.T) {
218218

219219
for _, labelNamesWithMatchers := range []bool{false, true} {
220220
for _, streamingEnabled := range []bool{false, true} {
221-
streamingEnabled := streamingEnabled
222-
labelNamesWithMatchers := labelNamesWithMatchers
223-
t.Run("with matchers", func(t *testing.T) {
224-
t.Parallel()
225-
226-
metrics := []model.Metric{
227-
{"foo": "bar"},
228-
{"job": "baz"},
229-
{"job": "baz", "foo": "boom"},
230-
}
231-
d := &MockDistributor{}
232-
233-
if labelNamesWithMatchers {
234-
d.On("LabelNames", mock.Anything, model.Time(mint), model.Time(maxt), mock.Anything, someMatchers).
235-
Return(labelNames, nil)
236-
d.On("LabelNamesStream", mock.Anything, model.Time(mint), model.Time(maxt), mock.Anything, someMatchers).
237-
Return(labelNames, nil)
238-
} else {
239-
d.On("MetricsForLabelMatchers", mock.Anything, model.Time(mint), model.Time(maxt), mock.Anything, someMatchers).
240-
Return(metrics, nil)
241-
d.On("MetricsForLabelMatchersStream", mock.Anything, model.Time(mint), model.Time(maxt), mock.Anything, someMatchers).
242-
Return(metrics, nil)
243-
}
221+
for _, partialDataEnabled := range []bool{false, true} {
222+
streamingEnabled := streamingEnabled
223+
labelNamesWithMatchers := labelNamesWithMatchers
224+
t.Run("with matchers", func(t *testing.T) {
225+
t.Parallel()
226+
227+
metrics := []model.Metric{
228+
{"foo": "bar"},
229+
{"job": "baz"},
230+
{"job": "baz", "foo": "boom"},
231+
}
232+
d := &MockDistributor{}
244233

245-
queryable := newDistributorQueryable(d, streamingEnabled, labelNamesWithMatchers, nil, 0, nil)
246-
querier, err := queryable.Querier(mint, maxt)
247-
require.NoError(t, err)
234+
var partialDataErr error
235+
if partialDataEnabled {
236+
partialDataErr = partialdata.Error{}
237+
}
238+
if labelNamesWithMatchers {
239+
d.On("LabelNames", mock.Anything, model.Time(mint), model.Time(maxt), mock.Anything, someMatchers).
240+
Return(labelNames, partialDataErr)
241+
d.On("LabelNamesStream", mock.Anything, model.Time(mint), model.Time(maxt), mock.Anything, someMatchers).
242+
Return(labelNames, partialDataErr)
243+
} else {
244+
d.On("MetricsForLabelMatchers", mock.Anything, model.Time(mint), model.Time(maxt), mock.Anything, someMatchers).
245+
Return(metrics, partialDataErr)
246+
d.On("MetricsForLabelMatchersStream", mock.Anything, model.Time(mint), model.Time(maxt), mock.Anything, someMatchers).
247+
Return(metrics, partialDataErr)
248+
}
248249

249-
ctx := context.Background()
250-
names, warnings, err := querier.LabelNames(ctx, nil, someMatchers...)
251-
require.NoError(t, err)
252-
assert.Empty(t, warnings)
253-
assert.Equal(t, labelNames, names)
254-
})
250+
queryable := newDistributorQueryable(d, streamingEnabled, labelNamesWithMatchers, nil, 0, func(string) bool {
251+
return partialDataEnabled
252+
})
253+
querier, err := queryable.Querier(mint, maxt)
254+
require.NoError(t, err)
255+
256+
ctx := context.Background()
257+
names, warnings, err := querier.LabelNames(ctx, nil, someMatchers...)
258+
require.NoError(t, err)
259+
if partialDataEnabled {
260+
assert.Contains(t, warnings, partialdata.ErrorMsg)
261+
} else {
262+
assert.Empty(t, warnings)
263+
}
264+
assert.Equal(t, labelNames, names)
265+
})
266+
}
255267
}
256268
}
257269
}

‎pkg/util/validation/limits_test.go

+9-5
Original file line numberDiff line numberDiff line change
@@ -638,17 +638,18 @@ tenant2:
638638
require.Equal(t, 5, ov.MaxDownloadedBytesPerRequest("tenant3"))
639639
}
640640

641-
func TestQueryPartialDataOverridesPerTenant(t *testing.T) {
641+
func TestPartialDataOverridesPerTenant(t *testing.T) {
642642
SetDefaultLimitsForYAMLUnmarshalling(Limits{})
643643

644644
baseYAML := `
645-
query_partial_data: false`
645+
query_partial_data: false
646+
rules_partial_data: false`
646647
overridesYAML := `
647648
tenant1:
648649
query_partial_data: true
649650
tenant2:
650-
query_partial_data: false
651-
`
651+
query_partial_data: true
652+
rules_partial_data: true`
652653

653654
l := Limits{}
654655
err := yaml.UnmarshalStrict([]byte(baseYAML), &l)
@@ -664,8 +665,11 @@ tenant2:
664665
require.NoError(t, err)
665666

666667
require.True(t, ov.QueryPartialData("tenant1"))
667-
require.False(t, ov.QueryPartialData("tenant2"))
668+
require.False(t, ov.RulesPartialData("tenant1"))
669+
require.True(t, ov.QueryPartialData("tenant2"))
670+
require.True(t, ov.RulesPartialData("tenant2"))
668671
require.False(t, ov.QueryPartialData("tenant3"))
672+
require.False(t, ov.RulesPartialData("tenant3"))
669673
}
670674

671675
func TestHasQueryAttributeRegexChanged(t *testing.T) {

0 commit comments

Comments
 (0)