fix: don't cache invalid metrics

collector.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"database/sql"
 	"fmt"
-	"strings"
 	"sync"
 	"time"
 
@@ -31,13 +30,7 @@ type collector struct {
 // NewCollector returns a new Collector with the given configuration and database. The metrics it creates will all have
 // the provided const labels applied.
 func NewCollector(logContext string, cc *config.CollectorConfig, constLabels []*dto.LabelPair) (Collector, errors.WithContext) {
-	logContext = fmt.Sprintf(`%s,collector=%s`, logContext, cc.Name)
-
-	// Leading comma appears when previous parameter is undefined, which is a side-effect of running in single target mode.
-	// Let's trim to avoid confusions.
-	if strings.HasPrefix(logContext, ",") {
-		logContext = strings.TrimLeft(logContext, ", ")
-	}
+	logContext = TrimMissingCtx(fmt.Sprintf(`%s,collector=%s`, logContext, cc.Name))
 
 	// Maps each query to the list of metric families it populates.
 	queryMFs := make(map[*config.QueryConfig][]*MetricFamily, len(cc.Metrics))
@@ -121,12 +114,12 @@ func (cc *cachingCollector) Collect(ctx context.Context, conn *sql.DB, ch chan<-
 		ch <- NewInvalidMetric(errors.Wrap(cc.rawColl.logContext, ctx.Err()))
 		return
 	}
-
+	klog.Infof("Cache size: %d", len(cc.cache))
 	collTime := time.Now()
 	select {
 	case cacheTime := <-cc.cacheSem:
 		// Have the lock.
-		if age := collTime.Sub(cacheTime); age > cc.minInterval {
+		if age := collTime.Sub(cacheTime); age > cc.minInterval || len(cc.cache) == 0 {
 			// Cache contents are older than minInterval, collect fresh metrics, cache them and pipe them through.
 			klog.V(2).Infof("[%s] Collecting fresh metrics: min_interval=%.3fs cache_age=%.3fs",
 				cc.rawColl.logContext, cc.minInterval.Seconds(), age.Seconds())
@@ -137,6 +130,13 @@ func (cc *cachingCollector) Collect(ctx context.Context, conn *sql.DB, ch chan<-
 				close(cacheChan)
 			}()
 			for metric := range cacheChan {
+				// catch invalid metrics and return them immediately, don't cache them
+				if ctx.Err() != nil {
+					klog.V(2).Infof("[%s] Context closed, returning invalid metric", cc.rawColl.logContext)
+					ch <- NewInvalidMetric(errors.Wrap(cc.rawColl.logContext, ctx.Err()))
+					continue
+				}
+
 				cc.cache = append(cc.cache, metric)
 				ch <- metric
 			}

exporter.go

@@ -197,3 +197,12 @@ func parseContextLog(list string) map[string]string {
 	}
 	return m
 }
+
+// Leading comma appears when previous parameter is undefined, which is a side-effect of running in single target mode.
+// Let's trim to avoid confusions.
+func TrimMissingCtx(logContext string) string {
+	if strings.HasPrefix(logContext, ",") {
+		logContext = strings.TrimLeft(logContext, ", ")
+	}
+	return logContext
+}

metric.go

@@ -36,7 +36,7 @@ type MetricFamily struct {
 
 // NewMetricFamily creates a new MetricFamily with the given metric config and const labels (e.g. job and instance).
 func NewMetricFamily(logContext string, mc *config.MetricConfig, constLabels []*dto.LabelPair) (*MetricFamily, errors.WithContext) {
-	logContext = fmt.Sprintf(`%s,metric=%s`, logContext, mc.Name)
+	logContext = TrimMissingCtx(fmt.Sprintf(`%s,metric=%s`, logContext, mc.Name))
 
 	if len(mc.Values) == 0 && mc.StaticValue == nil {
 		return nil, errors.New(logContext, "no value column defined")

query.go

@@ -34,7 +34,7 @@ const (
 
 // NewQuery returns a new Query that will populate the given metric families.
 func NewQuery(logContext string, qc *config.QueryConfig, metricFamilies ...*MetricFamily) (*Query, errors.WithContext) {
-	logContext = fmt.Sprintf(`%s,query=%s`, logContext, qc.Name)
+	logContext = TrimMissingCtx(fmt.Sprintf(`%s,query=%s`, logContext, qc.Name))
 
 	columnTypes := make(columnTypeMap)
 

target.go

@@ -57,7 +57,7 @@ func NewTarget(
 ) {
 
 	if tname != "" {
-		logContext = fmt.Sprintf(`%s,target=%s`, logContext, tname)
+		logContext = TrimMissingCtx(fmt.Sprintf(`%s,target=%s`, logContext, tname))
 		if constLabels == nil {
 			constLabels = prometheus.Labels{config.TargetLabel: tname}
 		}