filter out label with empty value in distributor (#7069)

* filter out label with empty value in distributor

Signed-off-by: yeya24 <[email protected]>

* update changelog

Signed-off-by: yeya24 <[email protected]>

---------

Signed-off-by: yeya24 <[email protected]>

Author: yeya24

CHANGELOG.md

@@ -84,6 +84,7 @@
 * [ENHANCEMENT] Add new metric `cortex_discarded_series` and `cortex_discarded_series_per_labelset` to track number of series that have a discarded sample. #6995
 * [ENHANCEMENT] Ingester: Add `cortex_ingester_tsdb_head_stale_series` metric to keep track of number of stale series on head. #7071
 * [ENHANCEMENT] Expose more Go runtime metrics. #7070
+* [ENHANCEMENT] Distributor: Filter out label with empty value. #7069
 * [BUGFIX] Ingester: Avoid error or early throttling when READONLY ingesters are present in the ring #6517
 * [BUGFIX] Ingester: Fix labelset data race condition. #6573
 * [BUGFIX] Compactor: Cleaner should not put deletion marker for blocks with no-compact marker. #6576

pkg/distributor/distributor.go

@@ -600,6 +600,18 @@ func removeLabel(labelName string, labels *[]cortexpb.LabelAdapter) {
 	}
 }
 
+// Remove all labels with empty values from a slice of LabelPairs.
+func removeEmptyLabels(labels *[]cortexpb.LabelAdapter) {
+	i := 0
+	for j := 0; j < len(*labels); j++ {
+		if (*labels)[j].Value != "" {
+			(*labels)[i] = (*labels)[j]
+			i++
+		}
+	}
+	*labels = (*labels)[:i]
+}
+
 // Returns a boolean that indicates whether or not we want to remove the replica label going forward,
 // and an error that indicates whether we want to accept samples based on the cluster/replica found in ts.
 // nil for the error means accept the sample.
@@ -1080,6 +1092,9 @@ func (d *Distributor) prepareSeriesKeys(ctx context.Context, req *cortexpb.Write
 			removeLabel(labelName, &ts.Labels)
 		}
 
+		// Make sure no label with empty value is sent to the Ingester.
+		removeEmptyLabels(&ts.Labels)
+
 		if len(ts.Labels) == 0 {
 			d.validateMetrics.DiscardedSamples.WithLabelValues(
 				validation.DroppedByUserConfigurationOverride,

pkg/distributor/distributor_test.go

@@ -4192,6 +4192,53 @@ func TestDistributor_Push_Relabel(t *testing.T) {
 	}
 }
 
+func TestRemoveEmptyLabels(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		input    []cortexpb.LabelAdapter
+		expected []cortexpb.LabelAdapter
+	}{
+		{
+			name:     "no empty labels",
+			input:    []cortexpb.LabelAdapter{{Name: "job", Value: "test"}, {Name: "instance", Value: "localhost"}},
+			expected: []cortexpb.LabelAdapter{{Name: "job", Value: "test"}, {Name: "instance", Value: "localhost"}},
+		},
+		{
+			name:     "empty label at beginning",
+			input:    []cortexpb.LabelAdapter{{Name: "empty", Value: ""}, {Name: "job", Value: "test"}},
+			expected: []cortexpb.LabelAdapter{{Name: "job", Value: "test"}},
+		},
+		{
+			name:     "empty label in middle",
+			input:    []cortexpb.LabelAdapter{{Name: "job", Value: "test"}, {Name: "empty", Value: ""}, {Name: "instance", Value: "localhost"}},
+			expected: []cortexpb.LabelAdapter{{Name: "job", Value: "test"}, {Name: "instance", Value: "localhost"}},
+		},
+		{
+			name:     "empty label at end",
+			input:    []cortexpb.LabelAdapter{{Name: "job", Value: "test"}, {Name: "empty", Value: ""}},
+			expected: []cortexpb.LabelAdapter{{Name: "job", Value: "test"}},
+		},
+		{
+			name:     "multiple empty labels - removes all empty ones",
+			input:    []cortexpb.LabelAdapter{{Name: "empty1", Value: ""}, {Name: "job", Value: "test"}, {Name: "empty2", Value: ""}},
+			expected: []cortexpb.LabelAdapter{{Name: "job", Value: "test"}},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Make a copy of the input to avoid modifying the original
+			input := make([]cortexpb.LabelAdapter, len(tt.input))
+			copy(input, tt.input)
+
+			removeEmptyLabels(&input)
+			assert.Equal(t, tt.expected, input)
+		})
+	}
+}
+
 func TestDistributor_Push_EmptyLabel(t *testing.T) {
 	t.Parallel()
 	ctx := user.InjectOrgID(context.Background(), "pushEmptyLabel")
@@ -4255,6 +4302,10 @@ func TestDistributor_Push_EmptyLabel(t *testing.T) {
 				timeseries := ingesters[i].series()
 				if len(timeseries) > 0 {
 					ingesterWithSeries++
+					// Assert on the expected series
+					for _, v := range timeseries {
+						assert.Equal(t, tc.expectedSeries, cortexpb.FromLabelAdaptersToLabels(v.Labels))
+					}
 				}
 			}
 			assert.Equal(t, 1, ingesterWithSeries)