cleanup

Signed-off-by: Owen Williams <[email protected]>

prometheus/desc.go

@@ -168,7 +168,6 @@ func (v2) NewDesc(fqName, help string, variableLabels ConstrainableLabels, const
 		xxh.WriteString(labelName)
 		xxh.Write(separatorByteSlice)
 	}
-
 	d.dimHash = xxh.Sum64()
 
 	d.constLabelPairs = make([]*dto.LabelPair, 0, len(constLabels))

prometheus/registry.go

@@ -271,11 +271,12 @@ func (errs MultiError) MaybeUnwrap() error {
 type Registry struct {
 	mtx            sync.RWMutex
 	collectorsByID map[uint64]Collector // ID is a hash of the descIDs.
-	// stores colletors by escapedID, only if escaped id is different (otherwise
-	// we can just do the lookup in the regular map).
+	// collectorsByEscapedID stores colletors by escapedID, only if escaped id is
+	// different (otherwise we can just do the lookup in the regular map).
 	collectorsByEscapedID map[uint64]Collector
 	descIDs               map[uint64]struct{}
-	// desc ids, only if different
+	// escapedDescIDs records desc ids of the escaped version of the metric, only
+	// if different from the regular name.
 	escapedDescIDs        map[uint64]struct{}
 	dimHashesByName       map[string]uint64
 	uncheckedCollectors   []Collector
@@ -329,10 +330,11 @@ func (r *Registry) Register(c Collector) error {
 		// Unless we are in pure UTF-8 mode, also check to see if the descID is
 		// unique when all the names are escaped to underscores.
 		if !r.allowEscapedCollision {
-			if _, exists := r.escapedDescIDs[desc.escapedID]; exists {
+			// First check the primary map, then check the secondary map.
+			if _, exists := r.descIDs[desc.escapedID]; exists {
 				duplicateDescErr = fmt.Errorf("descriptor %s will collide with an existing descriptor when escaped for compatibility with non-UTF8 systems", desc)
 			}
-			if _, exists := r.descIDs[desc.escapedID]; exists {
+			if _, exists := r.escapedDescIDs[desc.escapedID]; exists {
 				duplicateDescErr = fmt.Errorf("descriptor %s will collide with an existing descriptor when escaped for compatibility with non-UTF8 systems", desc)
 			}
 		}

prometheus/registry_test.go

@@ -1197,10 +1197,9 @@ func TestAlreadyRegisteredEscapingCollision(t *testing.T) {
 		counterB      func() prometheus.Counter
 		utf8Collision bool
 		expectErr     bool
-		// Since the collision mode will be Compat on startup, metrics created in
-		// init() functions will use that hashing mode. Metrics created *after*
-		// startup could be created with a different hashing mode. This bool tests
-		// that case.
+		// postInitFlagFlip tests for the case where metrics are created in an
+		// init() function, when the mode will be to disallow legacy collisions, and
+		// then the user later selects to allow them.
 		postInitFlagFlip bool
 	}{
 		{
@@ -1247,6 +1246,8 @@ func TestAlreadyRegisteredEscapingCollision(t *testing.T) {
 			expectErr: true,
 		},
 		{
+			// This is a regression test to make sure we are not accidentally
+			// reporting collisions when label values are different.
 			name: "no label value collision",
 			counterA: func() prometheus.Counter {
 				return prometheus.NewCounter(prometheus.CounterOpts{
@@ -1344,7 +1345,6 @@ func TestAlreadyRegisteredEscapingCollision(t *testing.T) {
 			} else {
 				reg.AllowEscapedCollisions(tc.utf8Collision)
 			}
-			fmt.Println("------------")
 			err := reg.Register(tc.counterA())
 			if err != nil {
 				t.Errorf("expected no error, got: %v", err)