Skip to content

Commit

Permalink
fix: panics when ingester response is nil (#12946)
Browse files Browse the repository at this point in the history
  • Loading branch information
shantanualsi authored May 13, 2024
1 parent b05c4f7 commit 3cc28aa
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 6 deletions.
4 changes: 4 additions & 0 deletions pkg/querier/ingester_querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,10 @@ func (q *IngesterQuerier) DetectedLabel(ctx context.Context, req *logproto.Detec
"response", resp)
}

if thisIngester == nil {
continue
}

for label, thisIngesterValues := range thisIngester.Labels {
var combinedValues []string
allIngesterValues, isLabelPresent := labelMap[label]
Expand Down
12 changes: 6 additions & 6 deletions pkg/querier/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -985,14 +985,14 @@ func (q *SingleTenantQuerier) DetectedLabels(ctx context.Context, req *logproto.
}, nil
}

// append static labels before so they are in sorted order
for l := range staticLabels {
if values, present := ingesterLabels.Labels[l]; present {
detectedLabels = append(detectedLabels, &logproto.DetectedLabel{Label: l, Cardinality: uint64(len(values.Values))})
if ingesterLabels != nil {
// append static labels before so they are in sorted order
for l := range staticLabels {
if values, present := ingesterLabels.Labels[l]; present {
detectedLabels = append(detectedLabels, &logproto.DetectedLabel{Label: l, Cardinality: uint64(len(values.Values))})
}
}
}

if ingesterLabels != nil {
for label, values := range ingesterLabels.Labels {
if q.isLabelRelevant(label, values.Values, staticLabels) {
combinedValues := values.Values
Expand Down
27 changes: 27 additions & 0 deletions pkg/querier/querier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1714,4 +1714,31 @@ func TestQuerier_DetectedLabels(t *testing.T) {
assert.Contains(t, detectedLabels, &logproto.DetectedLabel{Label: "pod", Cardinality: 4})
assert.Contains(t, detectedLabels, &logproto.DetectedLabel{Label: "namespace", Cardinality: 60})
})

t.Run("no panics with ingester response is nil", func(t *testing.T) {
ingesterClient := newQuerierClientMock()
storeClient := newStoreMock()

ingesterClient.On("GetDetectedLabels", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(nil, nil)
storeClient.On("LabelNamesForMetricName", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return([]string{}, nil)
request := logproto.DetectedLabelsRequest{
Start: &now,
End: &now,
Query: "",
}

querier, err := newQuerier(
conf,
mockIngesterClientConfig(),
newIngesterClientMockFactory(ingesterClient),
mockReadRingWithOneActiveIngester(),
&mockDeleteGettter{},
storeClient, limits)
require.NoError(t, err)

_, err = querier.DetectedLabels(ctx, &request)
require.NoError(t, err)
})
}

0 comments on commit 3cc28aa

Please sign in to comment.