Repository: prometheus/prometheus
File: storage/remote/otlptranslator/prometheusremotewrite/combined_appender.go (Go)

Code before:
			newRef, err = b.app.Append(ref, ls, t, v)
		}
		if err != nil {
			// Although Append does not currently return ErrDuplicateSampleForTimestamp there is
			// a note indicating its inclusion in the future.
			if errors.Is(err, storage.ErrOutOfOrderSample) ||
				errors.Is(err, storage.ErrOutOfBounds) ||
				errors.Is(err, storage.ErrDuplicateSampleForTimestamp) {
				b.logger.Error("Error when appending sample from OTLP", "err", err.Error(), "series", ls.String(), "timestamp", t, "sample_type", sampleType(h))
			}
		} else {
			// If the append was successful, we can use the returned reference.
			ref = newRef
		}
	}

	if ref == 0 {
		// We cannot update metadata or add exemplars on non existent series.
		return err
	}

	if !exists || series.meta.Help != meta.Help || series.meta.Type != meta.Type || series.meta.Unit != meta.Unit {
		updateRefs = true
		// Only update metadata in WAL if the metadata-wal-records feature is enabled.
		// Without this feature, metadata is not persisted to WAL.
		if b.appendMetadata {
			newRef, err := b.app.UpdateMetadata(ref, ls, meta)
			if err != nil {
				b.samplesAppendedWithoutMetadata.Add(1)
				b.logger.Warn("Error while updating metadata from OTLP", "err", err)
			} else {
				// Use the returned reference on success.
				ref = newRef
			}
		}
	}

	if updateRefs {
		b.refs[hash] = seriesRef{
			ref:  ref,
			ct:   ct,
			ls:   ls,
			meta: meta,
		}
	}

	b.appendExemplars(ref, ls, es)

	return err
}


Review comment (refactor):
no need to make an extra if, you can move this into line 194:
`if b.appendMetadata && (!exists || series.meta.Help != ...) {`

Code after:
			newRef, err = b.app.Append(ref, ls, t, v)
		}
		if err != nil {
			// Although Append does not currently return ErrDuplicateSampleForTimestamp there is
			// a note indicating its inclusion in the future.
			if errors.Is(err, storage.ErrOutOfOrderSample) ||
				errors.Is(err, storage.ErrOutOfBounds) ||
				errors.Is(err, storage.ErrDuplicateSampleForTimestamp) {
				b.logger.Error("Error when appending sample from OTLP", "err", err.Error(), "series", ls.String(), "timestamp", t, "sample_type", sampleType(h))
			}
		} else {
			// If the append was successful, we can use the returned reference.
			ref = newRef
		}
	}

	if ref == 0 {
		// We cannot update metadata or add exemplars on non existent series.
		return
	}

	metadataChanged := exists && (series.meta.Help != meta.Help || series.meta.Type != meta.Type || series.meta.Unit != meta.Unit)

	// Update cache if references changed or metadata changed.
	if updateRefs || metadataChanged {
		b.refs[hash] = seriesRef{
			ref:  ref,
			ct:   ct,
			ls:   ls,
			meta: meta,
		}
	}

	// Update metadata in storage if enabled and needed.
	if b.appendMetadata && (!exists || metadataChanged) {
		// Only update metadata in WAL if the metadata-wal-records feature is enabled.
		// Without this feature, metadata is not persisted to WAL.
		_, err := b.app.UpdateMetadata(ref, ls, meta)
		if err != nil {
			b.samplesAppendedWithoutMetadata.Add(1)
			b.logger.Warn("Error while updating metadata from OTLP", "err", err)
		}
	}

	b.appendExemplars(ref, ls, es)

	return
}

func sampleType(h *histogram.Histogram) string {
	if h == nil {