perf: Improve Detected labels API

pkg/ingester/ingester.go

@@ -1397,7 +1397,7 @@ func (i *Ingester) GetDetectedLabels(ctx context.Context, req *logproto.Detected
 		}
 	}
 
-	labelMap, err := instance.LabelsWithValues(ctx, *req.Start, matchers...)
+	labelMap, err := instance.LabelsWithValues(ctx, req.Start, matchers...)
 
 	if err != nil {
 		return nil, err

pkg/ingester/ingester_test.go

@@ -833,8 +833,8 @@ func TestIngester_GetDetectedLabels(t *testing.T) {
 	require.NoError(t, err)
 
 	res, err := i.GetDetectedLabels(ctx, &logproto.DetectedLabelsRequest{
-		Start: &[]time.Time{time.Now().Add(11 * time.Nanosecond)}[0],
-		End:   nil,
+		Start: []time.Time{time.Now().Add(11 * time.Nanosecond)}[0],
+		End:   []time.Time{time.Now().Add(11 * time.Nanosecond)}[0],
 		Query: "",
 	})
 
@@ -893,8 +893,8 @@ func TestIngester_GetDetectedLabelsWithQuery(t *testing.T) {
 	require.NoError(t, err)
 
 	res, err := i.GetDetectedLabels(ctx, &logproto.DetectedLabelsRequest{
-		Start: &[]time.Time{time.Now().Add(11 * time.Nanosecond)}[0],
-		End:   nil,
+		Start: []time.Time{time.Now().Add(11 * time.Nanosecond)}[0],
+		End:   []time.Time{time.Now().Add(11 * time.Nanosecond)}[0],
 		Query: `{foo="bar"}`,
 	})
 

pkg/loghttp/labels.go

@@ -1,6 +1,7 @@
 package loghttp
 
 import (
+	"errors"
 	"net/http"
 	"sort"
 	"strconv"
@@ -88,14 +89,20 @@ func ParseLabelQuery(r *http.Request) (*logproto.LabelRequest, error) {
 }
 
 func ParseDetectedLabelsQuery(r *http.Request) (*logproto.DetectedLabelsRequest, error) {
+	var err error
+
 	start, end, err := bounds(r)
 	if err != nil {
 		return nil, err
 	}
 
+	if end.Before(start) {
+		return nil, errors.New("end timestamp must not be before or equal to start time")
+	}
+
 	return &logproto.DetectedLabelsRequest{
-		Start: &start,
-		End:   &end,
+		Start: start,
+		End:   end,
 		Query: query(r),
 	}, nil
 }

pkg/logproto/logproto.proto

@@ -478,11 +478,11 @@ message DetectedField {
 message DetectedLabelsRequest {
   google.protobuf.Timestamp start = 1 [
     (gogoproto.stdtime) = true,
-    (gogoproto.nullable) = true
+    (gogoproto.nullable) = false
   ];
   google.protobuf.Timestamp end = 2 [
     (gogoproto.stdtime) = true,
-    (gogoproto.nullable) = true
+    (gogoproto.nullable) = false
   ];
   string query = 3;
 }
@@ -494,4 +494,5 @@ message DetectedLabelsResponse {
 message DetectedLabel {
   string label = 1;
   uint64 cardinality = 2;
+  bytes sketch = 3 [(gogoproto.jsontag) = "sketch,omitempty"];
 }

pkg/querier/querier.go

@@ -5,7 +5,6 @@ import (
 	"flag"
 	"fmt"
 	"net/http"
-	"regexp"
 	"sort"
 	"strconv"
 	"time"
@@ -14,6 +13,7 @@ import (
 	"github.com/dustin/go-humanize"
 	"github.com/go-kit/log"
 	"github.com/go-kit/log/level"
+	"github.com/google/uuid"
 	"github.com/grafana/dskit/httpgrpc"
 	"github.com/grafana/dskit/tenant"
 	"github.com/opentracing/opentracing-go"
@@ -915,7 +915,6 @@ func (q *SingleTenantQuerier) DetectedLabels(ctx context.Context, req *logproto.
 	if err != nil {
 		return nil, err
 	}
-	var detectedLabels []*logproto.DetectedLabel
 	staticLabels := map[string]struct{}{"cluster": {}, "namespace": {}, "instance": {}, "pod": {}}
 
 	// Enforce the query timeout while querying backends
@@ -924,24 +923,26 @@ func (q *SingleTenantQuerier) DetectedLabels(ctx context.Context, req *logproto.
 	defer cancel()
 	g, ctx := errgroup.WithContext(ctx)
 
-	if *req.Start, *req.End, err = validateQueryTimeRangeLimits(ctx, userID, q.limits, *req.Start, *req.End); err != nil {
+	if req.Start, req.End, err = validateQueryTimeRangeLimits(ctx, userID, q.limits, req.Start, req.End); err != nil {
 		return nil, err
 	}
-	ingesterQueryInterval, storeQueryInterval := q.buildQueryIntervals(*req.Start, *req.End)
+	ingesterQueryInterval, storeQueryInterval := q.buildQueryIntervals(req.Start, req.End)
 
+	// Fetch labels from ingesters
 	var ingesterLabels *logproto.LabelToValuesResponse
 	if !q.cfg.QueryStoreOnly && ingesterQueryInterval != nil {
 		g.Go(func() error {
 			var err error
 			splitReq := *req
-			splitReq.Start = &ingesterQueryInterval.start
-			splitReq.End = &ingesterQueryInterval.end
+			splitReq.Start = ingesterQueryInterval.start
+			splitReq.End = ingesterQueryInterval.end
 
 			ingesterLabels, err = q.ingesterQuerier.DetectedLabel(ctx, &splitReq)
 			return err
 		})
 	}
 
+	// Fetch labels from the store
 	storeLabelsMap := make(map[string][]string)
 	if !q.cfg.QueryIngesterOnly && storeQueryInterval != nil {
 		var matchers []*labels.Matcher
@@ -961,9 +962,7 @@ func (q *SingleTenantQuerier) DetectedLabels(ctx context.Context, req *logproto.
 				if err != nil {
 					return err
 				}
-				if q.isLabelRelevant(label, values, staticLabels) {
-					storeLabelsMap[label] = values
-				}
+				storeLabelsMap[label] = values
 			}
 			return err
 		})
@@ -979,40 +978,58 @@ 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))})
-		}
-	}
+	return &logproto.DetectedLabelsResponse{
+		DetectedLabels: countLabelsAndCardinality(storeLabelsMap, ingesterLabels, staticLabels),
+	}, nil
+}
+
+func countLabelsAndCardinality(storeLabelsMap map[string][]string, ingesterLabels *logproto.LabelToValuesResponse, staticLabels map[string]struct{}) []*logproto.DetectedLabel {
+	dlMap := make(map[string]*parsedFields)
 
 	if ingesterLabels != nil {
-		for label, values := range ingesterLabels.Labels {
-			if q.isLabelRelevant(label, values.Values, staticLabels) {
-				combinedValues := values.Values
-				storeValues, storeHasLabel := storeLabelsMap[label]
-				if storeHasLabel {
-					combinedValues = append(combinedValues, storeValues...)
+		for label, val := range ingesterLabels.Labels {
+			if _, isStatic := staticLabels[label]; isStatic || !containsAllIDTypes(val.Values) {
+				_, ok := dlMap[label]
+				if !ok {
+					dlMap[label] = newParsedLabels()
 				}
 
-				slices.Sort(combinedValues)
-				uniqueValues := slices.Compact(combinedValues)
-				// TODO(shantanu): There's a bug here. Unique values can go above 50. Will need a bit of refactoring
-				detectedLabels = append(detectedLabels, &logproto.DetectedLabel{Label: label, Cardinality: uint64(len(uniqueValues))})
-				delete(storeLabelsMap, label)
+				parsedFields := dlMap[label]
+				for _, v := range val.Values {
+					parsedFields.Insert(v)
+				}
 			}
 		}
 	}
 
 	for label, values := range storeLabelsMap {
-		slices.Sort(values)
-		uniqueValues := slices.Compact(values)
-		detectedLabels = append(detectedLabels, &logproto.DetectedLabel{Label: label, Cardinality: uint64(len(uniqueValues))})
+		if _, isStatic := staticLabels[label]; isStatic || !containsAllIDTypes(values) {
+			_, ok := dlMap[label]
+			if !ok {
+				dlMap[label] = newParsedLabels()
+			}
+
+			parsedFields := dlMap[label]
+			for _, v := range values {
+				parsedFields.Insert(v)
+			}
+		}
 	}
 
-	return &logproto.DetectedLabelsResponse{
-		DetectedLabels: detectedLabels,
-	}, nil
+	var detectedLabels []*logproto.DetectedLabel
+	for k, v := range dlMap {
+		sketch, err := v.sketch.MarshalBinary()
+		if err != nil {
+			// TODO: add log here
+			continue
+		}
+		detectedLabels = append(detectedLabels, &logproto.DetectedLabel{
+			Label:       k,
+			Cardinality: v.Estimate(),
+			Sketch:      sketch,
+		})
+	}
+	return detectedLabels
 }
 
 type PatterQuerier interface {
@@ -1031,28 +1048,15 @@ func (q *SingleTenantQuerier) Patterns(ctx context.Context, req *logproto.QueryP
 	return res, err
 }
 
-// isLabelRelevant returns if the label is relevant for logs app. A label is relevant if it is not of any numeric, UUID or GUID type
-// It is also not relevant to return if the values are less than 1 or beyond 50.
-func (q *SingleTenantQuerier) isLabelRelevant(label string, values []string, staticLabels map[string]struct{}) bool {
-	cardinality := len(values)
-	_, isStaticLabel := staticLabels[label]
-	if isStaticLabel || (cardinality < 2 || cardinality > 50) ||
-		containsAllIDTypes(values) {
-		return false
-	}
-
-	return true
-}
-
 // containsAllIDTypes filters out all UUID, GUID and numeric types. Returns false if even one value is not of the type
 func containsAllIDTypes(values []string) bool {
-	pattern := `^(?:(?:[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})|(?:(?:\{)?[0-9a-fA-F]{8}(?:-?[0-9a-fA-F]{4}){3}-?[0-9a-fA-F]{12}(?:\})?)|(\d+(?:\.\d+)?))$`
-
-	re := regexp.MustCompile(pattern)
-
 	for _, v := range values {
-		if !re.MatchString(v) {
-			return false
+		_, err := strconv.ParseFloat(v, 64)
+		if err != nil {
+			_, err = uuid.Parse(v)
+			if err != nil {
+				return false
+			}
 		}
 	}
 
@@ -1139,6 +1143,13 @@ func newParsedFields(parser *string) *parsedFields {
 	}
 }
 
+func newParsedLabels() *parsedFields {
+	return &parsedFields{
+		sketch:    hyperloglog.New(),
+		fieldType: logproto.DetectedFieldString,
+	}
+}
+
 func (p *parsedFields) Insert(value string) {
 	p.sketch.Insert([]byte(value))
 }