fix: Remote-write-reciever returns 4xx when request contains a time series with duplicate labels. (#14716)

Signed-off-by: Devin Trejo <dtrejo@palantir.com>

storage/remote/write_handler.go

@@ -236,11 +236,16 @@ func (h *writeHandler) write(ctx context.Context, req *prompb.WriteRequest) (err
 	b := labels.NewScratchBuilder(0)
 	for _, ts := range req.Timeseries {
 		ls := ts.ToLabels(&b, nil)
+
+		// TODO(bwplotka): Even as per 1.0 spec, this should be a 400 error, while other samples are
+		// potentially written. Perhaps unify with fixed writeV2 implementation a bit.
 		if !ls.Has(labels.MetricName) || !ls.IsValid() {
 			level.Warn(h.logger).Log("msg", "Invalid metric names or labels", "got", ls.String())
 			samplesWithInvalidLabels++
-			// TODO(bwplotka): Even as per 1.0 spec, this should be a 400 error, while other samples are
-			// potentially written. Perhaps unify with fixed writeV2 implementation a bit.
+			continue
+		} else if duplicateLabel, hasDuplicate := ls.HasDuplicateLabelNames(); hasDuplicate {
+			level.Warn(h.logger).Log("msg", "Invalid labels for series.", "labels", ls.String(), "duplicated_label", duplicateLabel)
+			samplesWithInvalidLabels++
 			continue
 		}
 
@@ -379,6 +384,10 @@ func (h *writeHandler) appendV2(app storage.Appender, req *writev2.Request, rs *
 			badRequestErrs = append(badRequestErrs, fmt.Errorf("invalid metric name or labels, got %v", ls.String()))
 			samplesWithInvalidLabels += len(ts.Samples) + len(ts.Histograms)
 			continue
+		} else if duplicateLabel, hasDuplicate := ls.HasDuplicateLabelNames(); hasDuplicate {
+			badRequestErrs = append(badRequestErrs, fmt.Errorf("invalid labels for series, labels %v, duplicated label %s", ls.String(), duplicateLabel))
+			samplesWithInvalidLabels += len(ts.Samples) + len(ts.Histograms)
+			continue
 		}
 
 		allSamplesSoFar := rs.AllSamples()

storage/remote/write_handler_test.go

@@ -338,6 +338,15 @@ func TestRemoteWriteHandler_V2Message(t *testing.T) {
 			expectedCode:     http.StatusBadRequest,
 			expectedRespBody: "invalid metric name or labels, got {__name__=\"\"}\n",
 		},
+		{
+			desc: "Partial write; first series with duplicate labels",
+			input: append(
+				// Series with __name__="test_metric1",test_metric1="test_metric1",test_metric1="test_metric1" labels.
+				[]writev2.TimeSeries{{LabelsRefs: []uint32{1, 2, 2, 2, 2, 2}, Samples: []writev2.Sample{{Value: 1, Timestamp: 1}}}},
+				writeV2RequestFixture.Timeseries...),
+			expectedCode:     http.StatusBadRequest,
+			expectedRespBody: "invalid labels for series, labels {__name__=\"test_metric1\", test_metric1=\"test_metric1\", test_metric1=\"test_metric1\"}, duplicated label test_metric1\n",
+		},
 		{
 			desc: "Partial write; first series with one OOO sample",
 			input: func() []writev2.TimeSeries {
@@ -836,6 +845,13 @@ func (m *mockAppendable) Append(_ storage.SeriesRef, l labels.Labels, t int64, v
 		return 0, storage.ErrDuplicateSampleForTimestamp
 	}
 
+	if l.IsEmpty() {
+		return 0, tsdb.ErrInvalidSample
+	}
+	if _, hasDuplicates := l.HasDuplicateLabelNames(); hasDuplicates {
+		return 0, tsdb.ErrInvalidSample
+	}
+
 	m.latestSample[l.Hash()] = t
 	m.samples = append(m.samples, mockSample{l, t, v})
 	return 0, nil
@@ -887,6 +903,13 @@ func (m *mockAppendable) AppendHistogram(_ storage.SeriesRef, l labels.Labels, t
 		return 0, storage.ErrDuplicateSampleForTimestamp
 	}
 
+	if l.IsEmpty() {
+		return 0, tsdb.ErrInvalidSample
+	}
+	if _, hasDuplicates := l.HasDuplicateLabelNames(); hasDuplicates {
+		return 0, tsdb.ErrInvalidSample
+	}
+
 	m.latestHistogram[l.Hash()] = t
 	m.histograms = append(m.histograms, mockHistogram{l, t, h, fh})
 	return 0, nil