Resolve comments for metrics package

fixes: #60
Signed-off-by: Jing Liu <[email protected]>

Author: 7ing

manager/manager.go

@@ -34,6 +34,7 @@ import (
 	cminformers "github.com/cert-manager/cert-manager/pkg/client/informers/externalversions"
 	cmlisters "github.com/cert-manager/cert-manager/pkg/client/listers/certmanager/v1"
 	"github.com/go-logr/logr"
+	"github.com/prometheus/client_golang/prometheus"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -131,7 +132,7 @@ func NewManager(opts Options) (*Manager, error) {
 		return nil, errors.New("log must be set")
 	}
 	if opts.Metrics == nil {
-		opts.Metrics = metrics.New(opts.Log)
+		opts.Metrics = metrics.New(opts.Log, prometheus.NewRegistry())
 	}
 	if opts.MetadataReader == nil {
 		return nil, errors.New("MetadataReader must be set")

metrics/certificaterequest_test.go

@@ -22,6 +22,7 @@ import (
 	"time"
 
 	"github.com/go-logr/logr/testr"
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/testutil"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -188,7 +189,7 @@ func TestCertificateRequestMetrics(t *testing.T) {
 	for n, test := range tests {
 		t.Run(n, func(t *testing.T) {
 			testLog := testr.New(t)
-			m := New(&testLog)
+			m := New(&testLog, prometheus.NewRegistry())
 			m.UpdateCertificateRequest(test.cr, test.notAfter, test.renewBefore)
 
 			if err := testutil.CollectAndCompare(m.certificateRequestExpiryTimeSeconds,
@@ -217,7 +218,7 @@ func TestCertificateRequestMetrics(t *testing.T) {
 
 func TestCertificateRequestCache(t *testing.T) {
 	testLog := testr.New(t)
-	m := New(&testLog)
+	m := New(&testLog, prometheus.NewRegistry())
 
 	// private key to be used to generate X509 certificate
 	privKey := testcrypto.MustCreatePEMPrivateKey(t)

metrics/metrics.go

@@ -17,23 +17,17 @@ limitations under the License.
 package metrics
 
 import (
-	"net"
 	"net/http"
-	"time"
 
 	"github.com/go-logr/logr"
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prometheus/client_golang/prometheus/collectors"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 )
 
 const (
 	// Namespace is the namespace for csi-lib metric names
-	namespace                             = "certmanager"
-	subsystem                             = "csi"
-	prometheusMetricsServerReadTimeout    = 8 * time.Second
-	prometheusMetricsServerWriteTimeout   = 8 * time.Second
-	prometheusMetricsServerMaxHeaderBytes = 1 << 20 // 1 MiB
+	namespace = "certmanager"
+	subsystem = "csi"
 )
 
 // Metrics is designed to be a shared object for updating the metrics exposed by csi-lib
@@ -51,7 +45,7 @@ type Metrics struct {
 }
 
 // New creates a Metrics struct and populates it with prometheus metric types.
-func New(logger *logr.Logger) *Metrics {
+func New(logger *logr.Logger, registry *prometheus.Registry) *Metrics {
 	var (
 		certificateRequestExpiryTimeSeconds = prometheus.NewGaugeVec(
 			prometheus.GaugeOpts{
@@ -124,12 +118,6 @@ func New(logger *logr.Logger) *Metrics {
 		)
 	)
 
-	// Create Registry and register the recommended collectors
-	registry := prometheus.NewRegistry()
-	registry.MustRegister(
-		collectors.NewProcessCollector(collectors.ProcessCollectorOpts{}),
-		collectors.NewGoCollector(),
-	)
 	// Create server and register Prometheus metrics handler
 	m := &Metrics{
 		log:      logger.WithName("metrics"),
@@ -144,11 +132,6 @@ func New(logger *logr.Logger) *Metrics {
 		managedCertificateCount:              managedCertificateCount,
 	}
 
-	return m
-}
-
-// NewServer registers Prometheus metrics and returns a new Prometheus metrics HTTP server.
-func (m *Metrics) NewServer(ln net.Listener) *http.Server {
 	m.registry.MustRegister(m.certificateRequestExpiryTimeSeconds)
 	m.registry.MustRegister(m.certificateRequestRenewalTimeSeconds)
 	m.registry.MustRegister(m.certificateRequestReadyStatus)
@@ -157,18 +140,15 @@ func (m *Metrics) NewServer(ln net.Listener) *http.Server {
 	m.registry.MustRegister(m.managedVolumeCount)
 	m.registry.MustRegister(m.managedCertificateCount)
 
+	return m
+}
+
+// DefaultHandler returns a default prometheus metrics HTTP handler
+func (m *Metrics) DefaultHandler() http.Handler {
 	mux := http.NewServeMux()
 	mux.Handle("/metrics", promhttp.HandlerFor(m.registry, promhttp.HandlerOpts{}))
 
-	server := &http.Server{
-		Addr:           ln.Addr().String(),
-		ReadTimeout:    prometheusMetricsServerReadTimeout,
-		WriteTimeout:   prometheusMetricsServerWriteTimeout,
-		MaxHeaderBytes: prometheusMetricsServerMaxHeaderBytes,
-		Handler:        mux,
-	}
-
-	return server
+	return mux
 }
 
 // IncrementIssueCallCount will increase the issue call counter for the driver.

test/integration/metrics_test.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"github.com/go-logr/logr/testr"
+	"github.com/prometheus/client_golang/prometheus"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	fakeclock "k8s.io/utils/clock/testing"
@@ -80,12 +81,19 @@ func TestMetricsServer(t *testing.T) {
 	testNamespace := "test-ns"
 
 	// Build metrics handler, and start metrics server with a random available port
-	metricsHandler := metrics.New(&testLog)
+	metricsHandler := metrics.New(&testLog, prometheus.NewRegistry())
 	metricsLn, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
 		t.Fatal(err)
 	}
-	metricsServer := metricsHandler.NewServer(metricsLn)
+	metricsServer := &http.Server{
+		Addr:           metricsLn.Addr().String(),
+		ReadTimeout:    8 * time.Second,
+		WriteTimeout:   8 * time.Second,
+		MaxHeaderBytes: 1 << 20, // 1 MiB
+		Handler:        metricsHandler.DefaultHandler(),
+	}
+
 	errCh := make(chan error)
 	go func() {
 		defer close(errCh)