Fix deadlock, structure target logging

retrieval/target.go

@@ -453,6 +453,7 @@ func (t *Target) scrape(appender storage.SampleAppender) (err error) {
 	var (
 		samples       model.Vector
 		numOutOfOrder int
+		logger        = log.With("target", t.InstanceIdentifier())
 	)
 	for {
 		if err = sdec.Decode(&samples); err != nil {
@@ -464,14 +465,14 @@ func (t *Target) scrape(appender storage.SampleAppender) (err error) {
 				if err == local.ErrOutOfOrderSample {
 					numOutOfOrder++
 				} else {
-					log.Warnf("Error inserting sample %v: %s", s, err)
+					logger.With("sample", s).Warnf("Error inserting sample: %s", err)
 				}
 			}
 
 		}
 	}
 	if numOutOfOrder > 0 {
-		log.Warnf("Error on ingesting %d out-of-order samples")
+		logger.With("numDropped", numOutOfOrder).Warn("Error on ingesting out-of-order samples")
 	}
 
 	if err == io.EOF {

storage/local/storage.go

@@ -591,6 +591,7 @@ func (s *memorySeriesStorage) Append(sample *model.Sample) error {
 	series := s.getOrCreateSeries(fp, sample.Metric)
 
 	if sample.Timestamp <= series.lastTime {
+		s.fpLocker.Unlock(fp)
 		// Don't log and track equal timestamps, as they are a common occurrence
 		// when using client-side timestamps (e.g. Pushgateway or federation).
 		// It would be even better to also compare the sample values here, but
@@ -599,7 +600,6 @@ func (s *memorySeriesStorage) Append(sample *model.Sample) error {
 			s.outOfOrderSamplesCount.Inc()
 			return ErrOutOfOrderSample
 		}
-		s.fpLocker.Unlock(fp)
 		return nil
 	}
 	completedChunksCount := series.add(&model.SamplePair{

storage/storage.go

@@ -25,8 +25,7 @@ type SampleAppender interface {
 	// of the sample after Append has returned. Remote storage
 	// implementation will simply drop samples if they cannot keep up with
 	// sending samples. Local storage implementations will only drop metrics
-	// upon unrecoverable errors. Reporting any errors is done via metrics
+	// upon unrecoverable errors.
-	// and logs and not the concern of the caller.
 	Append(*model.Sample) error
 	// NeedsThrottling returns true if the underlying storage wishes to not
 	// receive any more samples. Append will still work but might lead to