From 8e260fef834877b2fe677146b1fcaf2dd8b01c4d Mon Sep 17 00:00:00 2001
From: Trevor Whitney <trevorjwhitney@gmail.com>
Date: Fri, 3 Jan 2025 17:17:15 -0700
Subject: [PATCH] fix: bytes unit detection in detected fields (#15525)

---
 pkg/querier/queryrange/detected_fields.go     | 41 ++++++++++++++++++-
 .../queryrange/detected_fields_test.go        | 26 +++++++++++-
 2 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/pkg/querier/queryrange/detected_fields.go b/pkg/querier/queryrange/detected_fields.go
index be42d1f0a6868..00859b5cd68ec 100644
--- a/pkg/querier/queryrange/detected_fields.go
+++ b/pkg/querier/queryrange/detected_fields.go
@@ -91,6 +91,45 @@ func NewDetectedFieldsHandler(
 		})
 }
 
+type bytesUnit []string
+
+func (b bytesUnit) Contains(s string) bool {
+	for _, u := range b {
+		if strings.HasSuffix(s, u) {
+			return true
+		}
+	}
+	return false
+}
+
+var allowedBytesUnits = bytesUnit{
+	"b",
+	"kib",
+	"kb",
+	"mib",
+	"mb",
+	"gib",
+	"gb",
+	"tib",
+	"tb",
+	"pib",
+	"pb",
+	"eib",
+	"eb",
+	"ki",
+	"k",
+	"mi",
+	"m",
+	"gi",
+	"g",
+	"ti",
+	"t",
+	"pi",
+	"p",
+	"ei",
+	"e",
+}
+
 func parseDetectedFieldValues(limit uint32, streams []push.Stream, name string) []string {
 	values := map[string]struct{}{}
 	for _, stream := range streams {
@@ -116,7 +155,7 @@ func parseDetectedFieldValues(limit uint32, streams []push.Stream, name string)
 			if vals, ok := parsedLabels[name]; ok {
 				for _, v := range vals {
 					// special case bytes values, so they can be directly inserted into a query
-					if bs, err := humanize.ParseBytes(v); err == nil {
+					if bs, err := humanize.ParseBytes(v); err == nil && allowedBytesUnits.Contains(strings.ToLower(v)) {
 						bsString := strings.Replace(humanize.Bytes(bs), " ", "", 1)
 						values[bsString] = struct{}{}
 					} else {
diff --git a/pkg/querier/queryrange/detected_fields_test.go b/pkg/querier/queryrange/detected_fields_test.go
index 22aa63e284888..0faa2adb519d9 100644
--- a/pkg/querier/queryrange/detected_fields_test.go
+++ b/pkg/querier/queryrange/detected_fields_test.go
@@ -1334,7 +1334,7 @@ func TestQuerier_DetectedFields(t *testing.T) {
 		lines := []push.Entry{
 			{
 				Timestamp:          now,
-				Line:               "ts=2024-09-05T15:36:38.757788067Z caller=metrics.go:66 tenant=2419 level=info bytes=1,024",
+				Line:               "ts=2024-09-05T15:36:38.757788067Z caller=metrics.go:66 tenant=2419 level=info bytes=1024",
 				StructuredMetadata: infoDetectdFiledMetadata,
 			},
 			{
@@ -1378,7 +1378,29 @@ func TestQuerier_DetectedFields(t *testing.T) {
 		require.Equal(t, []string{
 			"1.0GB",
 			"1.0MB",
-			"1.0kB",
+			"1024",
+		}, detectedFieldValues)
+
+		// does not affect other numeric values
+		request = DetectedFieldsRequest{
+			logproto.DetectedFieldsRequest{
+				Start:     time.Now().Add(-1 * time.Minute),
+				End:       time.Now(),
+				Query:     `{cluster="us-east-1"} | logfmt`,
+				LineLimit: 1000,
+				Limit:     3,
+				Values:    true,
+				Name:      "tenant",
+			},
+			"/loki/api/v1/detected_field/tenant/values",
+		}
+
+		detectedFieldValues = handleRequest(handler, request).Values
+		slices.Sort(detectedFieldValues)
+		require.Equal(t, []string{
+			"2419",
+			"29",
+			"2919",
 		}, detectedFieldValues)
 	})
 }