Skip to content

Commit 65f4f13

Browse files
justinjung04justinjung04
committed
Lint + fix tests
1 parent c167f4a commit 65f4f13

File tree

7 files changed

+21
-61
lines changed

7 files changed

+21
-61
lines changed

‎pkg/distributor/distributor.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -1228,8 +1228,8 @@ func (d *Distributor) LabelValuesForLabelNameCommon(ctx context.Context, from, t
12281228
}
12291229

12301230
// LabelValuesForLabelName returns all the label values that are associated with a given label name.
1231-
func (d *Distributor) LabelValuesForLabelName(ctx context.Context, from, to model.Time, label model.LabelName, hint *storage.LabelHints, partialDataEnabled bool, matchers ...*labels.Matcher) ([]string, error) {
1232-
return d.LabelValuesForLabelNameCommon(ctx, from, to, label, hint, func(ctx context.Context, rs ring.ReplicationSet, req *ingester_client.LabelValuesRequest) ([]interface{}, error) {
1231+
func (d *Distributor) LabelValuesForLabelName(ctx context.Context, from, to model.Time, labelName model.LabelName, hint *storage.LabelHints, partialDataEnabled bool, matchers ...*labels.Matcher) ([]string, error) {
1232+
return d.LabelValuesForLabelNameCommon(ctx, from, to, labelName, hint, func(ctx context.Context, rs ring.ReplicationSet, req *ingester_client.LabelValuesRequest) ([]interface{}, error) {
12331233
return d.ForReplicationSet(ctx, rs, d.cfg.ZoneResultsQuorumMetadata, partialDataEnabled, func(ctx context.Context, client ingester_client.IngesterClient) (interface{}, error) {
12341234
resp, err := client.LabelValues(ctx, req)
12351235
if err != nil {
@@ -1241,8 +1241,8 @@ func (d *Distributor) LabelValuesForLabelName(ctx context.Context, from, to mode
12411241
}
12421242

12431243
// LabelValuesForLabelNameStream returns all the label values that are associated with a given label name.
1244-
func (d *Distributor) LabelValuesForLabelNameStream(ctx context.Context, from, to model.Time, label model.LabelName, hint *storage.LabelHints, partialDataEnabled bool, matchers ...*labels.Matcher) ([]string, error) {
1245-
return d.LabelValuesForLabelNameCommon(ctx, from, to, label, hint, func(ctx context.Context, rs ring.ReplicationSet, req *ingester_client.LabelValuesRequest) ([]interface{}, error) {
1244+
func (d *Distributor) LabelValuesForLabelNameStream(ctx context.Context, from, to model.Time, labelName model.LabelName, hint *storage.LabelHints, partialDataEnabled bool, matchers ...*labels.Matcher) ([]string, error) {
1245+
return d.LabelValuesForLabelNameCommon(ctx, from, to, labelName, hint, func(ctx context.Context, rs ring.ReplicationSet, req *ingester_client.LabelValuesRequest) ([]interface{}, error) {
12461246
return d.ForReplicationSet(ctx, rs, d.cfg.ZoneResultsQuorumMetadata, partialDataEnabled, func(ctx context.Context, client ingester_client.IngesterClient) (interface{}, error) {
12471247
stream, err := client.LabelValuesStream(ctx, req)
12481248
if err != nil {

‎pkg/distributor/query.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ func (d *Distributor) queryIngesterStream(ctx context.Context, replicationSet ri
331331

332332
if partialdata.IsPartialDataError(err) {
333333
level.Info(d.log).Log("msg", "returning partial data")
334-
return resp, partialdata.Error{}
334+
return resp, err
335335
}
336336

337337
return resp, nil

‎pkg/querier/distributor_queryable.go

+5-9
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func (q *distributorQuerier) LabelValues(ctx context.Context, name string, hints
204204

205205
func (q *distributorQuerier) LabelNames(ctx context.Context, hints *storage.LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) {
206206
if len(matchers) > 0 && !q.labelNamesMatchers {
207-
return q.labelNamesWithMatchers(ctx, hints, matchers...)
207+
return q.labelNamesWithMatchers(ctx, hints, q.isPartialDataEnabled(ctx), matchers...)
208208
}
209209

210210
log, ctx := spanlogger.New(ctx, "distributorQuerier.LabelNames")
@@ -227,7 +227,7 @@ func (q *distributorQuerier) LabelNames(ctx context.Context, hints *storage.Labe
227227
}
228228

229229
// labelNamesWithMatchers performs the LabelNames call by calling ingester's MetricsForLabelMatchers method
230-
func (q *distributorQuerier) labelNamesWithMatchers(ctx context.Context, hints *storage.LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) {
230+
func (q *distributorQuerier) labelNamesWithMatchers(ctx context.Context, hints *storage.LabelHints, partialDataEnabled bool, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) {
231231
log, ctx := spanlogger.New(ctx, "distributorQuerier.labelNamesWithMatchers")
232232
defer log.Span.Finish()
233233

@@ -237,9 +237,9 @@ func (q *distributorQuerier) labelNamesWithMatchers(ctx context.Context, hints *
237237
)
238238

239239
if q.streamingMetadata {
240-
ms, err = q.distributor.MetricsForLabelMatchersStream(ctx, model.Time(q.mint), model.Time(q.maxt), labelHintsToSelectHints(hints), false, matchers...)
240+
ms, err = q.distributor.MetricsForLabelMatchersStream(ctx, model.Time(q.mint), model.Time(q.maxt), labelHintsToSelectHints(hints), partialDataEnabled, matchers...)
241241
} else {
242-
ms, err = q.distributor.MetricsForLabelMatchers(ctx, model.Time(q.mint), model.Time(q.maxt), labelHintsToSelectHints(hints), false, matchers...)
242+
ms, err = q.distributor.MetricsForLabelMatchers(ctx, model.Time(q.mint), model.Time(q.maxt), labelHintsToSelectHints(hints), partialDataEnabled, matchers...)
243243
}
244244

245245
if err != nil {
@@ -267,16 +267,12 @@ func (q *distributorQuerier) Close() error {
267267
}
268268

269269
func (q *distributorQuerier) isPartialDataEnabled(ctx context.Context) bool {
270-
if q.limits != nil {
271-
return false
272-
}
273-
274270
userID, err := tenant.TenantID(ctx)
275271
if err != nil {
276272
return false
277273
}
278274

279-
return q.limits.QueryPartialData(userID)
275+
return q.limits != nil && q.limits.QueryPartialData(userID)
280276
}
281277

282278
type distributorExemplarQueryable struct {

‎pkg/querier/distributor_queryable_test.go

+8-13
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func TestDistributorQuerier_SelectShouldHonorQueryIngestersWithin(t *testing.T)
9090
distributor.On("MetricsForLabelMatchersStream", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]model.Metric{}, nil)
9191

9292
ctx := user.InjectOrgID(context.Background(), "test")
93-
queryable := newDistributorQueryable(distributor, streamingMetadataEnabled, true, nil, testData.queryIngestersWithin, queryPartialDataDisabledFn)
93+
queryable := newDistributorQueryable(distributor, streamingMetadataEnabled, true, nil, testData.queryIngestersWithin, nil)
9494
querier, err := queryable.Querier(testData.queryMinT, testData.queryMaxT)
9595
require.NoError(t, err)
9696

@@ -129,7 +129,7 @@ func TestDistributorQueryableFilter(t *testing.T) {
129129
t.Parallel()
130130

131131
d := &MockDistributor{}
132-
dq := newDistributorQueryable(d, false, true, nil, 1*time.Hour, queryPartialDataDisabledFn)
132+
dq := newDistributorQueryable(d, false, true, nil, 1*time.Hour, nil)
133133

134134
now := time.Now()
135135

@@ -174,13 +174,16 @@ func TestIngesterStreaming(t *testing.T) {
174174
d.On("QueryStream", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(queryResponse, nil)
175175

176176
ctx := user.InjectOrgID(context.Background(), "0")
177-
partialDataFn := queryPartialDataDisabledFn
178177
if partialDataEnabled {
179-
partialDataFn = queryPartialDataEnabledFn
180178
d = &MockDistributor{}
181179
d.On("QueryStream", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(queryResponse, partialdata.Error{})
182180
}
183-
queryable := newDistributorQueryable(d, true, true, batch.NewChunkMergeIterator, 0, partialDataFn)
181+
182+
limits := validation.Limits{QueryPartialData: partialDataEnabled}
183+
overrides, err := validation.NewOverrides(limits, nil)
184+
require.NoError(t, err)
185+
186+
queryable := newDistributorQueryable(d, true, true, batch.NewChunkMergeIterator, 0, overrides)
184187
querier, err := queryable.Querier(mint, maxt)
185188
require.NoError(t, err)
186189

@@ -254,11 +257,3 @@ func TestDistributorQuerier_LabelNames(t *testing.T) {
254257
}
255258
}
256259
}
257-
258-
func queryPartialDataEnabledFn(string) bool {
259-
return true
260-
}
261-
262-
func queryPartialDataDisabledFn(string) bool {
263-
return false
264-
}

‎pkg/querier/partialdata/partia_data.go

-4
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,6 @@ func (e Error) Error() string {
1212
return ErrorMsg
1313
}
1414

15-
func ReturnPartialData(err error, isEnabled bool) bool {
16-
return isEnabled && errors.As(err, &Error{})
17-
}
18-
1915
func IsPartialDataError(err error) bool {
2016
return errors.As(err, &Error{})
2117
}

‎pkg/querier/partialdata/partial_data_test.go

+2-4
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ import (
88
)
99

1010
func TestPartialData_ReturnPartialData(t *testing.T) {
11-
assert.False(t, ReturnPartialData(fmt.Errorf(""), false))
12-
assert.False(t, ReturnPartialData(fmt.Errorf(""), true))
13-
assert.False(t, ReturnPartialData(Error{}, false))
14-
assert.True(t, ReturnPartialData(Error{}, true))
11+
assert.False(t, IsPartialDataError(fmt.Errorf("")))
12+
assert.True(t, IsPartialDataError(Error{}))
1513
}

‎pkg/querier/querier_test.go

+1-26
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ func TestLimits(t *testing.T) {
444444
response: &streamResponse,
445445
}
446446

447-
distributorQueryableStreaming := newDistributorQueryable(distributor, cfg.IngesterMetadataStreaming, cfg.IngesterLabelNamesWithMatchers, batch.NewChunkMergeIterator, cfg.QueryIngestersWithin, queryPartialDataDisabledFn)
447+
distributorQueryableStreaming := newDistributorQueryable(distributor, cfg.IngesterMetadataStreaming, cfg.IngesterLabelNamesWithMatchers, batch.NewChunkMergeIterator, cfg.QueryIngestersWithin, nil)
448448

449449
tCases := []struct {
450450
name string
@@ -1702,28 +1702,3 @@ func (m *mockQueryableWithFilter) UseQueryable(_ time.Time, _, _ int64) bool {
17021702
m.useQueryableCalled = true
17031703
return true
17041704
}
1705-
1706-
type mockChunkStore struct {
1707-
chunks []chunk.Chunk
1708-
}
1709-
1710-
func (m mockChunkStore) Get() ([]chunk.Chunk, error) {
1711-
return m.chunks, nil
1712-
}
1713-
1714-
func makeMockChunkStore(t require.TestingT, numChunks int, enc promchunk.Encoding) (mockChunkStore, model.Time) {
1715-
chks, from := makeMockChunks(t, numChunks, enc, 0)
1716-
return mockChunkStore{chks}, from
1717-
}
1718-
1719-
func makeMockChunks(t require.TestingT, numChunks int, enc promchunk.Encoding, from model.Time, additionalLabels ...labels.Label) ([]chunk.Chunk, model.Time) {
1720-
var (
1721-
chunks = make([]chunk.Chunk, 0, numChunks)
1722-
)
1723-
for i := 0; i < numChunks; i++ {
1724-
c := util.GenerateChunk(t, sampleRate, from, int(samplesPerChunk), enc, additionalLabels...)
1725-
chunks = append(chunks, c)
1726-
from = from.Add(chunkOffset)
1727-
}
1728-
return chunks, from
1729-
}

0 commit comments

Comments
 (0)