Skip to content

Commit

Permalink
fix(detected labels): response when store label values are empty (#13970
Browse files Browse the repository at this point in the history
)
  • Loading branch information
shantanualsi authored Aug 27, 2024
1 parent b5ac6a0 commit 6f99af6
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 2 deletions.
4 changes: 2 additions & 2 deletions pkg/querier/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down
3 changes: 3 additions & 0 deletions pkg/querier/querier_mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
34 changes: 34 additions & 0 deletions pkg/querier/querier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}},
Expand Down

0 comments on commit 6f99af6

Please sign in to comment.