metrics: only report workers ready when actually ready (php#2210)

In php#2205 it appears that workers could be reported in metrics as "ready"
before they are actually ready. This changes the reporting so that
workers are only reported ready once they have completed booting.

Signed-off-by: Robert Landers <landers.robert@gmail.com>

Author: withinboredom

metrics.go

@@ -11,7 +11,7 @@ import (
 const (
 	StopReasonCrash = iota
 	StopReasonRestart
-	//StopReasonShutdown
+	StopReasonBootFailure // worker crashed before reaching frankenphp_handle_request
 )
 
 type StopReason int
@@ -125,10 +125,14 @@ func (m *PrometheusMetrics) StopWorker(name string, reason StopReason) {
 	}
 
 	m.totalWorkers.WithLabelValues(name).Dec()
-	m.readyWorkers.WithLabelValues(name).Dec()
+
+	// only decrement readyWorkers if the worker actually reached frankenphp_handle_request
+	if reason != StopReasonBootFailure {
+		m.readyWorkers.WithLabelValues(name).Dec()
+	}
 
 	switch reason {
-	case StopReasonCrash:
+	case StopReasonCrash, StopReasonBootFailure:
 		m.workerCrashes.WithLabelValues(name).Inc()
 	case StopReasonRestart:
 		m.workerRestarts.WithLabelValues(name).Inc()

threadworker.go

@@ -101,10 +101,6 @@ func (handler *workerThread) name() string {
 func setupWorkerScript(handler *workerThread, worker *worker) {
 	metrics.StartWorker(worker.name)
 
-	if handler.state.Is(state.Ready) {
-		metrics.ReadyWorker(handler.worker.name)
-	}
-
 	// Create a dummy request to set up the worker
 	fc, err := newDummyContext(
 		filepath.Base(worker.fileName),
@@ -152,7 +148,11 @@ func tearDownWorkerScript(handler *workerThread, exitStatus int) {
 	}
 
 	// worker has thrown a fatal error or has not reached frankenphp_handle_request
-	metrics.StopWorker(worker.name, StopReasonCrash)
+	if handler.isBootingScript {
+		metrics.StopWorker(worker.name, StopReasonBootFailure)
+	} else {
+		metrics.StopWorker(worker.name, StopReasonCrash)
+	}
 
 	if !handler.isBootingScript {
 		// fatal error (could be due to exit(1), timeouts, etc.)
@@ -207,13 +207,12 @@ func (handler *workerThread) waitForWorkerRequest() (bool, any) {
 		if !C.frankenphp_shutdown_dummy_request() {
 			panic("Not in CGI context")
 		}
+
+		// worker is truly ready only after reaching frankenphp_handle_request()
+		metrics.ReadyWorker(handler.worker.name)
 	}
 
-	// worker threads are 'ready' after they first reach frankenphp_handle_request()
-	// 'state.TransitionComplete' is only true on the first boot of the worker script,
-	// while 'isBootingScript' is true on every boot of the worker script
 	if handler.state.Is(state.TransitionComplete) {
-		metrics.ReadyWorker(handler.worker.name)
 		handler.state.Set(state.Ready)
 	}
 

worker.go

@@ -98,7 +98,6 @@ func initWorkers(opt []workerOpt) error {
 	return nil
 }
 
-
 func newWorker(o workerOpt) (*worker, error) {
 	// Order is important!
 	// This order ensures that FrankenPHP started from inside a symlinked directory will properly resolve any paths.