Use promauto.With(stats) instead of stats.MustRegister() (#8483)

This removes the need to remember to register each metric that we
declare, and reduces the likelihood of bugs related to not registering
metrics. Using this throughout the codebase means that we'll remember to
continue using it in the future.

Author: aarongable

ca/ca.go

@@ -21,6 +21,7 @@ import (
 	"github.com/jmhodges/clock"
 	"github.com/miekg/pkcs11"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/trace"
@@ -79,34 +80,25 @@ type caMetrics struct {
 }
 
 func NewCAMetrics(stats prometheus.Registerer) *caMetrics {
-	signatureCount := prometheus.NewCounterVec(
-		prometheus.CounterOpts{
-			Name: "signatures",
-			Help: "Number of signatures",
-		},
-		[]string{"purpose", "issuer"})
-	stats.MustRegister(signatureCount)
-
-	signErrorCount := prometheus.NewCounterVec(prometheus.CounterOpts{
+	signatureCount := promauto.With(stats).NewCounterVec(prometheus.CounterOpts{
+		Name: "signatures",
+		Help: "Number of signatures",
+	}, []string{"purpose", "issuer"})
+
+	signErrorCount := promauto.With(stats).NewCounterVec(prometheus.CounterOpts{
 		Name: "signature_errors",
 		Help: "A counter of signature errors labelled by error type",
 	}, []string{"type"})
-	stats.MustRegister(signErrorCount)
-
-	lintErrorCount := prometheus.NewCounter(
-		prometheus.CounterOpts{
-			Name: "lint_errors",
-			Help: "Number of issuances that were halted by linting errors",
-		})
-	stats.MustRegister(lintErrorCount)
-
-	certificates := prometheus.NewCounterVec(
-		prometheus.CounterOpts{
-			Name: "certificates",
-			Help: "Number of certificates issued",
-		},
-		[]string{"profile"})
-	stats.MustRegister(certificates)
+
+	lintErrorCount := promauto.With(stats).NewCounter(prometheus.CounterOpts{
+		Name: "lint_errors",
+		Help: "Number of issuances that were halted by linting errors",
+	})
+
+	certificates := promauto.With(stats).NewCounterVec(prometheus.CounterOpts{
+		Name: "certificates",
+		Help: "Number of certificates issued",
+	}, []string{"profile"})
 
 	return &caMetrics{signatureCount, signErrorCount, lintErrorCount, certificates}
 }

cmd/bad-key-revoker/main.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/jmhodges/clock"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
 	"google.golang.org/grpc"
 	"google.golang.org/protobuf/types/known/emptypb"
 
@@ -25,19 +26,6 @@ import (
 
 const blockedKeysGaugeLimit = 1000
 
-var keysToProcess = prometheus.NewGauge(prometheus.GaugeOpts{
-	Name: "bad_keys_to_process",
-	Help: fmt.Sprintf("A gauge of blockedKeys rows to process (max: %d)", blockedKeysGaugeLimit),
-})
-var keysProcessed = prometheus.NewCounterVec(prometheus.CounterOpts{
-	Name: "bad_keys_processed",
-	Help: "A counter of blockedKeys rows processed labelled by processing state",
-}, []string{"state"})
-var certsRevoked = prometheus.NewCounter(prometheus.CounterOpts{
-	Name: "bad_keys_certs_revoked",
-	Help: "A counter of certificates associated with rows in blockedKeys that have been revoked",
-})
-
 // revoker is an interface used to reduce the scope of a RA gRPC client
 // to only the single method we need to use, this makes testing significantly
 // simpler
@@ -57,6 +45,9 @@ type badKeyRevoker struct {
 	backoffFactor             float64
 	backoffTicker             int
 	maxExpectedReplicationLag time.Duration
+	keysToProcess             prometheus.Gauge
+	keysProcessed             *prometheus.CounterVec
+	certsRevoked              prometheus.Counter
 }
 
 // uncheckedBlockedKey represents a row in the blockedKeys table
@@ -196,7 +187,7 @@ func (bkr *badKeyRevoker) revokeCerts(certs []unrevokedCertificate) error {
 		if err != nil {
 			return err
 		}
-		certsRevoked.Inc()
+		bkr.certsRevoked.Inc()
 	}
 	return nil
 }
@@ -212,7 +203,7 @@ func (bkr *badKeyRevoker) invoke(ctx context.Context) (bool, error) {
 
 	// Set the gauge to the number of rows to be processed (max:
 	// blockedKeysGaugeLimit).
-	keysToProcess.Set(float64(uncheckedCount))
+	bkr.keysToProcess.Set(float64(uncheckedCount))
 
 	if uncheckedCount >= blockedKeysGaugeLimit {
 		bkr.logger.AuditInfof("found >= %d unchecked blocked keys left to process", uncheckedCount)
@@ -324,21 +315,31 @@ func main() {
 		config.BadKeyRevoker.DebugAddr = *debugAddr
 	}
 
-	scope, logger, oTelShutdown := cmd.StatsAndLogging(config.Syslog, config.OpenTelemetry, config.BadKeyRevoker.DebugAddr)
+	stats, logger, oTelShutdown := cmd.StatsAndLogging(config.Syslog, config.OpenTelemetry, config.BadKeyRevoker.DebugAddr)
 	defer oTelShutdown(context.Background())
 	logger.Info(cmd.VersionString())
 	clk := clock.New()
 
-	scope.MustRegister(keysProcessed)
-	scope.MustRegister(certsRevoked)
-
-	dbMap, err := sa.InitWrappedDb(config.BadKeyRevoker.DB, scope, logger)
+	keysToProcess := promauto.With(stats).NewGauge(prometheus.GaugeOpts{
+		Name: "bad_keys_to_process",
+		Help: fmt.Sprintf("A gauge of blockedKeys rows to process (max: %d)", blockedKeysGaugeLimit),
+	})
+	keysProcessed := promauto.With(stats).NewCounterVec(prometheus.CounterOpts{
+		Name: "bad_keys_processed",
+		Help: "A counter of blockedKeys rows processed labelled by processing state",
+	}, []string{"state"})
+	certsRevoked := promauto.With(stats).NewCounter(prometheus.CounterOpts{
+		Name: "bad_keys_certs_revoked",
+		Help: "A counter of certificates associated with rows in blockedKeys that have been revoked",
+	})
+
+	dbMap, err := sa.InitWrappedDb(config.BadKeyRevoker.DB, stats, logger)
 	cmd.FailOnError(err, "While initializing dbMap")
 
-	tlsConfig, err := config.BadKeyRevoker.TLS.Load(scope)
+	tlsConfig, err := config.BadKeyRevoker.TLS.Load(stats)
 	cmd.FailOnError(err, "TLS config")
 
-	conn, err := bgrpc.ClientSetup(config.BadKeyRevoker.RAService, tlsConfig, scope, clk)
+	conn, err := bgrpc.ClientSetup(config.BadKeyRevoker.RAService, tlsConfig, stats, clk)
 	cmd.FailOnError(err, "Failed to load credentials and create gRPC connection to RA")
 	rac := rapb.NewRegistrationAuthorityClient(conn)
 
@@ -353,6 +354,9 @@ func main() {
 		backoffIntervalBase:       config.BadKeyRevoker.Interval.Duration,
 		backoffFactor:             1.3,
 		maxExpectedReplicationLag: config.BadKeyRevoker.MaxExpectedReplicationLag.Duration,
+		keysToProcess:             keysToProcess,
+		keysProcessed:             keysProcessed,
+		certsRevoked:              certsRevoked,
 	}
 
 	// If `BackoffIntervalMax` was not set via the config, set it to 60

cmd/bad-key-revoker/main_test.go

@@ -276,9 +276,10 @@ func TestRevokeCerts(t *testing.T) {
 	fc := clock.NewFake()
 	mr := &mockRevoker{}
 	bkr := &badKeyRevoker{
-		dbMap:    dbMap,
-		raClient: mr,
-		clk:      fc,
+		dbMap:        dbMap,
+		raClient:     mr,
+		clk:          fc,
+		certsRevoked: prometheus.NewCounter(prometheus.CounterOpts{}),
 	}
 
 	err = bkr.revokeCerts([]unrevokedCertificate{
@@ -305,6 +306,7 @@ func TestCertificateAbsent(t *testing.T) {
 		logger:                    blog.NewMock(),
 		clk:                       fc,
 		maxExpectedReplicationLag: time.Second * 22,
+		keysToProcess:             prometheus.NewGauge(prometheus.GaugeOpts{}),
 	}
 
 	// populate DB with all the test data
@@ -345,6 +347,8 @@ func TestInvoke(t *testing.T) {
 		logger:                    blog.NewMock(),
 		clk:                       fc,
 		maxExpectedReplicationLag: time.Second * 22,
+		keysToProcess:             prometheus.NewGauge(prometheus.GaugeOpts{}),
+		certsRevoked:              prometheus.NewCounter(prometheus.CounterOpts{}),
 	}
 
 	// populate DB with all the test data
@@ -363,7 +367,7 @@ func TestInvoke(t *testing.T) {
 	test.AssertNotError(t, err, "invoke failed")
 	test.AssertEquals(t, noWork, false)
 	test.AssertEquals(t, mr.revoked, 4)
-	test.AssertMetricWithLabelsEquals(t, keysToProcess, prometheus.Labels{}, 1)
+	test.AssertMetricWithLabelsEquals(t, bkr.keysToProcess, prometheus.Labels{}, 1)
 
 	var checked struct {
 		ExtantCertificatesChecked bool
@@ -411,6 +415,8 @@ func TestInvokeRevokerHasNoExtantCerts(t *testing.T) {
 		logger:                    blog.NewMock(),
 		clk:                       fc,
 		maxExpectedReplicationLag: time.Second * 22,
+		keysToProcess:             prometheus.NewGauge(prometheus.GaugeOpts{}),
+		certsRevoked:              prometheus.NewCounter(prometheus.CounterOpts{}),
 	}
 
 	// populate DB with all the test data

cmd/cert-checker/main.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/jmhodges/clock"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
 	zX509 "github.com/zmap/zcrypto/x509"
 	"github.com/zmap/zlint/v3"
 	"github.com/zmap/zlint/v3/lint"
@@ -603,11 +604,10 @@ func main() {
 	saDbMap, err := sa.InitWrappedDb(config.CertChecker.DB, prometheus.DefaultRegisterer, logger)
 	cmd.FailOnError(err, "While initializing dbMap")
 
-	checkerLatency := prometheus.NewHistogram(prometheus.HistogramOpts{
+	checkerLatency := promauto.NewHistogram(prometheus.HistogramOpts{
 		Name: "cert_checker_latency",
 		Help: "Histogram of latencies a cert-checker worker takes to complete a batch",
 	})
-	prometheus.DefaultRegisterer.MustRegister(checkerLatency)
 
 	pa, err := policy.New(config.PA.Identifiers, config.PA.Challenges, logger)
 	cmd.FailOnError(err, "Failed to create PA")

crl/storer/storer.go

@@ -18,6 +18,7 @@ import (
 	smithyhttp "github.com/aws/smithy-go/transport/http"
 	"github.com/jmhodges/clock"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
 	"google.golang.org/grpc"
 	"google.golang.org/protobuf/types/known/emptypb"
 
@@ -62,25 +63,22 @@ func New(
 		issuersByNameID[issuer.NameID()] = issuer
 	}
 
-	uploadCount := prometheus.NewCounterVec(prometheus.CounterOpts{
+	uploadCount := promauto.With(stats).NewCounterVec(prometheus.CounterOpts{
 		Name: "crl_storer_uploads",
 		Help: "A counter of the number of CRLs uploaded by crl-storer",
 	}, []string{"issuer", "result"})
-	stats.MustRegister(uploadCount)
 
-	sizeHistogram := prometheus.NewHistogramVec(prometheus.HistogramOpts{
+	sizeHistogram := promauto.With(stats).NewHistogramVec(prometheus.HistogramOpts{
 		Name:    "crl_storer_sizes",
 		Help:    "A histogram of the sizes (in bytes) of CRLs uploaded by crl-storer",
 		Buckets: []float64{0, 256, 1024, 4096, 16384, 65536},
 	}, []string{"issuer"})
-	stats.MustRegister(sizeHistogram)
 
-	latencyHistogram := prometheus.NewHistogramVec(prometheus.HistogramOpts{
+	latencyHistogram := promauto.With(stats).NewHistogramVec(prometheus.HistogramOpts{
 		Name:    "crl_storer_upload_times",
 		Help:    "A histogram of the time (in seconds) it took crl-storer to upload CRLs",
 		Buckets: []float64{0.01, 0.2, 0.5, 1, 2, 5, 10, 20, 50, 100, 200, 500, 1000, 2000, 5000},
 	}, []string{"issuer"})
-	stats.MustRegister(latencyHistogram)
 
 	return &crlStorer{
 		issuers:          issuersByNameID,

crl/updater/updater.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/jmhodges/clock"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
 	"google.golang.org/protobuf/types/known/timestamppb"
 
 	capb "github.com/letsencrypt/boulder/ca/proto"
@@ -92,18 +93,16 @@ func NewUpdater(
 		maxAttempts = 1
 	}
 
-	tickHistogram := prometheus.NewHistogramVec(prometheus.HistogramOpts{
+	tickHistogram := promauto.With(stats).NewHistogramVec(prometheus.HistogramOpts{
 		Name:    "crl_updater_ticks",
 		Help:    "A histogram of crl-updater tick latencies labeled by issuer and result",
 		Buckets: []float64{0.01, 0.2, 0.5, 1, 2, 5, 10, 20, 50, 100, 200, 500, 1000, 2000, 5000},
 	}, []string{"issuer", "result"})
-	stats.MustRegister(tickHistogram)
 
-	updatedCounter := prometheus.NewCounterVec(prometheus.CounterOpts{
+	updatedCounter := promauto.With(stats).NewCounterVec(prometheus.CounterOpts{
 		Name: "crl_updater_generated",
 		Help: "A counter of CRL generation calls labeled by result",
 	}, []string{"issuer", "result"})
-	stats.MustRegister(updatedCounter)
 
 	return &crlUpdater{
 		issuersByNameID,

ctpolicy/ctpolicy.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
 
 	"github.com/letsencrypt/boulder/core"
 	"github.com/letsencrypt/boulder/ctpolicy/loglist"
@@ -36,23 +37,15 @@ type CTPolicy struct {
 
 // New creates a new CTPolicy struct
 func New(pub pubpb.PublisherClient, sctLogs loglist.List, infoLogs loglist.List, finalLogs loglist.List, stagger time.Duration, log blog.Logger, stats prometheus.Registerer) *CTPolicy {
-	winnerCounter := prometheus.NewCounterVec(
-		prometheus.CounterOpts{
-			Name: "sct_winner",
-			Help: "Counter of logs which are selected for sct submission, by log URL and result (succeeded or failed).",
-		},
-		[]string{"url", "result"},
-	)
-	stats.MustRegister(winnerCounter)
+	winnerCounter := promauto.With(stats).NewCounterVec(prometheus.CounterOpts{
+		Name: "sct_winner",
+		Help: "Counter of logs which are selected for sct submission, by log URL and result (succeeded or failed).",
+	}, []string{"url", "result"})
 
-	shardExpiryGauge := prometheus.NewGaugeVec(
-		prometheus.GaugeOpts{
-			Name: "ct_shard_expiration_seconds",
-			Help: "CT shard end_exclusive field expressed as Unix epoch time, by operator and logID.",
-		},
-		[]string{"operator", "logID"},
-	)
-	stats.MustRegister(shardExpiryGauge)
+	shardExpiryGauge := promauto.With(stats).NewGaugeVec(prometheus.GaugeOpts{
+		Name: "ct_shard_expiration_seconds",
+		Help: "CT shard end_exclusive field expressed as Unix epoch time, by operator and logID.",
+	}, []string{"operator", "logID"})
 
 	for _, log := range sctLogs {
 		if log.EndExclusive.IsZero() {

email/cache.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/golang/groupcache/lru"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
 )
 
 type EmailCache struct {
@@ -16,10 +17,9 @@ type EmailCache struct {
 }
 
 func NewHashedEmailCache(maxEntries int, stats prometheus.Registerer) *EmailCache {
-	requests := prometheus.NewCounterVec(prometheus.CounterOpts{
+	requests := promauto.With(stats).NewCounterVec(prometheus.CounterOpts{
 		Name: "email_cache_requests",
 	}, []string{"status"})
-	stats.MustRegister(requests)
 
 	return &EmailCache{
 		cache:    lru.New(maxEntries),