Skip to content

Commit a6e1d35

Browse files
authored
fix negative frankenphp_ready_workers metrics (#1491)
1 parent 6f1b4f3 commit a6e1d35

File tree

3 files changed

+118
-0
lines changed

3 files changed

+118
-0
lines changed

caddy/caddy_test.go

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,3 +1052,104 @@ func TestDisabledMetrics(t *testing.T) {
10521052
require.NoError(t, err, "failed to count metrics")
10531053
require.Zero(t, count, "metrics should be missing")
10541054
}
1055+
1056+
func TestWorkerRestart(t *testing.T) {
1057+
var wg sync.WaitGroup
1058+
tester := caddytest.NewTester(t)
1059+
tester.InitServer(`
1060+
{
1061+
skip_install_trust
1062+
admin localhost:2999
1063+
http_port `+testPort+`
1064+
https_port 9443
1065+
1066+
metrics
1067+
frankenphp {
1068+
worker {
1069+
name service
1070+
file ../testdata/worker-restart.php
1071+
num 1
1072+
# restart every 3 requests
1073+
env EVERY 3
1074+
}
1075+
}
1076+
}
1077+
1078+
localhost:`+testPort+` {
1079+
route {
1080+
php {
1081+
root ../testdata
1082+
}
1083+
}
1084+
}
1085+
`, "caddyfile")
1086+
1087+
ctx := caddy.ActiveContext()
1088+
1089+
resp, err := http.Get("http://localhost:2999/metrics")
1090+
require.NoError(t, err, "failed to fetch metrics")
1091+
defer resp.Body.Close()
1092+
1093+
// Read and parse metrics
1094+
metrics := new(bytes.Buffer)
1095+
_, err = metrics.ReadFrom(resp.Body)
1096+
require.NoError(t, err, "failed to read metrics")
1097+
1098+
// frankenphp_worker_restarts should be missing
1099+
count, err := testutil.GatherAndCount(
1100+
ctx.GetMetricsRegistry(),
1101+
"frankenphp_worker_restarts",
1102+
)
1103+
require.NoError(t, err, "failed to count metrics")
1104+
require.Zero(t, count, "metrics should be missing")
1105+
1106+
// Check metrics
1107+
expectedMetrics := `
1108+
# HELP frankenphp_ready_workers Running workers that have successfully called frankenphp_handle_request at least once
1109+
# TYPE frankenphp_ready_workers gauge
1110+
frankenphp_ready_workers{worker="service"} 1
1111+
# HELP frankenphp_total_workers Total number of PHP workers for this worker
1112+
# TYPE frankenphp_total_workers gauge
1113+
frankenphp_total_workers{worker="service"} 1
1114+
`
1115+
1116+
require.NoError(t,
1117+
testutil.GatherAndCompare(
1118+
ctx.GetMetricsRegistry(),
1119+
strings.NewReader(expectedMetrics),
1120+
"frankenphp_total_workers",
1121+
"frankenphp_ready_workers",
1122+
))
1123+
1124+
// Make some requests
1125+
for i := 0; i < 10; i++ {
1126+
wg.Add(1)
1127+
go func(i int) {
1128+
tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/worker-restart.php?i=%d", i), http.StatusOK, fmt.Sprintf("Counter (%d)", i))
1129+
wg.Done()
1130+
}(i)
1131+
}
1132+
wg.Wait()
1133+
1134+
// frankenphp_ready_workers should be back to 1 even after worker restarts
1135+
expectedMetrics = `
1136+
# HELP frankenphp_ready_workers Running workers that have successfully called frankenphp_handle_request at least once
1137+
# TYPE frankenphp_ready_workers gauge
1138+
frankenphp_ready_workers{worker="service"} 1
1139+
# HELP frankenphp_total_workers Total number of PHP workers for this worker
1140+
# TYPE frankenphp_total_workers gauge
1141+
frankenphp_total_workers{worker="service"} 1
1142+
# HELP frankenphp_worker_restarts Number of PHP worker restarts for this worker
1143+
# TYPE frankenphp_worker_restarts counter
1144+
frankenphp_worker_restarts{worker="service"} 3
1145+
`
1146+
1147+
require.NoError(t,
1148+
testutil.GatherAndCompare(
1149+
ctx.GetMetricsRegistry(),
1150+
strings.NewReader(expectedMetrics),
1151+
"frankenphp_total_workers",
1152+
"frankenphp_ready_workers",
1153+
"frankenphp_worker_restarts",
1154+
))
1155+
}

testdata/worker-restart.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
$fn = static function () {
4+
echo sprintf("Counter (%s)", $_GET['i'] ?? 'i not set');
5+
};
6+
7+
$loopMax = $_SERVER['EVERY'] ?? 10;
8+
$loops = 0;
9+
do {
10+
$ret = \frankenphp_handle_request($fn);
11+
} while ($ret && (-1 === $loopMax || ++$loops < $loopMax));
12+
13+
exit(0);

threadworker.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ func setupWorkerScript(handler *workerThread, worker *worker) {
7777
handler.backoff.wait()
7878
metrics.StartWorker(worker.name)
7979

80+
if handler.state.is(stateReady) {
81+
metrics.ReadyWorker(handler.worker.name)
82+
}
83+
8084
// Create a dummy request to set up the worker
8185
fc, err := newDummyContext(
8286
filepath.Base(worker.fileName),

0 commit comments

Comments
 (0)