Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Bugfixes for detected labels API #12466

Merged
merged 2 commits into from
Apr 4, 2024
+22 −11
Merged

fix: Bugfixes for detected labels API #12466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
505904f
Bugfixes for detected labels API
shantanualsi Apr 4, 2024
e05077f
Merge branch 'main' into detected-labels-bugfixes
shantanualsi Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/ingester/ingester.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1389,7 +1389,7 @@ func (i *Ingester) GetDetectedLabels(ctx context.Context, req *logproto.Detected
}
var matchers []*labels.Matcher
if req.Query != "" {
matchers, err := syntax.ParseMatchers(req.Query, true)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:= again after declaring zero value above 🧠

matchers, err = syntax.ParseMatchers(req.Query, true)
if err != nil {
return nil, err
}
Expand All @@ -1403,7 +1403,7 @@ func (i *Ingester) GetDetectedLabels(ctx context.Context, req *logproto.Detected
}
result := make(map[string]*logproto.UniqueLabelValues)
for label, values := range labelMap {
uniqueValues := make([]string, len(values))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use of make introduced blank values in the slice

var uniqueValues []string
for v := range values {
uniqueValues = append(uniqueValues, v)
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/ingester/instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,8 +564,6 @@ func (i *instance) Label(ctx context.Context, req *logproto.LabelRequest, matche
}

labels := util.NewUniqueStrings(0)
// (shantanu) can create a map here to store label names::values and count the unique values
// just return a string to int map
err := i.forMatchingStreams(ctx, *req.Start, matchers, nil, func(s *stream) error {
for _, label := range s.labels {
if req.Values && label.Name == req.Name {
Expand Down
19 changes: 13 additions & 6 deletions pkg/querier/ingester_querier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,17 +370,24 @@ func (q *IngesterQuerier) DetectedLabel(ctx context.Context, req *logproto.Detec
labelMap := make(map[string][]string)
for _, resp := range ingesterResponses {
thisIngester := resp.response.(*logproto.LabelToValuesResponse)
for label, values := range thisIngester.Labels {
uniqueValues := make([]string, len(values.Values))
allValues := thisIngester.Labels[label]
uniqueValues = append(uniqueValues, allValues.Values...)

labelMap[label] = uniqueValues
for label, thisIngesterValues := range thisIngester.Labels {
var combinedValues []string
allIngesterValues, isLabelPresent := labelMap[label]
if isLabelPresent {
combinedValues = append(allIngesterValues, thisIngesterValues.Values...)
} else {
combinedValues = thisIngesterValues.Values
}
labelMap[label] = combinedValues
}
}

// Dedupe all ingester values
mergedResult := make(map[string]*logproto.UniqueLabelValues)
for label, val := range labelMap {
uniqueValues := slices.CompactFunc(val, strings.EqualFold)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compact only works for dedupe if values are sorted

slices.Sort(val)
uniqueValues := slices.Compact(val)

mergedResult[label] = &logproto.UniqueLabelValues{
Values: uniqueValues,
Expand Down
8 changes: 7 additions & 1 deletion pkg/querier/querier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ func (q *SingleTenantQuerier) DetectedLabels(ctx context.Context, req *logproto.

g, ctx := errgroup.WithContext(ctx)
ingesterQueryInterval, _ := q.buildQueryIntervals(*req.Start, *req.End)
if !q.cfg.QueryStoreOnly {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

if !q.cfg.QueryStoreOnly && ingesterQueryInterval != nil {
g.Go(func() error {
var err error
splitReq := *req
Expand All @@ -932,6 +932,12 @@ func (q *SingleTenantQuerier) DetectedLabels(ctx context.Context, req *logproto.
return nil, err
}

if ingesterLabels == nil {
return &logproto.DetectedLabelsResponse{
DetectedLabels: []*logproto.DetectedLabel{},
}, nil
}

for label, values := range ingesterLabels.Labels {
if q.isLabelRelevant(label, values) {
detectedLabels = append(detectedLabels, &logproto.DetectedLabel{Label: label, Cardinality: uint64(len(values.Values))})
Expand Down
Loading