Fix for bad prometheus metrics name (#205)

* improved internal metrics config setup
* internal metric names are sanitized at runtime

m3/reporter_integration_test.go

@@ -110,7 +110,7 @@ func testProcessFlushOnExit(t *testing.T, i int) {
 	require.Equal(t, 1, len(server.Service.getBatches()))
 	require.NotNil(t, server.Service.getBatches()[0])
 	// 3 metrics are emitted by mainFileFmt plus various other internal metrics.
-	require.Equal(t, internalMetrics+cardinalityMetrics+3, len(server.Service.getBatches()[0].GetMetrics()))
+	require.Equal(t, internalMetrics+3, len(server.Service.getBatches()[0].GetMetrics()))
 	metrics := server.Service.getBatches()[0].GetMetrics()
 	fmt.Printf("Test %d emitted:\n%v\n", i, metrics)
 }

m3/scope_test.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2021 Uber Technologies, Inc.
+// Copyright (c) 2023 Uber Technologies, Inc.
 //
 // Permission is hereby granted, free of charge, to any person obtaining a copy
 // of this software and associated documentation files (the "Software"), to deal
@@ -53,6 +53,7 @@ func newTestReporterScope(
 		Prefix:         scopePrefix,
 		Tags:           scopeTags,
 		CachedReporter: r,
+		MetricsOption:  tally.SendInternalMetrics,
 	}, shortInterval)
 
 	return r, scope, func() {

scope.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2022 Uber Technologies, Inc.
+// Copyright (c) 2023 Uber Technologies, Inc.
 //
 // Permission is hereby granted, free of charge, to any person obtaining a copy
 // of this software and associated documentation files (the "Software"), to deal
@@ -28,7 +28,17 @@ import (
 	"go.uber.org/atomic"
 )
 
+// InternalMetricOption is used to configure internal metrics.
+type InternalMetricOption int
+
 const (
+	// Unset is the "no-op" config, which turns off internal metrics.
+	Unset InternalMetricOption = iota
+	// SendInternalMetrics turns on internal metrics submission.
+	SendInternalMetrics
+	// OmitInternalMetrics turns off internal metrics submission.
+	OmitInternalMetrics
+
 	_defaultInitialSliceSize = 16
 )
 
@@ -95,15 +105,15 @@ type scope struct {
 
 // ScopeOptions is a set of options to construct a scope.
 type ScopeOptions struct {
-	Tags                map[string]string
-	Prefix              string
-	Reporter            StatsReporter
-	CachedReporter      CachedStatsReporter
-	Separator           string
-	DefaultBuckets      Buckets
-	SanitizeOptions     *SanitizeOptions
-	registryShardCount  uint
-	skipInternalMetrics bool
+	Tags               map[string]string
+	Prefix             string
+	Reporter           StatsReporter
+	CachedReporter     CachedStatsReporter
+	Separator          string
+	DefaultBuckets     Buckets
+	SanitizeOptions    *SanitizeOptions
+	registryShardCount uint
+	MetricsOption      InternalMetricOption
 }
 
 // NewRootScope creates a new root Scope with a set of options and
@@ -173,7 +183,7 @@ func newRootScope(opts ScopeOptions, interval time.Duration) *scope {
 	s.tags = s.copyAndSanitizeMap(opts.Tags)
 
 	// Register the root scope
-	s.registry = newScopeRegistryWithShardCount(s, opts.registryShardCount, opts.skipInternalMetrics)
+	s.registry = newScopeRegistryWithShardCount(s, opts.registryShardCount, opts.MetricsOption)
 
 	if interval > 0 {
 		s.wg.Add(1)

scope_registry.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2022 Uber Technologies, Inc.
+// Copyright (c) 2023 Uber Technologies, Inc.
 //
 // Permission is hereby granted, free of charge, to any person obtaining a copy
 // of this software and associated documentation files (the "Software"), to deal
@@ -35,35 +35,45 @@ var (
 
 	// Metrics related.
 	internalTags             = map[string]string{"version": Version}
-	counterCardinalityName   = "tally.internal.counter-cardinality"
-	gaugeCardinalityName     = "tally.internal.gauge-cardinality"
-	histogramCardinalityName = "tally.internal.histogram-cardinality"
+	counterCardinalityName   = "tally_internal_counter_cardinality"
+	gaugeCardinalityName     = "tally_internal_gauge_cardinality"
+	histogramCardinalityName = "tally_internal_histogram_cardinality"
 )
 
 type scopeRegistry struct {
 	seed maphash.Seed
 	root *scope
 	// We need a subscope per GOPROC so that we can take advantage of all the cpu available to the application.
 	subscopes []*scopeBucket
-	// Toggles internal metrics reporting.
-	skipInternalMetrics bool
+	// Internal metrics related.
+	internalMetricsOption             InternalMetricOption
+	sanitizedCounterCardinalityName   string
+	sanitizedGaugeCardinalityName     string
+	sanitizedHistogramCardinalityName string
 }
 
 type scopeBucket struct {
 	mu sync.RWMutex
 	s  map[string]*scope
 }
 
-func newScopeRegistryWithShardCount(root *scope, shardCount uint, skipInternalMetrics bool) *scopeRegistry {
+func newScopeRegistryWithShardCount(
+	root *scope,
+	shardCount uint,
+	internalMetricsOption InternalMetricOption,
+) *scopeRegistry {
 	if shardCount == 0 {
 		shardCount = uint(runtime.GOMAXPROCS(-1))
 	}
 
 	r := &scopeRegistry{
-		root:                root,
-		subscopes:           make([]*scopeBucket, shardCount),
-		seed:                maphash.MakeSeed(),
-		skipInternalMetrics: skipInternalMetrics,
+		root:                              root,
+		subscopes:                         make([]*scopeBucket, shardCount),
+		seed:                              maphash.MakeSeed(),
+		internalMetricsOption:             internalMetricsOption,
+		sanitizedCounterCardinalityName:   root.sanitizer.Name(counterCardinalityName),
+		sanitizedGaugeCardinalityName:     root.sanitizer.Name(gaugeCardinalityName),
+		sanitizedHistogramCardinalityName: root.sanitizer.Name(histogramCardinalityName),
 	}
 	for i := uint(0); i < shardCount; i++ {
 		r.subscopes[i] = &scopeBucket{
@@ -228,40 +238,42 @@ func (r *scopeRegistry) removeWithRLock(subscopeBucket *scopeBucket, key string)
 
 // Records internal Metrics' cardinalities.
 func (r *scopeRegistry) reportInternalMetrics() {
-	if r.skipInternalMetrics {
+	if r.internalMetricsOption != SendInternalMetrics {
 		return
 	}
 
 	counters, gauges, histograms := atomic.Int64{}, atomic.Int64{}, atomic.Int64{}
 	rootCounters, rootGauges, rootHistograms := atomic.Int64{}, atomic.Int64{}, atomic.Int64{}
-	r.ForEachScope(func(ss *scope) {
-		counterSliceLen, gaugeSliceLen, histogramSliceLen := int64(len(ss.countersSlice)), int64(len(ss.gaugesSlice)), int64(len(ss.histogramsSlice))
-		if ss.root { // Root scope is referenced across all buckets.
-			rootCounters.Store(counterSliceLen)
-			rootGauges.Store(gaugeSliceLen)
-			rootHistograms.Store(histogramSliceLen)
-			return
-		}
-		counters.Add(counterSliceLen)
-		gauges.Add(gaugeSliceLen)
-		histograms.Add(histogramSliceLen)
-	})
+	r.ForEachScope(
+		func(ss *scope) {
+			counterSliceLen, gaugeSliceLen, histogramSliceLen := int64(len(ss.countersSlice)), int64(len(ss.gaugesSlice)), int64(len(ss.histogramsSlice))
+			if ss.root { // Root scope is referenced across all buckets.
+				rootCounters.Store(counterSliceLen)
+				rootGauges.Store(gaugeSliceLen)
+				rootHistograms.Store(histogramSliceLen)
+				return
+			}
+			counters.Add(counterSliceLen)
+			gauges.Add(gaugeSliceLen)
+			histograms.Add(histogramSliceLen)
+		},
+	)
 
 	counters.Add(rootCounters.Load())
 	gauges.Add(rootGauges.Load())
 	histograms.Add(rootHistograms.Load())
 	log.Printf("counters: %v, gauges: %v, histograms: %v\n", counters.Load(), gauges.Load(), histograms.Load())
 
 	if r.root.reporter != nil {
-		r.root.reporter.ReportCounter(counterCardinalityName, internalTags, counters.Load())
-		r.root.reporter.ReportCounter(gaugeCardinalityName, internalTags, gauges.Load())
-		r.root.reporter.ReportCounter(histogramCardinalityName, internalTags, histograms.Load())
+		r.root.reporter.ReportCounter(r.sanitizedCounterCardinalityName, internalTags, counters.Load())
+		r.root.reporter.ReportCounter(r.sanitizedGaugeCardinalityName, internalTags, gauges.Load())
+		r.root.reporter.ReportCounter(r.sanitizedHistogramCardinalityName, internalTags, histograms.Load())
 	}
 
 	if r.root.cachedReporter != nil {
-		numCounters := r.root.cachedReporter.AllocateCounter(counterCardinalityName, internalTags)
-		numGauges := r.root.cachedReporter.AllocateCounter(gaugeCardinalityName, internalTags)
-		numHistograms := r.root.cachedReporter.AllocateCounter(histogramCardinalityName, internalTags)
+		numCounters := r.root.cachedReporter.AllocateCounter(r.sanitizedCounterCardinalityName, internalTags)
+		numGauges := r.root.cachedReporter.AllocateCounter(r.sanitizedGaugeCardinalityName, internalTags)
+		numHistograms := r.root.cachedReporter.AllocateCounter(r.sanitizedHistogramCardinalityName, internalTags)
 		numCounters.ReportCount(counters.Load())
 		numGauges.ReportCount(gauges.Load())
 		numHistograms.ReportCount(histograms.Load())

scope_registry_test.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2022 Uber Technologies, Inc.
+// Copyright (c) 2023 Uber Technologies, Inc.
 //
 // Permission is hereby granted, free of charge, to any person obtaining a copy
 // of this software and associated documentation files (the "Software"), to deal
@@ -55,7 +55,7 @@ func TestVerifyCachedTaggedScopesAlloc(t *testing.T) {
 
 func TestNewTestStatsReporterOneScope(t *testing.T) {
 	r := newTestStatsReporter()
-	root, closer := NewRootScope(ScopeOptions{Reporter: r, skipInternalMetrics: false}, 0)
+	root, closer := NewRootScope(ScopeOptions{Reporter: r, MetricsOption: SendInternalMetrics}, 0)
 	s := root.(*scope)
 
 	numFakeCounters := 3
@@ -101,7 +101,7 @@ func TestNewTestStatsReporterOneScope(t *testing.T) {
 
 func TestNewTestStatsReporterManyScopes(t *testing.T) {
 	r := newTestStatsReporter()
-	root, closer := NewRootScope(ScopeOptions{Reporter: r, skipInternalMetrics: false}, 0)
+	root, closer := NewRootScope(ScopeOptions{Reporter: r, MetricsOption: SendInternalMetrics}, 0)
 	wantCounters, wantGauges, wantHistograms := int64(3), int64(2), int64(1)
 
 	s := root.(*scope)