Skip to content

Commit 855b3f9

Browse files
metrics: register prometheus collectors only if enabled (#1457)
* collect metrics only if enabled * Update caddy/caddy.go Co-authored-by: Kévin Dunglas <kevin@dunglas.fr> * Update caddy/caddy.go Co-authored-by: Kévin Dunglas <kevin@dunglas.fr> --------- Co-authored-by: Kévin Dunglas <kevin@dunglas.fr>
1 parent f85ca1c commit 855b3f9

File tree

4 files changed

+94
-3
lines changed

4 files changed

+94
-3
lines changed

caddy/caddy.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,17 @@ func (f FrankenPHPApp) CaddyModule() caddy.ModuleInfo {
8383

8484
// Provision sets up the module.
8585
func (f *FrankenPHPApp) Provision(ctx caddy.Context) error {
86-
f.metrics = frankenphp.NewPrometheusMetrics(ctx.GetMetricsRegistry())
8786
f.logger = ctx.Logger()
8887

88+
if httpApp, err := ctx.AppIfConfigured("http"); err == nil {
89+
if httpApp.(*caddyhttp.App).Metrics != nil {
90+
f.metrics = frankenphp.NewPrometheusMetrics(ctx.GetMetricsRegistry())
91+
}
92+
} else {
93+
// if the http module is not configured (this should never happen) then collect the metrics by default
94+
f.metrics = frankenphp.NewPrometheusMetrics(ctx.GetMetricsRegistry())
95+
}
96+
8997
return nil
9098
}
9199

caddy/caddy_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ func TestMetrics(t *testing.T) {
325325
admin localhost:2999
326326
http_port `+testPort+`
327327
https_port 9443
328+
metrics
328329
329330
frankenphp
330331
}
@@ -405,6 +406,7 @@ func TestWorkerMetrics(t *testing.T) {
405406
admin localhost:2999
406407
http_port `+testPort+`
407408
https_port 9443
409+
metrics
408410
409411
frankenphp {
410412
worker ../testdata/index.php 2
@@ -502,6 +504,7 @@ func TestNamedWorkerMetrics(t *testing.T) {
502504
admin localhost:2999
503505
http_port `+testPort+`
504506
https_port 9443
507+
metrics
505508
506509
frankenphp {
507510
worker {
@@ -594,6 +597,7 @@ func TestAutoWorkerConfig(t *testing.T) {
594597
admin localhost:2999
595598
http_port `+testPort+`
596599
https_port 9443
600+
metrics
597601
598602
frankenphp {
599603
worker ../testdata/index.php
@@ -873,6 +877,7 @@ func TestMultiWorkersMetrics(t *testing.T) {
873877
admin localhost:2999
874878
http_port `+testPort+`
875879
https_port 9443
880+
metrics
876881
877882
frankenphp {
878883
worker {
@@ -979,6 +984,7 @@ func TestMultiWorkersMetricsWithDuplicateName(t *testing.T) {
979984
admin localhost:2999
980985
http_port `+testPort+`
981986
https_port 9443
987+
metrics
982988
983989
frankenphp {
984990
worker {
@@ -1073,3 +1079,81 @@ func TestMultiWorkersMetricsWithDuplicateName(t *testing.T) {
10731079
"frankenphp_ready_workers",
10741080
))
10751081
}
1082+
1083+
func TestDisabledMetrics(t *testing.T) {
1084+
var wg sync.WaitGroup
1085+
tester := caddytest.NewTester(t)
1086+
tester.InitServer(`
1087+
{
1088+
skip_install_trust
1089+
admin localhost:2999
1090+
http_port `+testPort+`
1091+
https_port 9443
1092+
1093+
frankenphp {
1094+
worker {
1095+
name service1
1096+
file ../testdata/index.php
1097+
num 2
1098+
}
1099+
worker {
1100+
name service1
1101+
file ../testdata/ini.php
1102+
num 3
1103+
}
1104+
}
1105+
}
1106+
1107+
localhost:`+testPort+` {
1108+
route {
1109+
php {
1110+
root ../testdata
1111+
}
1112+
}
1113+
}
1114+
1115+
example.com:`+testPort+` {
1116+
route {
1117+
php {
1118+
root ../testdata
1119+
}
1120+
}
1121+
}
1122+
`, "caddyfile")
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+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i))
1129+
wg.Done()
1130+
}(i)
1131+
}
1132+
wg.Wait()
1133+
1134+
// Fetch metrics
1135+
resp, err := http.Get("http://localhost:2999/metrics")
1136+
require.NoError(t, err, "failed to fetch metrics")
1137+
defer resp.Body.Close()
1138+
1139+
// Read and parse metrics
1140+
metrics := new(bytes.Buffer)
1141+
_, err = metrics.ReadFrom(resp.Body)
1142+
require.NoError(t, err, "failed to read metrics")
1143+
1144+
ctx := caddy.ActiveContext()
1145+
count, err := testutil.GatherAndCount(
1146+
ctx.GetMetricsRegistry(),
1147+
"frankenphp_busy_threads",
1148+
"frankenphp_busy_workers",
1149+
"frankenphp_queue_depth",
1150+
"frankenphp_ready_workers",
1151+
"frankenphp_total_threads",
1152+
"frankenphp_total_workers",
1153+
"frankenphp_worker_request_count",
1154+
"frankenphp_worker_request_time",
1155+
)
1156+
1157+
require.NoError(t, err, "failed to count metrics")
1158+
require.Zero(t, count, "metrics should be missing")
1159+
}

testdata/performance/k6.Caddyfile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
root /go/src/app/testdata
1515
php {
1616
root /go/src/app/testdata
17-
enable_root_symlink false
1817
}
1918
}
2019
}

testdata/performance/perf-test.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ select filename in ./testdata/performance/*.js; do
2727

2828
sleep 10
2929

30-
docker run --entrypoint "" -it -v .:/app -w /app \
30+
docker run --entrypoint "" -it --rm -v .:/app -w /app \
3131
--add-host "host.docker.internal:host-gateway" \
3232
grafana/k6:latest \
3333
k6 run -e "CADDY_HOSTNAME=$CADDY_HOSTNAME:8125" "./$filename"

0 commit comments

Comments
 (0)