From 6f99af62227f98c7d9de8a5cf480ae792ce6220a Mon Sep 17 00:00:00 2001 From: Shantanu Alshi Date: Tue, 27 Aug 2024 17:27:58 +0530 Subject: [PATCH] fix(detected labels): response when store label values are empty (#13970) --- pkg/querier/querier.go | 4 ++-- pkg/querier/querier_mock_test.go | 3 +++ pkg/querier/querier_test.go | 34 ++++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/pkg/querier/querier.go b/pkg/querier/querier.go index ac83218f9331..b320e5c5fd6a 100644 --- a/pkg/querier/querier.go +++ b/pkg/querier/querier.go @@ -995,7 +995,7 @@ func countLabelsAndCardinality(storeLabelsMap map[string][]string, ingesterLabel if ingesterLabels != nil { for label, val := range ingesterLabels.Labels { - if _, isStatic := staticLabels[label]; isStatic || !containsAllIDTypes(val.Values) { + if _, isStatic := staticLabels[label]; (isStatic && val.Values != nil) || !containsAllIDTypes(val.Values) { _, ok := dlMap[label] if !ok { dlMap[label] = newParsedLabels() @@ -1010,7 +1010,7 @@ func countLabelsAndCardinality(storeLabelsMap map[string][]string, ingesterLabel } for label, values := range storeLabelsMap { - if _, isStatic := staticLabels[label]; isStatic || !containsAllIDTypes(values) { + if _, isStatic := staticLabels[label]; (isStatic && values != nil) || !containsAllIDTypes(values) { _, ok := dlMap[label] if !ok { dlMap[label] = newParsedLabels() diff --git a/pkg/querier/querier_mock_test.go b/pkg/querier/querier_mock_test.go index e8a3b40ebd61..4ddbab7ed2e5 100644 --- a/pkg/querier/querier_mock_test.go +++ b/pkg/querier/querier_mock_test.go @@ -350,6 +350,9 @@ func (s *storeMock) PutOne(_ context.Context, _, _ model.Time, _ chunk.Chunk) er func (s *storeMock) LabelValuesForMetricName(ctx context.Context, userID string, from, through model.Time, metricName string, labelName string, _ ...*labels.Matcher) ([]string, error) { args := s.Called(ctx, userID, from, through, metricName, labelName) + if args.Get(0) == nil { + return nil, args.Error(1) + } return args.Get(0).([]string), args.Error(1) } diff --git a/pkg/querier/querier_test.go b/pkg/querier/querier_test.go index 9e98a780561f..7336c3b11bfa 100644 --- a/pkg/querier/querier_test.go +++ b/pkg/querier/querier_test.go @@ -1518,6 +1518,40 @@ func TestQuerier_DetectedLabels(t *testing.T) { } }) + t.Run("label is not present when the values are nil", func(t *testing.T) { + ingesterClient := newQuerierClientMock() + storeClient := newStoreMock() + + ingesterClient.On("GetDetectedLabels", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(&logproto.LabelToValuesResponse{}, nil) + storeClient.On("LabelNamesForMetricName", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return([]string{"storeLabel1", "pod"}, nil). + On("LabelValuesForMetricName", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, "storeLabel1", mock.Anything). + Return([]string{"val1", "val2"}, nil). + On("LabelValuesForMetricName", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, "pod", mock.Anything). + Return(nil, nil) + + querier, err := newQuerier( + conf, + mockIngesterClientConfig(), + newIngesterClientMockFactory(ingesterClient), + mockReadRingWithOneActiveIngester(), + &mockDeleteGettter{}, + storeClient, limits) + require.NoError(t, err) + + resp, err := querier.DetectedLabels(ctx, &request) + require.NoError(t, err) + + detectedLabels := resp.DetectedLabels + assert.Len(t, detectedLabels, 1) + expectedCardinality := map[string]uint64{"storeLabel1": 2} + for _, d := range detectedLabels { + card := expectedCardinality[d.Label] + assert.Equal(t, d.Cardinality, card, "Expected cardinality mismatch for: ", d.Label) + } + }) + t.Run("returns a response when store data is empty", func(t *testing.T) { ingesterResponse := logproto.LabelToValuesResponse{Labels: map[string]*logproto.UniqueLabelValues{ "cluster": {Values: []string{"ingester"}},