From db899774c37c6bcc17665d1cbb17009dd29c2b07 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sun, 6 Sep 2026 14:59:08 +0200 Subject: [PATCH 1/6] fix: log opcache restarts opcache schedules a restart of its shared memory on exhaustion or hash overflow, then carries it out at the next request init on any thread. Under ZTS it does that while other threads are still running, because the deferral gate (accel_is_inactive()) probes for a conflicting lock with fcntl F_GETLK, and POSIX fcntl locks belong to the process, so the probe never sees the threads of the process holding them. Workers are the worst case: they hold shared memory references for their whole life rather than for a single request. The result is a crash or a slowdown with nothing in the logs pointing at opcache. opcache does report it, but only at opcache.log_verbosity_level=4 and in its own log. zend_accel_schedule_restart_hook is the only in-process signal for this, so it is used to emit one warning naming the restart reason and the two settings that make restarts less likely. Nothing else is done with it: the threads are not rebooted, which is what #2564 removed. --- frankenphp.c | 13 +++++++++++++ frankenphp.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/frankenphp.c b/frankenphp.c index d5ee9f6add..de2b5bb1e6 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -1014,6 +1014,14 @@ PHP_FUNCTION(frankenphp_log) { } } +/* Guarded like its only assignment in php_main(), so builds without the hook + * do not trip -Werror=unused-function. */ +#if defined(ZTS) && PHP_VERSION_ID >= 80400 +static void frankenphp_opcache_restart_hook(int reason) { + go_log_opcache_restart(reason); +} +#endif + /* {{{ thread-safe opcache reset */ PHP_FUNCTION(frankenphp_opcache_reset) { go_schedule_opcache_reset(frankenphp_thread_index()); @@ -1791,6 +1799,11 @@ static void *php_main(void *arg) { frankenphp_sapi_module.startup(&frankenphp_sapi_module); +#if defined(ZTS) && PHP_VERSION_ID >= 80400 + /* Report the opcache restarts that opcache schedules on its own */ + zend_accel_schedule_restart_hook = frankenphp_opcache_restart_hook; +#endif + /* check if a default filter is set in php.ini and only filter if * it is, this is deprecated and will be removed in PHP 9 */ char *default_filter; diff --git a/frankenphp.go b/frankenphp.go index eabee746e8..2dd58e2175 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -845,6 +845,35 @@ func go_schedule_opcache_reset(threadIndex C.uintptr_t) { } } +// Restart reasons opcache reports to the hook, in the order of +// zend_accel_restart_reason (ext/opcache/ZendAccelerator.h). +var opcacheRestartReasons = [...]string{"out of memory", "hash overflow", "user"} + +// go_log_opcache_restart reports the restarts opcache schedules on its own. +// Under ZTS they rewind shared memory that running threads still point into, +// which surfaces as an unexplained crash or slowdown, so make the event +// visible. The line is written inline even though opcache can be holding its +// shared memory lock: that costs far less than the restart it precedes, and a +// line deferred to a goroutine would be lost when the restart takes the +// process down. +// +//export go_log_opcache_restart +func go_log_opcache_restart(reason C.int) { + if !globalLogger.Enabled(globalCtx, slog.LevelWarn) { + return + } + + reasonText := "unknown" + if i := int(reason); i >= 0 && i < len(opcacheRestartReasons) { + reasonText = opcacheRestartReasons[i] + } + + globalLogger.LogAttrs(globalCtx, slog.LevelWarn, + "opcache restart scheduled, running PHP threads may hold stale references to its shared memory: raise opcache.memory_consumption and opcache.max_accelerated_files to make restarts less likely", + slog.String("reason", reasonText), + ) +} + func convertArgs(args []string) (C.int, []*C.char) { argc := C.int(len(args)) argv := make([]*C.char, argc) From 09e64b47d9cb5e983fd39b112063fbdea114fd13 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 7 Sep 2026 11:01:29 +0200 Subject: [PATCH 2/6] feat: count opcache restarts in frankenphp_opcache_restarts The log line alone cannot be alerted on. Expose the same event as a counter labelled by reason, pre-populated at zero for the known reasons so a rate or an alert works from the first restart on. --- docs/metrics.md | 1 + frankenphp.c | 2 +- frankenphp.go | 22 ++++++++-------------- metrics.go | 25 +++++++++++++++++++++++++ metrics_test.go | 27 +++++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 15 deletions(-) diff --git a/docs/metrics.md b/docs/metrics.md index 932707265a..7a6a0981cf 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -23,6 +23,7 @@ When [Caddy metrics](https://caddyserver.com/docs/metrics) are enabled, FrankenP - `frankenphp_worker_crashes{worker="[worker_name]"}`: The number of times a worker has unexpectedly terminated. - `frankenphp_worker_restarts{worker="[worker_name]"}`: The number of times a worker has been deliberately restarted. - `frankenphp_worker_queue_depth{worker="[worker_name]"}`: The number of queued requests. +- `frankenphp_opcache_restarts{reason="[reason]"}`: The number of times opcache restarted its shared memory on its own, by reason (`out of memory`, `hash overflow`, `user`). Each restart is also logged. Under ZTS, running PHP threads may hold stale references to the rewound memory, so raise `opcache.memory_consumption` and `opcache.max_accelerated_files` when this counter grows. For worker metrics, the `[worker_name]` placeholder is replaced by the worker name in the Caddyfile, otherwise the absolute path of the worker file will be used. diff --git a/frankenphp.c b/frankenphp.c index de2b5bb1e6..0b0ac2a50a 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -1018,7 +1018,7 @@ PHP_FUNCTION(frankenphp_log) { * do not trip -Werror=unused-function. */ #if defined(ZTS) && PHP_VERSION_ID >= 80400 static void frankenphp_opcache_restart_hook(int reason) { - go_log_opcache_restart(reason); + go_opcache_restart_scheduled(reason); } #endif diff --git a/frankenphp.go b/frankenphp.go index 2dd58e2175..bca8af4b0b 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -849,25 +849,19 @@ func go_schedule_opcache_reset(threadIndex C.uintptr_t) { // zend_accel_restart_reason (ext/opcache/ZendAccelerator.h). var opcacheRestartReasons = [...]string{"out of memory", "hash overflow", "user"} -// go_log_opcache_restart reports the restarts opcache schedules on its own. -// Under ZTS they rewind shared memory that running threads still point into, -// which surfaces as an unexplained crash or slowdown, so make the event -// visible. The line is written inline even though opcache can be holding its -// shared memory lock: that costs far less than the restart it precedes, and a -// line deferred to a goroutine would be lost when the restart takes the -// process down. -// -//export go_log_opcache_restart -func go_log_opcache_restart(reason C.int) { - if !globalLogger.Enabled(globalCtx, slog.LevelWarn) { - return - } - +//export go_opcache_restart_scheduled +func go_opcache_restart_scheduled(reason C.int) { reasonText := "unknown" if i := int(reason); i >= 0 && i < len(opcacheRestartReasons) { reasonText = opcacheRestartReasons[i] } + metrics.OpcacheRestart(reasonText) + + if !globalLogger.Enabled(globalCtx, slog.LevelWarn) { + return + } + globalLogger.LogAttrs(globalCtx, slog.LevelWarn, "opcache restart scheduled, running PHP threads may hold stale references to its shared memory: raise opcache.memory_consumption and opcache.max_accelerated_files to make restarts less likely", slog.String("reason", reasonText), diff --git a/metrics.go b/metrics.go index fc25816506..6c1736588c 100644 --- a/metrics.go +++ b/metrics.go @@ -40,6 +40,8 @@ type Metrics interface { DequeuedWorkerRequest(name string) QueuedRequest() DequeuedRequest() + // OpcacheRestart collects the restarts of opcache's shared memory, by reason + OpcacheRestart(reason string) } type nullMetrics struct{} @@ -81,6 +83,8 @@ func (n nullMetrics) DequeuedWorkerRequest(string) {} func (n nullMetrics) QueuedRequest() {} func (n nullMetrics) DequeuedRequest() {} +func (n nullMetrics) OpcacheRestart(string) {} + type PrometheusMetrics struct { registry prometheus.Registerer totalThreads prometheus.Gauge @@ -94,6 +98,7 @@ type PrometheusMetrics struct { workerRequestCount *prometheus.CounterVec workerQueueDepth *prometheus.GaugeVec queueDepth prometheus.Gauge + opcacheRestarts *prometheus.CounterVec mu sync.RWMutex } @@ -317,6 +322,13 @@ func (m *PrometheusMetrics) DequeuedRequest() { m.queueDepth.Dec() } +func (m *PrometheusMetrics) OpcacheRestart(reason string) { + m.mu.RLock() + defer m.mu.RUnlock() + + m.opcacheRestarts.WithLabelValues(reason).Inc() +} + func (m *PrometheusMetrics) Shutdown() { m.mu.Lock() defer m.mu.Unlock() @@ -324,6 +336,7 @@ func (m *PrometheusMetrics) Shutdown() { m.registry.Unregister(m.totalThreads) m.registry.Unregister(m.busyThreads) m.registry.Unregister(m.queueDepth) + m.registry.Unregister(m.opcacheRestarts) if m.totalWorkers != nil { m.registry.Unregister(m.totalWorkers) @@ -377,6 +390,10 @@ func NewPrometheusMetrics(registry prometheus.Registerer) *PrometheusMetrics { Name: "frankenphp_queue_depth", Help: "Number of regular queued requests", }), + opcacheRestarts: prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "frankenphp_opcache_restarts", + Help: "Number of restarts of opcache's shared memory, by reason", + }, []string{"reason"}), totalWorkers: nil, busyWorkers: nil, workerRequestTime: nil, @@ -393,5 +410,13 @@ func NewPrometheusMetrics(registry prometheus.Registerer) *PrometheusMetrics { m.mustRegister(m.queueDepth) + m.mustRegister(m.opcacheRestarts) + + // expose the series at zero so a rate or an alert on them works from the + // first restart on, instead of missing it for lack of a previous sample + for _, reason := range opcacheRestartReasons { + m.opcacheRestarts.WithLabelValues(reason) + } + return m } diff --git a/metrics_test.go b/metrics_test.go index 846a926569..99bbcfac4f 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -212,3 +212,30 @@ func TestPrometheusMetrics_TestStopReasonCrash(t *testing.T) { } } + +func TestPrometheusMetrics_OpcacheRestart(t *testing.T) { + m := NewPrometheusMetrics(prometheus.NewRegistry()) + m.OpcacheRestart("hash overflow") + m.OpcacheRestart("hash overflow") + m.OpcacheRestart("out of memory") + + // known reasons are exposed from the start, unknown ones only once seen + require.NoError(t, testutil.CollectAndCompare(m.opcacheRestarts, strings.NewReader(` + # HELP frankenphp_opcache_restarts Number of restarts of opcache's shared memory, by reason + # TYPE frankenphp_opcache_restarts counter + frankenphp_opcache_restarts{reason="hash overflow"} 2 + frankenphp_opcache_restarts{reason="out of memory"} 1 + frankenphp_opcache_restarts{reason="user"} 0 + `))) + + m.OpcacheRestart("unknown") + + require.NoError(t, testutil.CollectAndCompare(m.opcacheRestarts, strings.NewReader(` + # HELP frankenphp_opcache_restarts Number of restarts of opcache's shared memory, by reason + # TYPE frankenphp_opcache_restarts counter + frankenphp_opcache_restarts{reason="hash overflow"} 2 + frankenphp_opcache_restarts{reason="out of memory"} 1 + frankenphp_opcache_restarts{reason="unknown"} 1 + frankenphp_opcache_restarts{reason="user"} 0 + `))) +} From 81a9bfb30c71ef5e0c8db91bb57b9b03e3cc870e Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 8 Sep 2026 10:15:37 +0200 Subject: [PATCH 3/6] docs: mark frankenphp_opcache_restarts as experimental Should always be zero, to be removed once opcache handles restarts safely under ZTS. --- docs/metrics.md | 2 +- metrics.go | 3 ++- metrics_test.go | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/metrics.md b/docs/metrics.md index 7a6a0981cf..c4bffb356e 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -23,7 +23,7 @@ When [Caddy metrics](https://caddyserver.com/docs/metrics) are enabled, FrankenP - `frankenphp_worker_crashes{worker="[worker_name]"}`: The number of times a worker has unexpectedly terminated. - `frankenphp_worker_restarts{worker="[worker_name]"}`: The number of times a worker has been deliberately restarted. - `frankenphp_worker_queue_depth{worker="[worker_name]"}`: The number of queued requests. -- `frankenphp_opcache_restarts{reason="[reason]"}`: The number of times opcache restarted its shared memory on its own, by reason (`out of memory`, `hash overflow`, `user`). Each restart is also logged. Under ZTS, running PHP threads may hold stale references to the rewound memory, so raise `opcache.memory_consumption` and `opcache.max_accelerated_files` when this counter grows. +- `frankenphp_opcache_restarts{reason="[reason]"}`: (experimental) The number of times opcache restarted its shared memory on its own, by reason (`out of memory`, `hash overflow`, `user`). This counter should always be zero: a restart rewinds memory that running PHP threads may still reference, which can crash the process. A non-zero value means opcache is undersized for the application, raise `opcache.memory_consumption` and `opcache.max_accelerated_files`. Each restart is also logged. This metric will be removed once opcache handles restarts safely under ZTS. For worker metrics, the `[worker_name]` placeholder is replaced by the worker name in the Caddyfile, otherwise the absolute path of the worker file will be used. diff --git a/metrics.go b/metrics.go index 6c1736588c..9ee8ff2cca 100644 --- a/metrics.go +++ b/metrics.go @@ -390,9 +390,10 @@ func NewPrometheusMetrics(registry prometheus.Registerer) *PrometheusMetrics { Name: "frankenphp_queue_depth", Help: "Number of regular queued requests", }), + // experimental: to be removed once opcache handles restarts safely under ZTS opcacheRestarts: prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "frankenphp_opcache_restarts", - Help: "Number of restarts of opcache's shared memory, by reason", + Help: "Number of restarts of opcache's shared memory, by reason (experimental, should stay at zero)", }, []string{"reason"}), totalWorkers: nil, busyWorkers: nil, diff --git a/metrics_test.go b/metrics_test.go index 99bbcfac4f..132a0db0db 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -221,7 +221,7 @@ func TestPrometheusMetrics_OpcacheRestart(t *testing.T) { // known reasons are exposed from the start, unknown ones only once seen require.NoError(t, testutil.CollectAndCompare(m.opcacheRestarts, strings.NewReader(` - # HELP frankenphp_opcache_restarts Number of restarts of opcache's shared memory, by reason + # HELP frankenphp_opcache_restarts Number of restarts of opcache's shared memory, by reason (experimental, should stay at zero) # TYPE frankenphp_opcache_restarts counter frankenphp_opcache_restarts{reason="hash overflow"} 2 frankenphp_opcache_restarts{reason="out of memory"} 1 @@ -231,7 +231,7 @@ func TestPrometheusMetrics_OpcacheRestart(t *testing.T) { m.OpcacheRestart("unknown") require.NoError(t, testutil.CollectAndCompare(m.opcacheRestarts, strings.NewReader(` - # HELP frankenphp_opcache_restarts Number of restarts of opcache's shared memory, by reason + # HELP frankenphp_opcache_restarts Number of restarts of opcache's shared memory, by reason (experimental, should stay at zero) # TYPE frankenphp_opcache_restarts counter frankenphp_opcache_restarts{reason="hash overflow"} 2 frankenphp_opcache_restarts{reason="out of memory"} 1 From a5e9f515445835c4f29b5e0b3050a3327e62bcd2 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 21 Sep 2026 19:10:12 +0200 Subject: [PATCH 4/6] fix: name what gates an opcache restart, and expose the counter only where the hook exists --- docs/metrics.md | 4 +++- frankenphp.c | 6 ++---- frankenphp.go | 22 ++++++++++++++----- frankenphp.h | 6 ++++++ metrics.go | 25 +++++++++++++++------- metrics_test.go | 57 +++++++++++++++++++++++++++++++++++++------------ 6 files changed, 88 insertions(+), 32 deletions(-) diff --git a/docs/metrics.md b/docs/metrics.md index c4bffb356e..b87d2231f4 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -23,10 +23,12 @@ When [Caddy metrics](https://caddyserver.com/docs/metrics) are enabled, FrankenP - `frankenphp_worker_crashes{worker="[worker_name]"}`: The number of times a worker has unexpectedly terminated. - `frankenphp_worker_restarts{worker="[worker_name]"}`: The number of times a worker has been deliberately restarted. - `frankenphp_worker_queue_depth{worker="[worker_name]"}`: The number of queued requests. -- `frankenphp_opcache_restarts{reason="[reason]"}`: (experimental) The number of times opcache restarted its shared memory on its own, by reason (`out of memory`, `hash overflow`, `user`). This counter should always be zero: a restart rewinds memory that running PHP threads may still reference, which can crash the process. A non-zero value means opcache is undersized for the application, raise `opcache.memory_consumption` and `opcache.max_accelerated_files`. Each restart is also logged. This metric will be removed once opcache handles restarts safely under ZTS. +- `frankenphp_opcache_restarts{reason="[reason]"}`: (experimental) The number of restarts of opcache's shared memory scheduled, by reason (`oom`, `hash`, `manual`). PHP 8.4 and up. For worker metrics, the `[worker_name]` placeholder is replaced by the worker name in the Caddyfile, otherwise the absolute path of the worker file will be used. +opcache schedules a restart when its cache is full and more than `opcache.max_wasted_percentage` of it was wasted by invalidations. The restart runs at the next request start; caching is off until then, and running PHP threads may still reference the old memory, which can crash the process. The counter should stay at zero: raise `opcache.memory_consumption`, `opcache.max_accelerated_files` or `opcache.max_wasted_percentage`. `manual` should never appear, FrankenPHP overrides `opcache_reset()`. Each restart is also logged. This metric will be removed once opcache handles restarts safely under ZTS. + ## Threads State Endpoint FrankenPHP exposes a `/frankenphp/threads` endpoint through the [Caddy admin API](https://caddyserver.com/docs/api). diff --git a/frankenphp.c b/frankenphp.c index 0b0ac2a50a..5f14209820 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -1014,9 +1014,7 @@ PHP_FUNCTION(frankenphp_log) { } } -/* Guarded like its only assignment in php_main(), so builds without the hook - * do not trip -Werror=unused-function. */ -#if defined(ZTS) && PHP_VERSION_ID >= 80400 +#if FRANKENPHP_OPCACHE_RESTART_HOOK static void frankenphp_opcache_restart_hook(int reason) { go_opcache_restart_scheduled(reason); } @@ -1799,7 +1797,7 @@ static void *php_main(void *arg) { frankenphp_sapi_module.startup(&frankenphp_sapi_module); -#if defined(ZTS) && PHP_VERSION_ID >= 80400 +#if FRANKENPHP_OPCACHE_RESTART_HOOK /* Report the opcache restarts that opcache schedules on its own */ zend_accel_schedule_restart_hook = frankenphp_opcache_restart_hook; #endif diff --git a/frankenphp.go b/frankenphp.go index bca8af4b0b..40e678122d 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -845,15 +845,25 @@ func go_schedule_opcache_reset(threadIndex C.uintptr_t) { } } +// opcacheRestartHook tells whether this build reports the restarts opcache +// schedules on its own: PHP 8.4 brought the hook, and only ZTS builds are +// exposed to them +var opcacheRestartHook = C.FRANKENPHP_OPCACHE_RESTART_HOOK != 0 + // Restart reasons opcache reports to the hook, in the order of -// zend_accel_restart_reason (ext/opcache/ZendAccelerator.h). -var opcacheRestartReasons = [...]string{"out of memory", "hash overflow", "user"} +// zend_accel_restart_reason (ext/opcache/ZendAccelerator.h), named like the +// counters of opcache_get_status() +var opcacheRestartReasons = [...]string{"oom", "hash", "manual"} //export go_opcache_restart_scheduled func go_opcache_restart_scheduled(reason C.int) { + opcacheRestartScheduled(int(reason)) +} + +func opcacheRestartScheduled(reason int) { reasonText := "unknown" - if i := int(reason); i >= 0 && i < len(opcacheRestartReasons) { - reasonText = opcacheRestartReasons[i] + if reason >= 0 && reason < len(opcacheRestartReasons) { + reasonText = opcacheRestartReasons[reason] } metrics.OpcacheRestart(reasonText) @@ -862,8 +872,10 @@ func go_opcache_restart_scheduled(reason C.int) { return } + // written synchronously, under opcache's lock: a line deferred to a + // goroutine is lost if the restart crashes the process globalLogger.LogAttrs(globalCtx, slog.LevelWarn, - "opcache restart scheduled, running PHP threads may hold stale references to its shared memory: raise opcache.memory_consumption and opcache.max_accelerated_files to make restarts less likely", + "opcache restart scheduled, caching stops until the next request start carries it out while other threads may still reference the old memory: raise opcache.memory_consumption, opcache.max_accelerated_files or opcache.max_wasted_percentage", slog.String("reason", reasonText), ) } diff --git a/frankenphp.h b/frankenphp.h index 99ac0ab7ec..47cdb3da57 100644 --- a/frankenphp.h +++ b/frankenphp.h @@ -41,6 +41,12 @@ typedef struct { #endif } force_kill_slot; +#if defined(ZTS) && PHP_VERSION_ID >= 80400 +#define FRANKENPHP_OPCACHE_RESTART_HOOK 1 +#else +#define FRANKENPHP_OPCACHE_RESTART_HOOK 0 +#endif + #ifndef FRANKENPHP_VERSION #define FRANKENPHP_VERSION dev #endif diff --git a/metrics.go b/metrics.go index 9ee8ff2cca..9701dc7d23 100644 --- a/metrics.go +++ b/metrics.go @@ -326,7 +326,9 @@ func (m *PrometheusMetrics) OpcacheRestart(reason string) { m.mu.RLock() defer m.mu.RUnlock() - m.opcacheRestarts.WithLabelValues(reason).Inc() + if m.opcacheRestarts != nil { + m.opcacheRestarts.WithLabelValues(reason).Inc() + } } func (m *PrometheusMetrics) Shutdown() { @@ -336,7 +338,10 @@ func (m *PrometheusMetrics) Shutdown() { m.registry.Unregister(m.totalThreads) m.registry.Unregister(m.busyThreads) m.registry.Unregister(m.queueDepth) - m.registry.Unregister(m.opcacheRestarts) + + if m.opcacheRestarts != nil { + m.registry.Unregister(m.opcacheRestarts) + } if m.totalWorkers != nil { m.registry.Unregister(m.totalWorkers) @@ -393,7 +398,7 @@ func NewPrometheusMetrics(registry prometheus.Registerer) *PrometheusMetrics { // experimental: to be removed once opcache handles restarts safely under ZTS opcacheRestarts: prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "frankenphp_opcache_restarts", - Help: "Number of restarts of opcache's shared memory, by reason (experimental, should stay at zero)", + Help: "Number of restarts of opcache's shared memory scheduled, by reason (experimental, should stay at zero)", }, []string{"reason"}), totalWorkers: nil, busyWorkers: nil, @@ -411,12 +416,16 @@ func NewPrometheusMetrics(registry prometheus.Registerer) *PrometheusMetrics { m.mustRegister(m.queueDepth) - m.mustRegister(m.opcacheRestarts) + // only where the hook exists: a series stuck at zero would read as "no + // restart" on a build that cannot report one + if opcacheRestartHook { + m.mustRegister(m.opcacheRestarts) - // expose the series at zero so a rate or an alert on them works from the - // first restart on, instead of missing it for lack of a previous sample - for _, reason := range opcacheRestartReasons { - m.opcacheRestarts.WithLabelValues(reason) + // expose the series at zero so a rate or an alert on it works from the + // first restart on, instead of missing it for lack of a previous sample + for _, reason := range opcacheRestartReasons { + m.opcacheRestarts.WithLabelValues(reason) + } } return m diff --git a/metrics_test.go b/metrics_test.go index 132a0db0db..1745a54d0a 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -1,12 +1,15 @@ package frankenphp import ( + "bytes" + "log/slog" "strings" "testing" "time" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -214,28 +217,54 @@ func TestPrometheusMetrics_TestStopReasonCrash(t *testing.T) { } func TestPrometheusMetrics_OpcacheRestart(t *testing.T) { - m := NewPrometheusMetrics(prometheus.NewRegistry()) - m.OpcacheRestart("hash overflow") - m.OpcacheRestart("hash overflow") - m.OpcacheRestart("out of memory") + if !opcacheRestartHook { + t.Skip("this build has no opcache restart hook") + } + + registry := prometheus.NewRegistry() + m := NewPrometheusMetrics(registry) + m.OpcacheRestart("hash") + m.OpcacheRestart("hash") + m.OpcacheRestart("oom") // known reasons are exposed from the start, unknown ones only once seen - require.NoError(t, testutil.CollectAndCompare(m.opcacheRestarts, strings.NewReader(` - # HELP frankenphp_opcache_restarts Number of restarts of opcache's shared memory, by reason (experimental, should stay at zero) + require.NoError(t, testutil.GatherAndCompare(registry, strings.NewReader(` + # HELP frankenphp_opcache_restarts Number of restarts of opcache's shared memory scheduled, by reason (experimental, should stay at zero) # TYPE frankenphp_opcache_restarts counter - frankenphp_opcache_restarts{reason="hash overflow"} 2 - frankenphp_opcache_restarts{reason="out of memory"} 1 - frankenphp_opcache_restarts{reason="user"} 0 - `))) + frankenphp_opcache_restarts{reason="hash"} 2 + frankenphp_opcache_restarts{reason="manual"} 0 + frankenphp_opcache_restarts{reason="oom"} 1 + `), "frankenphp_opcache_restarts")) m.OpcacheRestart("unknown") require.NoError(t, testutil.CollectAndCompare(m.opcacheRestarts, strings.NewReader(` - # HELP frankenphp_opcache_restarts Number of restarts of opcache's shared memory, by reason (experimental, should stay at zero) + # HELP frankenphp_opcache_restarts Number of restarts of opcache's shared memory scheduled, by reason (experimental, should stay at zero) # TYPE frankenphp_opcache_restarts counter - frankenphp_opcache_restarts{reason="hash overflow"} 2 - frankenphp_opcache_restarts{reason="out of memory"} 1 + frankenphp_opcache_restarts{reason="hash"} 2 + frankenphp_opcache_restarts{reason="manual"} 0 + frankenphp_opcache_restarts{reason="oom"} 1 frankenphp_opcache_restarts{reason="unknown"} 1 - frankenphp_opcache_restarts{reason="user"} 0 `))) } + +func TestOpcacheRestartScheduledLogsAndCounts(t *testing.T) { + if !opcacheRestartHook { + t.Skip("this build has no opcache restart hook") + } + + var buf bytes.Buffer + m := NewPrometheusMetrics(prometheus.NewRegistry()) + prevLogger, prevMetrics := globalLogger, metrics + globalLogger, metrics = slog.New(slog.NewTextHandler(&buf, nil)), m + t.Cleanup(func() { globalLogger, metrics = prevLogger, prevMetrics }) + + opcacheRestartScheduled(1) + opcacheRestartScheduled(7) + + assert.Contains(t, buf.String(), `level=WARN msg="opcache restart scheduled`) + assert.Contains(t, buf.String(), "reason=hash") + assert.Contains(t, buf.String(), "reason=unknown") + assert.Equal(t, float64(1), testutil.ToFloat64(m.opcacheRestarts.WithLabelValues("hash"))) + assert.Equal(t, float64(1), testutil.ToFloat64(m.opcacheRestarts.WithLabelValues("unknown"))) +} From 0d63bcce2ba264cfd2b4f9216efa4fee71d01669 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 21 Sep 2026 19:26:46 +0200 Subject: [PATCH 5/6] fix: keep the Metrics interface as it is, opcache restarts go through an optional one --- frankenphp.go | 4 +++- metrics.go | 18 +++++++++++++++--- metrics_test.go | 15 +++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/frankenphp.go b/frankenphp.go index 40e678122d..c18b06e2d2 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -866,7 +866,9 @@ func opcacheRestartScheduled(reason int) { reasonText = opcacheRestartReasons[reason] } - metrics.OpcacheRestart(reasonText) + if m, ok := metrics.(OpcacheMetrics); ok { + m.OpcacheRestart(reasonText) + } if !globalLogger.Enabled(globalCtx, slog.LevelWarn) { return diff --git a/metrics.go b/metrics.go index 9701dc7d23..852305aa35 100644 --- a/metrics.go +++ b/metrics.go @@ -16,6 +16,9 @@ const ( type StopReason int +// Metrics reports what the workers and the threads of a FrankenPHP instance +// are doing. An implementation that also satisfies OpcacheMetrics is told +// about opcache restarts as well. type Metrics interface { // StartWorker collects started workers StartWorker(name string) @@ -40,7 +43,13 @@ type Metrics interface { DequeuedWorkerRequest(name string) QueuedRequest() DequeuedRequest() - // OpcacheRestart collects the restarts of opcache's shared memory, by reason +} + +// OpcacheMetrics is the optional part of a Metrics implementation that counts +// the restarts of opcache's shared memory, by reason, where the build reports +// them (ZTS, PHP 8.4 and up). An implementation passed to WithMetrics() that +// lacks it only misses the counter, the restart is logged either way. +type OpcacheMetrics interface { OpcacheRestart(reason string) } @@ -83,8 +92,6 @@ func (n nullMetrics) DequeuedWorkerRequest(string) {} func (n nullMetrics) QueuedRequest() {} func (n nullMetrics) DequeuedRequest() {} -func (n nullMetrics) OpcacheRestart(string) {} - type PrometheusMetrics struct { registry prometheus.Registerer totalThreads prometheus.Gauge @@ -111,6 +118,11 @@ func (m *PrometheusMetrics) mustRegister(c prometheus.Collector) { } } +var ( + _ Metrics = (*PrometheusMetrics)(nil) + _ OpcacheMetrics = (*PrometheusMetrics)(nil) +) + func (m *PrometheusMetrics) StartWorker(name string) { m.mu.RLock() defer m.mu.RUnlock() diff --git a/metrics_test.go b/metrics_test.go index 1745a54d0a..4d5ec1a637 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -268,3 +268,18 @@ func TestOpcacheRestartScheduledLogsAndCounts(t *testing.T) { assert.Equal(t, float64(1), testutil.ToFloat64(m.opcacheRestarts.WithLabelValues("hash"))) assert.Equal(t, float64(1), testutil.ToFloat64(m.opcacheRestarts.WithLabelValues("unknown"))) } + +func TestOpcacheRestartScheduledWithoutOpcacheMetrics(t *testing.T) { + // nullMetrics is a Metrics without the optional part, like an + // implementation from before OpcacheMetrics existed + _, ok := Metrics(nullMetrics{}).(OpcacheMetrics) + require.False(t, ok) + + var buf bytes.Buffer + prevLogger, prevMetrics := globalLogger, metrics + globalLogger, metrics = slog.New(slog.NewTextHandler(&buf, nil)), nullMetrics{} + t.Cleanup(func() { globalLogger, metrics = prevLogger, prevMetrics }) + + assert.NotPanics(t, func() { opcacheRestartScheduled(0) }) + assert.Contains(t, buf.String(), "reason=oom") +} From a2e3cc0991b54d0cb5359884fe437cd757406adb Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 21 Sep 2026 19:30:49 +0200 Subject: [PATCH 6/6] fix: read PHP_VERSION_ID before gating the opcache restart hook, it was 0 on every build --- frankenphp.h | 1 + 1 file changed, 1 insertion(+) diff --git a/frankenphp.h b/frankenphp.h index 47cdb3da57..5288544f3f 100644 --- a/frankenphp.h +++ b/frankenphp.h @@ -16,6 +16,7 @@ #include #include +#include #include #include