Relax consistency checks during gathering

Also, clarify in the doc comment.

Previously, the assumption was that inconsistent label dimensions are
violating the exposition format spec. However, especially with the
knowledge that OpenMetrics will explicitly allow inconsistent label
dimensions in expositions, we should allow it in client_golang, too.

Note that registration with proper Descs provided will still check for
consistont label dimensions. However, you can "cheat" with custom
Collectors as you can collect metrics that don't follew the provided
Desc (this will be made more explicit and less cheaty once #47 is
fixed). You can also create expositions with inconsistent label
dimensions by merging Gatherers with the Gatherers slice type. (The
latter is used in the Pushgateway.)

Effectively, normal direct instrumentation will always have consistent
label dimensions in this way, but you can cover special use cases with
custom collectors or merging of different Gatherers.

Signed-off-by: beorn7 <beorn@soundcloud.com>

prometheus/examples_test.go

@@ -712,7 +712,7 @@ humidity_percent{location="inside"} 33.2
 # HELP temperature_kelvin Temperature in Kelvin.
 # Duplicate metric:
 temperature_kelvin{location="outside"} 265.3
- # Wrong labels:
+ # Missing location label (note that this is undesirable but valid):
 temperature_kelvin 4.5
 `
 
@@ -740,15 +740,14 @@ temperature_kelvin 4.5
 	// temperature_kelvin{location="outside"} 273.14
 	// temperature_kelvin{location="somewhere else"} 4.5
 	// ----------
-	// 2 error(s) occurred:
+	// collected metric temperature_kelvin label:<name:"location" value:"outside" > gauge:<value:265.3 >  was collected before with the same name and label values
-	// * collected metric temperature_kelvin label:<name:"location" value:"outside" > gauge:<value:265.3 >  was collected before with the same name and label values
-	// * collected metric temperature_kelvin gauge:<value:4.5 >  has label dimensions inconsistent with previously collected metrics in the same metric family
 	// # HELP humidity_percent Humidity in %.
 	// # TYPE humidity_percent gauge
 	// humidity_percent{location="inside"} 33.2
 	// humidity_percent{location="outside"} 45.4
 	// # HELP temperature_kelvin Temperature in Kelvin.
 	// # TYPE temperature_kelvin gauge
+	// temperature_kelvin 4.5
 	// temperature_kelvin{location="inside"} 298.44
 	// temperature_kelvin{location="outside"} 273.14
 }

prometheus/registry.go

@@ -126,15 +126,21 @@ type Registerer interface {
 type Gatherer interface {
 	// Gather calls the Collect method of the registered Collectors and then
 	// gathers the collected metrics into a lexicographically sorted slice
-	// of MetricFamily protobufs. Even if an error occurs, Gather attempts
+	// of uniquely named MetricFamily protobufs. Gather ensures that the
-	// to gather as many metrics as possible. Hence, if a non-nil error is
+	// returned slice is valid and self-consistent so that it can be used
-	// returned, the returned MetricFamily slice could be nil (in case of a
+	// for valid exposition. As an exception to the strict consistency
-	// fatal error that prevented any meaningful metric collection) or
+	// requirements described for metric.Desc, Gather will tolerate
-	// contain a number of MetricFamily protobufs, some of which might be
+	// different sets of label names for metrics of the same metric family.
-	// incomplete, and some might be missing altogether. The returned error
+	//
-	// (which might be a MultiError) explains the details. In scenarios
+	// Even if an error occurs, Gather attempts to gather as many metrics as
-	// where complete collection is critical, the returned MetricFamily
+	// possible. Hence, if a non-nil error is returned, the returned
-	// protobufs should be disregarded if the returned error is non-nil.
+	// MetricFamily slice could be nil (in case of a fatal error that
+	// prevented any meaningful metric collection) or contain a number of
+	// MetricFamily protobufs, some of which might be incomplete, and some
+	// might be missing altogether. The returned error (which might be a
+	// MultiError) explains the details. In scenarios where complete
+	// collection is critical, the returned MetricFamily protobufs should be
+	// disregarded if the returned error is non-nil.
 	Gather() ([]*dto.MetricFamily, error)
 }
 
@@ -370,7 +376,6 @@ func (r *Registry) Gather() ([]*dto.MetricFamily, error) {
 	var (
 		metricChan        = make(chan Metric, capMetricChan)
 		metricHashes      = map[uint64]struct{}{}
-		dimHashes         = map[string]uint64{}
 		wg                sync.WaitGroup
 		errs              MultiError          // The collected errors to return in the end.
 		registeredDescIDs map[uint64]struct{} // Only used for pedantic checks
@@ -433,7 +438,7 @@ collectLoop:
 			}
 			errs.Append(processMetric(
 				metric, metricFamiliesByName,
-				metricHashes, dimHashes,
+				metricHashes,
 				registeredDescIDs,
 			))
 		default:
@@ -445,7 +450,7 @@ collectLoop:
 				for metric := range metricChan {
 					errs.Append(processMetric(
 						metric, metricFamiliesByName,
-						metricHashes, dimHashes,
+						metricHashes,
 						registeredDescIDs,
 					))
 				}
@@ -465,7 +470,6 @@ func processMetric(
 	metric Metric,
 	metricFamiliesByName map[string]*dto.MetricFamily,
 	metricHashes map[uint64]struct{},
-	dimHashes map[string]uint64,
 	registeredDescIDs map[uint64]struct{},
 ) error {
 	desc := metric.Desc()
@@ -542,7 +546,7 @@ func processMetric(
 		}
 		metricFamiliesByName[desc.fqName] = metricFamily
 	}
-	if err := checkMetricConsistency(metricFamily, dtoMetric, metricHashes, dimHashes); err != nil {
+	if err := checkMetricConsistency(metricFamily, dtoMetric, metricHashes); err != nil {
 		return err
 	}
 	if registeredDescIDs != nil {
@@ -584,7 +588,6 @@ func (gs Gatherers) Gather() ([]*dto.MetricFamily, error) {
 	var (
 		metricFamiliesByName = map[string]*dto.MetricFamily{}
 		metricHashes         = map[uint64]struct{}{}
-		dimHashes            = map[string]uint64{}
 		errs                 MultiError // The collected errors to return in the end.
 	)
 
@@ -624,7 +627,7 @@ func (gs Gatherers) Gather() ([]*dto.MetricFamily, error) {
 				metricFamiliesByName[mf.GetName()] = existingMF
 			}
 			for _, m := range mf.Metric {
-				if err := checkMetricConsistency(existingMF, m, metricHashes, dimHashes); err != nil {
+				if err := checkMetricConsistency(existingMF, m, metricHashes); err != nil {
 					errs = append(errs, err)
 					continue
 				}
@@ -701,18 +704,13 @@ func normalizeMetricFamilies(metricFamiliesByName map[string]*dto.MetricFamily)
 }
 
 // checkMetricConsistency checks if the provided Metric is consistent with the
-// provided MetricFamily. It also hashed the Metric labels and the MetricFamily
+// provided MetricFamily. It also hashes the Metric labels and the MetricFamily
 // name. If the resulting hash is already in the provided metricHashes, an error
-// is returned. If not, it is added to metricHashes. The provided dimHashes maps
+// is returned. If not, it is added to metricHashes.
-// MetricFamily names to their dimHash (hashed sorted label names). If dimHashes
-// doesn't yet contain a hash for the provided MetricFamily, it is
-// added. Otherwise, an error is returned if the existing dimHashes in not equal
-// the calculated dimHash.
 func checkMetricConsistency(
 	metricFamily *dto.MetricFamily,
 	dtoMetric *dto.Metric,
 	metricHashes map[uint64]struct{},
-	dimHashes map[string]uint64,
 ) error {
 	// Type consistency with metric family.
 	if metricFamily.GetType() == dto.MetricType_GAUGE && dtoMetric.Gauge == nil ||
@@ -732,19 +730,18 @@ func checkMetricConsistency(
 		}
 	}
 
-	// Is the metric unique (i.e. no other metric with the same name and the same label values)?
+	// Is the metric unique (i.e. no other metric with the same name and the same labels)?
 	h := hashNew()
 	h = hashAdd(h, metricFamily.GetName())
 	h = hashAddByte(h, separatorByte)
-	dh := hashNew()
 	// Make sure label pairs are sorted. We depend on it for the consistency
 	// check.
 	sort.Sort(LabelPairSorter(dtoMetric.Label))
 	for _, lp := range dtoMetric.Label {
+		h = hashAdd(h, lp.GetName())
+		h = hashAddByte(h, separatorByte)
 		h = hashAdd(h, lp.GetValue())
 		h = hashAddByte(h, separatorByte)
-		dh = hashAdd(dh, lp.GetName())
-		dh = hashAddByte(dh, separatorByte)
 	}
 	if _, exists := metricHashes[h]; exists {
 		return fmt.Errorf(
@@ -752,16 +749,6 @@ func checkMetricConsistency(
 			metricFamily.GetName(), dtoMetric,
 		)
 	}
-	if dimHash, ok := dimHashes[metricFamily.GetName()]; ok {
-		if dimHash != dh {
-			return fmt.Errorf(
-				"collected metric %s %s has label dimensions inconsistent with previously collected metrics in the same metric family",
-				metricFamily.GetName(), dtoMetric,
-			)
-		}
-	} else {
-		dimHashes[metricFamily.GetName()] = dh
-	}
 	metricHashes[h] = struct{}{}
 	return nil
 }