revert metrics initialization

caddy/caddy_test.go

@@ -10,13 +10,12 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/caddyserver/caddy/v2"
+	"github.com/caddyserver/caddy/v2/caddytest"
 	"github.com/dunglas/frankenphp"
 	"github.com/dunglas/frankenphp/internal/fastabs"
 	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/stretchr/testify/require"
-
-	"github.com/caddyserver/caddy/v2"
-	"github.com/caddyserver/caddy/v2/caddytest"
 )
 
 var testPort = "9080"

frankenphp.go

@@ -247,14 +247,13 @@ var MaxThreads int
 func calculateMaxThreads(opt *opt) (int, int, error) {
 	maxProcs := runtime.GOMAXPROCS(0) * 2
 
-	initWorkerMetrics(metrics)
-
 	var numWorkers int
 	for i, w := range opt.workers {
 		if w.num <= 0 {
 			// https://github.com/dunglas/frankenphp/issues/126
 			opt.workers[i].num = maxProcs
 		}
+		metrics.TotalWorkers(w.name, w.num)
 
 		numWorkers += opt.workers[i].num
 	}

metrics.go

@@ -5,7 +5,6 @@ import (
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prometheus/client_golang/prometheus/promauto"
 )
 
 const (
@@ -23,6 +22,8 @@ type Metrics interface {
 	ReadyWorker(name string)
 	// StopWorker collects stopped workers
 	StopWorker(name string, reason StopReason)
+	// TotalWorkers collects expected workers
+	TotalWorkers(name string, num int)
 	// TotalThreads collects total threads
 	TotalThreads(num int)
 	// StartRequest collects started requests
@@ -47,6 +48,9 @@ func (n nullMetrics) ReadyWorker(string) {
 func (n nullMetrics) StopWorker(string, StopReason) {
 }
 
+func (n nullMetrics) TotalWorkers(string, int) {
+}
+
 func (n nullMetrics) TotalThreads(int) {
 }
 
@@ -79,101 +83,104 @@ type PrometheusMetrics struct {
 	mu                 sync.Mutex
 }
 
-func initWorkerMetrics(metrics Metrics) {
-	prometheusMetrics, ok := metrics.(*PrometheusMetrics)
-	if !ok {
+func (m *PrometheusMetrics) getLabels(name string) prometheus.Labels {
+	return prometheus.Labels{"worker": name}
+}
+
+func (m *PrometheusMetrics) StartWorker(name string) {
+	m.busyThreads.Inc()
+
+	// tests do not register workers before starting them
+	if m.totalWorkers == nil {
+		return
+	}
+
+	m.totalWorkers.With(m.getLabels(name)).Inc()
+}
+
+func (m *PrometheusMetrics) ReadyWorker(name string) {
+	if m.totalWorkers == nil {
+		return
+	}
+
+	m.readyWorkers.With(m.getLabels(name)).Inc()
+}
+
+func (m *PrometheusMetrics) StopWorker(name string, reason StopReason) {
+	m.busyThreads.Dec()
+
+	// tests do not register workers before starting them
+	if m.totalWorkers == nil {
 		return
 	}
+	m.totalWorkers.With(m.getLabels(name)).Dec()
+	m.readyWorkers.With(m.getLabels(name)).Dec()
+
+	if reason == StopReasonCrash {
+		m.workerCrashes.With(m.getLabels(name)).Inc()
+	} else if reason == StopReasonRestart {
+		m.workerRestarts.With(m.getLabels(name)).Inc()
+	}
+}
 
-	prometheusMetrics.mu.Lock()
-	defer prometheusMetrics.mu.Unlock()
+func (m *PrometheusMetrics) TotalWorkers(string, int) {
+
+	m.mu.Lock()
+	defer m.mu.Unlock()
 
 	const ns, sub = "frankenphp", "worker"
 	basicLabels := []string{"worker"}
 
-	prometheusMetrics.totalWorkers = promauto.With(prometheusMetrics.registry).NewGaugeVec(prometheus.GaugeOpts{
+	m.totalWorkers = prometheus.NewGaugeVec(prometheus.GaugeOpts{
 		Namespace: ns,
 		Name:      "total_workers",
 		Help:      "Total number of PHP workers for this worker",
 	}, basicLabels)
+	m.registry.MustRegister(m.totalWorkers)
 
-	prometheusMetrics.readyWorkers = promauto.With(prometheusMetrics.registry).NewGaugeVec(prometheus.GaugeOpts{
+	m.readyWorkers = prometheus.NewGaugeVec(prometheus.GaugeOpts{
 		Namespace: ns,
 		Name:      "ready_workers",
 		Help:      "Running workers that have successfully called frankenphp_handle_request at least once",
 	}, basicLabels)
+	m.registry.MustRegister(m.readyWorkers)
 
-	prometheusMetrics.busyWorkers = promauto.With(prometheusMetrics.registry).NewGaugeVec(prometheus.GaugeOpts{
+	m.busyWorkers = prometheus.NewGaugeVec(prometheus.GaugeOpts{
 		Namespace: ns,
 		Name:      "busy_workers",
 		Help:      "Number of busy PHP workers for this worker",
 	}, basicLabels)
+	m.registry.MustRegister(m.busyWorkers)
 
-	prometheusMetrics.workerCrashes = promauto.With(prometheusMetrics.registry).NewCounterVec(prometheus.CounterOpts{
+	m.workerCrashes = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Namespace: ns,
 		Subsystem: sub,
 		Name:      "crashes",
 		Help:      "Number of PHP worker crashes for this worker",
 	}, basicLabels)
+	m.registry.MustRegister(m.workerCrashes)
 
-	prometheusMetrics.workerRestarts = promauto.With(prometheusMetrics.registry).NewCounterVec(prometheus.CounterOpts{
+	m.workerRestarts = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Namespace: ns,
 		Subsystem: sub,
 		Name:      "restarts",
 		Help:      "Number of PHP worker restarts for this worker",
 	}, basicLabels)
+	m.registry.MustRegister(m.workerRestarts)
 
-	prometheusMetrics.workerRequestTime = promauto.With(prometheusMetrics.registry).NewCounterVec(prometheus.CounterOpts{
+	m.workerRequestTime = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Namespace: ns,
 		Subsystem: sub,
 		Name:      "request_time",
 	}, basicLabels)
+	m.registry.MustRegister(m.workerRequestTime)
 
-	prometheusMetrics.workerRequestCount = promauto.With(prometheusMetrics.registry).NewCounterVec(prometheus.CounterOpts{
+	m.workerRequestCount = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Namespace: ns,
 		Subsystem: sub,
 		Name:      "request_count",
 	}, basicLabels)
-}
-
-func (m *PrometheusMetrics) getLabels(name string) prometheus.Labels {
-	return prometheus.Labels{"worker": name}
-}
-
-func (m *PrometheusMetrics) StartWorker(name string) {
-	m.busyThreads.Inc()
-
-	// tests do not register workers before starting them
-	if m.totalWorkers == nil {
-		return
-	}
-
-	m.totalWorkers.With(m.getLabels(name)).Inc()
-}
-
-func (m *PrometheusMetrics) ReadyWorker(name string) {
-	if m.totalWorkers == nil {
-		return
-	}
-
-	m.readyWorkers.With(m.getLabels(name)).Inc()
-}
-
-func (m *PrometheusMetrics) StopWorker(name string, reason StopReason) {
-	m.busyThreads.Dec()
-
-	// tests do not register workers before starting them
-	if m.totalWorkers == nil {
-		return
-	}
-	m.totalWorkers.With(m.getLabels(name)).Dec()
-	m.readyWorkers.With(m.getLabels(name)).Dec()
-
-	if reason == StopReasonCrash {
-		m.workerCrashes.With(m.getLabels(name)).Inc()
-	} else if reason == StopReasonRestart {
-		m.workerRestarts.With(m.getLabels(name)).Inc()
-	}
+	m.registry.MustRegister(m.workerRequestCount)
 }
 
 func (m *PrometheusMetrics) TotalThreads(num int) {
@@ -209,13 +216,27 @@ func (m *PrometheusMetrics) Shutdown() {
 	m.registry.Unregister(m.totalThreads)
 	m.registry.Unregister(m.busyThreads)
 
-	m.registry.Unregister(m.totalWorkers)
-	m.registry.Unregister(m.busyWorkers)
-	m.registry.Unregister(m.workerRequestTime)
-	m.registry.Unregister(m.workerRequestCount)
-	m.registry.Unregister(m.workerCrashes)
-	m.registry.Unregister(m.workerRestarts)
-	m.registry.Unregister(m.readyWorkers)
+	if m.totalWorkers != nil {
+		m.registry.Unregister(m.totalWorkers)
+	}
+	if m.busyWorkers != nil {
+		m.registry.Unregister(m.busyWorkers)
+	}
+	if m.workerRequestTime != nil {
+		m.registry.Unregister(m.workerRequestTime)
+	}
+	if m.workerRequestCount != nil {
+		m.registry.Unregister(m.workerRequestCount)
+	}
+	if m.workerCrashes != nil {
+		m.registry.Unregister(m.workerCrashes)
+	}
+	if m.workerRestarts != nil {
+		m.registry.Unregister(m.workerRestarts)
+	}
+	if m.readyWorkers != nil {
+		m.registry.Unregister(m.readyWorkers)
+	}
 
 	m.totalThreads = prometheus.NewCounter(prometheus.CounterOpts{
 		Name: "frankenphp_total_threads",

metrics_test.go

@@ -20,7 +20,7 @@ func createPrometheusMetrics() *PrometheusMetrics {
 	}
 }
 
-func TestPrometheusMetrics_Initialization(t *testing.T) {
+func TestPrometheusMetrics_TotalWorkers(t *testing.T) {
 	m := createPrometheusMetrics()
 
 	require.Nil(t, m.totalWorkers)
@@ -31,7 +31,7 @@ func TestPrometheusMetrics_Initialization(t *testing.T) {
 	require.Nil(t, m.workerRequestTime)
 	require.Nil(t, m.workerRequestCount)
 
-	initWorkerMetrics(m)
+	m.TotalWorkers("test_worker", 2)
 
 	require.NotNil(t, m.totalWorkers)
 	require.NotNil(t, m.busyWorkers)
@@ -44,7 +44,7 @@ func TestPrometheusMetrics_Initialization(t *testing.T) {
 
 func TestPrometheusMetrics_StopWorkerRequest(t *testing.T) {
 	m := createPrometheusMetrics()
-	initWorkerMetrics(m)
+	m.TotalWorkers("test_worker", 2)
 	m.StopWorkerRequest("test_worker", 2*time.Second)
 
 	inputs := []struct {
@@ -100,7 +100,7 @@ func TestPrometheusMetrics_StopWorkerRequest(t *testing.T) {
 
 func TestPrometheusMetrics_StartWorkerRequest(t *testing.T) {
 	m := createPrometheusMetrics()
-	initWorkerMetrics(m)
+	m.TotalWorkers("test_worker", 2)
 	m.StartWorkerRequest("test_worker")
 
 	inputs := []struct {
@@ -134,7 +134,7 @@ func TestPrometheusMetrics_StartWorkerRequest(t *testing.T) {
 
 func TestPrometheusMetrics_TestStopReasonCrash(t *testing.T) {
 	m := createPrometheusMetrics()
-	initWorkerMetrics(m)
+	m.TotalWorkers("test_worker", 2)
 	m.StopWorker("test_worker", StopReasonCrash)
 
 	inputs := []struct {