refactor out common code between write methods

Signed-off-by: Nicolás Pazos <npazosmendez@gmail.com>

storage/remote/write_handler.go

@@ -19,6 +19,8 @@ import (
 	"fmt"
 	"net/http"
 
+	"github.com/prometheus/prometheus/model/labels"
+
 	"github.com/go-kit/log"
 	"github.com/go-kit/log/level"
 
@@ -129,7 +131,6 @@ func (h *writeHandler) write(ctx context.Context, req *prompb.WriteRequest) (err
 		err = app.Commit()
 	}()
 
-	var exemplarErr error
 	for _, ts := range req.Timeseries {
 		labels := labelProtosToLabels(ts.Labels)
 		if !labels.IsValid() {
@@ -137,53 +138,19 @@ func (h *writeHandler) write(ctx context.Context, req *prompb.WriteRequest) (err
 			samplesWithInvalidLabels++
 			continue
 		}
-		var ref storage.SeriesRef
+		err := h.appendSamples(app, ts.Samples, labels)
-		for _, s := range ts.Samples {
+		if err != nil {
-			ref, err = app.Append(ref, labels, s.Timestamp, s.Value)
+			return err
-			if err != nil {
-				unwrappedErr := errors.Unwrap(err)
-				if unwrappedErr == nil {
-					unwrappedErr = err
-				}
-				if errors.Is(err, storage.ErrOutOfOrderSample) || errors.Is(unwrappedErr, storage.ErrOutOfBounds) || errors.Is(unwrappedErr, storage.ErrDuplicateSampleForTimestamp) {
-					level.Error(h.logger).Log("msg", "Out of order sample from remote write", "err", err.Error(), "series", labels.String(), "timestamp", s.Timestamp)
-				}
-				return err
-			}
-
 		}
 
 		for _, ep := range ts.Exemplars {
 			e := exemplarProtoToExemplar(ep)
-
+			h.appendExemplar(app, e, labels, &outOfOrderExemplarErrs)
-			_, exemplarErr = app.AppendExemplar(0, labels, e)
-			exemplarErr = h.checkAppendExemplarError(exemplarErr, e, &outOfOrderExemplarErrs)
-			if exemplarErr != nil {
-				// Since exemplar storage is still experimental, we don't fail the request on ingestion errors.
-				level.Debug(h.logger).Log("msg", "Error while adding exemplar in AddExemplar", "exemplar", fmt.Sprintf("%+v", e), "err", exemplarErr)
-			}
 		}
 
-		for _, hp := range ts.Histograms {
+		err = h.appendHistograms(app, ts.Histograms, labels)
-			if hp.IsFloatHistogram() {
+		if err != nil {
-				fhs := FloatHistogramProtoToFloatHistogram(hp)
+			return err
-				_, err = app.AppendHistogram(0, labels, hp.Timestamp, nil, fhs)
-			} else {
-				hs := HistogramProtoToHistogram(hp)
-				_, err = app.AppendHistogram(0, labels, hp.Timestamp, hs, nil)
-			}
-			if err != nil {
-				unwrappedErr := errors.Unwrap(err)
-				if unwrappedErr == nil {
-					unwrappedErr = err
-				}
-				// Although AppendHistogram does not currently return ErrDuplicateSampleForTimestamp there is
-				// a note indicating its inclusion in the future.
-				if errors.Is(unwrappedErr, storage.ErrOutOfOrderSample) || errors.Is(unwrappedErr, storage.ErrOutOfBounds) || errors.Is(unwrappedErr, storage.ErrDuplicateSampleForTimestamp) {
-					level.Error(h.logger).Log("msg", "Out of order histogram from remote write", "err", err.Error(), "series", labels.String(), "timestamp", hp.Timestamp)
-				}
-				return err
-			}
 		}
 	}
 
@@ -197,6 +164,61 @@ func (h *writeHandler) write(ctx context.Context, req *prompb.WriteRequest) (err
 	return nil
 }
 
+func (h *writeHandler) appendExemplar(app storage.Appender, e exemplar.Exemplar, labels labels.Labels, outOfOrderExemplarErrs *int) {
+	_, err := app.AppendExemplar(0, labels, e)
+	err = h.checkAppendExemplarError(err, e, outOfOrderExemplarErrs)
+	if err != nil {
+		// Since exemplar storage is still experimental, we don't fail the request on ingestion errors
+		level.Debug(h.logger).Log("msg", "Error while adding exemplar in AddExemplar", "exemplar", fmt.Sprintf("%+v", e), "err", err)
+	}
+}
+
+func (h *writeHandler) appendSamples(app storage.Appender, ss []prompb.Sample, labels labels.Labels) error {
+	var ref storage.SeriesRef
+	var err error
+	for _, s := range ss {
+		ref, err = app.Append(ref, labels, s.Timestamp, s.
+			Value)
+		if err != nil {
+			unwrappedErr := errors.Unwrap(err)
+			if unwrappedErr == nil {
+				unwrappedErr = err
+			}
+			if errors.Is(err, storage.ErrOutOfOrderSample) || errors.Is(unwrappedErr, storage.ErrOutOfBounds) || errors.Is(unwrappedErr, storage.ErrDuplicateSampleForTimestamp) {
+				level.Error(h.logger).Log("msg", "Out of order sample from remote write", "err", err.Error(), "series", labels.String(), "timestamp", s.Timestamp)
+			}
+			return err
+		}
+	}
+	return nil
+}
+
+func (h *writeHandler) appendHistograms(app storage.Appender, hh []prompb.Histogram, labels labels.Labels) error {
+	var err error
+	for _, hp := range hh {
+		if hp.IsFloatHistogram() {
+			fhs := FloatHistogramProtoToFloatHistogram(hp)
+			_, err = app.AppendHistogram(0, labels, hp.Timestamp, nil, fhs)
+		} else {
+			hs := HistogramProtoToHistogram(hp)
+			_, err = app.AppendHistogram(0, labels, hp.Timestamp, hs, nil)
+		}
+		if err != nil {
+			unwrappedErr := errors.Unwrap(err)
+			if unwrappedErr == nil {
+				unwrappedErr = err
+			}
+			// Although AppendHistogram does not currently return ErrDuplicateSampleForTimestamp there is
+			// a note indicating its inclusion in the future.
+			if errors.Is(unwrappedErr, storage.ErrOutOfOrderSample) || errors.Is(unwrappedErr, storage.ErrOutOfBounds) || errors.Is(unwrappedErr, storage.ErrDuplicateSampleForTimestamp) {
+				level.Error(h.logger).Log("msg", "Out of order histogram from remote write", "err", err.Error(), "series", labels.String(), "timestamp", hp.Timestamp)
+			}
+			return err
+		}
+	}
+	return nil
+}
+
 // NewOTLPWriteHandler creates a http.Handler that accepts OTLP write requests and
 // writes them to the provided appendable.
 func NewOTLPWriteHandler(logger log.Logger, appendable storage.Appendable) http.Handler {
@@ -268,51 +290,28 @@ func (h *writeHandler) writeReduced(ctx context.Context, req *prompb.WriteReques
 		err = app.Commit()
 	}()
 
-	var exemplarErr error
 	for _, ts := range req.Timeseries {
 		labels := labelRefProtosToLabels(req.StringSymbolTable, ts.Labels)
+		// TODO(npazosmendez): ?
+		// if !labels.IsValid() {
+		// 	level.Warn(h.logger).Log("msg", "Invalid metric names or labels", "got", labels.String())
+		// 	samplesWithInvalidLabels++
+		// 	continue
+		// }
 
-		for _, s := range ts.Samples {
+		err := h.appendSamples(app, ts.Samples, labels)
-			_, err = app.Append(0, labels, s.Timestamp, s.Value)
+		if err != nil {
-			if err != nil {
+			return err
-				unwrapedErr := errors.Unwrap(err)
-				if errors.Is(unwrapedErr, storage.ErrOutOfOrderSample) || errors.Is(unwrapedErr, storage.ErrOutOfBounds) || errors.Is(unwrapedErr, storage.ErrDuplicateSampleForTimestamp) {
-					level.Error(h.logger).Log("msg", "Out of order sample from remote write", "err", err.Error(), "series", labels.String(), "timestamp", s.Timestamp)
-				}
-				return err
-			}
-
 		}
 
 		for _, ep := range ts.Exemplars {
 			e := exemplarRefProtoToExemplar(req.StringSymbolTable, ep)
-
+			h.appendExemplar(app, e, labels, &outOfOrderExemplarErrs)
-			_, exemplarErr = app.AppendExemplar(0, labels, e)
-			exemplarErr = h.checkAppendExemplarError(exemplarErr, e, &outOfOrderExemplarErrs)
-			if exemplarErr != nil {
-				// Since exemplar storage is still experimental, we don't fail the request on ingestion errors.
-				level.Debug(h.logger).Log("msg", "Error while adding exemplar in AddExemplar", "exemplar", fmt.Sprintf("%+v", e), "err", exemplarErr)
-			}
 		}
 
-		for _, hp := range ts.Histograms {
+		err = h.appendHistograms(app, ts.Histograms, labels)
-			if hp.IsFloatHistogram() {
+		if err != nil {
-				fhs := FloatHistogramProtoToFloatHistogram(hp)
+			return err
-				_, err = app.AppendHistogram(0, labels, hp.Timestamp, nil, fhs)
-			} else {
-				hs := HistogramProtoToHistogram(hp)
-				_, err = app.AppendHistogram(0, labels, hp.Timestamp, hs, nil)
-			}
-
-			if err != nil {
-				unwrappedErr := errors.Unwrap(err)
-				// Althogh AppendHistogram does not currently return ErrDuplicateSampleForTimestamp there is
-				// a note indicating its inclusion in the future.
-				if errors.Is(unwrappedErr, storage.ErrOutOfOrderSample) || errors.Is(unwrappedErr, storage.ErrOutOfBounds) || errors.Is(unwrappedErr, storage.ErrDuplicateSampleForTimestamp) {
-					level.Error(h.logger).Log("msg", "Out of order histogram from remote write", "err", err.Error(), "series", labels.String(), "timestamp", hp.Timestamp)
-				}
-				return err
-			}
 		}
 	}