From 7327c42db974b788662a859a7816c604d4547bf2 Mon Sep 17 00:00:00 2001 From: Indra Gunawan Date: Mon, 17 Feb 2025 16:41:37 +0800 Subject: [PATCH] revert metrics initialization --- caddy/caddy_test.go | 5 +- frankenphp.go | 3 +- metrics.go | 141 +++++++++++++++++++++++++------------------- metrics_test.go | 10 ++-- 4 files changed, 89 insertions(+), 70 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index a8bcc8a87..757930a21 100644 --- a/caddy/caddy_test.go +++ b/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" diff --git a/frankenphp.go b/frankenphp.go index 1a8007a63..653898f33 100644 --- a/frankenphp.go +++ b/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 } diff --git a/metrics.go b/metrics.go index 1d503aa90..e5fc28906 100644 --- a/metrics.go +++ b/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", diff --git a/metrics_test.go b/metrics_test.go index 6e3678198..284ce54e4 100644 --- a/metrics_test.go +++ b/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 {