Repository: caddyserver/caddy (language: go)

Issue:
with per_host in metrics, how about compatible with old version
current caddyfile is using:
```
{
	servers {
		metrics 
	}
}
```

if i switch to caddy@master (with per_host support in metrics), then above config will cause panic!! because missing :port (ie servers :80, servers :443 ) 

the full log:
```
panic: duplicate metrics collector registration attempted

goroutine 1 [running]:
github.com/prometheus/client_golang/prometheus.(*Registry).MustRegister(0xc00068ddb0, {0xc000812f20?, 0x1d1de87?, 0x4?})
        github.com/prometheus/client_golang@v1.19.1/prometheus/registry.go:405 +0x66
github.com/prometheus/client_golang/prometheus/promauto.Factory.NewGaugeVec({{0x22edd30?, 0xc00068ddb0?}}, {{0x1d1fa3f, 0x5}, {0x1d1de87, 0x4}, {0x1d51a67, 0x12}, {0x1da476e, 0x34}, ...}, ...)
        github.com/prometheus/client_golang@v1.19.1/prometheus/promauto/auto.go:308 +0x163
github.com/caddyserver/caddy/v2/modules/caddyhttp.initHTTPMetrics({{0x22f2478, 0xc00080ee70}, 0xc0004ae360, 0xc0000bed20, {0xc000645150, 0x1, 0x1}, {0x0, 0x0, 0x0}, ...}, ...)
        github.com/caddyserver/caddy/v2@v2.8.4/modules/caddyhttp/metrics.go:46 +0x130
github.com/caddyserver/caddy/v2/modules/caddyhttp.newMetricsInstrumentedHandler.func1()
        github.com/caddyserver/caddy/v2@v2.8.4/modules/caddyhttp/metrics.go:121 +0x38
sync.(*Once).doSlow(0x1?, 0x3?)
        sync/once.go:76 +0xb4
sync.(*Once).Do(...)
        sync/once.go:67
github.com/caddyserver/caddy/v2/modules/caddyhttp.newMetricsInstrumentedHandler({{0x22f2478, 0xc00080ee70}, 0xc0004ae360, 0xc0000bed20, {0xc000645150, 0x1, 0x1}, {0x0, 0x0, 0x0}, ...}, ...)
        github.com/caddyserver/caddy/v2@v2.8.4/modules/caddyhttp/metrics.go:120 +0xb3
github.com/caddyserver/caddy/v2/modules/caddyhttp.wrapMiddleware({{0x22f2478, 0xc00080ee70}, 0xc0004ae360, 0xc0000bed20, {0xc000645150, 0x1, 0x1}, {0x0, 0x0, 0x0}, ...}, ...)
        github.com/caddyserver/caddy/v2@v2.8.4/modules/caddyhttp/routes.go:321 +0x99
github.com/caddyserver/caddy/v2/modules/caddyhttp.(*Route).ProvisionHandlers(0xc000978a88, {{0x22f2478, 0xc00080ee70}, 0xc0004ae360, 0xc0000bed20, {0xc000645150, 0x1, 0x1}, {0x0, 0x0, ...}, ...}, ...)
        github.com/caddyserver/caddy/v2@v2.8.4/modules/caddyhttp/routes.go:167 +0x2e5
github.com/caddyserver/caddy/v2/modules/caddyhttp.RouteList.ProvisionHandlers({0xc000978a88, 0xb, 0xc000975190?}, {{0x22f2478, 0xc00080ee70}, 0xc0004ae360, 0xc0000bed20, {0xc000645150, 0x1, 0x1}, ...}, ...)
        github.com/caddyserver/caddy/v2@v2.8.4/modules/caddyhttp/routes.go:212 +0xa9
github.com/caddyserver/caddy/v2/modules/caddyhttp.(*App).Provision(0xc0006f3680, {{0x22f2478, 0xc00080ee70}, 0xc0004ae360, 0xc0000bed20, {0xc000645150, 0x1, 0x1}, {0x0, 0x0, ...}, ...})
        github.com/caddyserver/caddy/v2@v2.8.4/modules/caddyhttp/app.go:354 +0xe1e
github.com/caddyserver/caddy/v2.Context.LoadModuleByID({{0x22f24b0, 0xc00068de00}, 0xc0004ae360, 0xc0000bed20, {0xc000645150, 0x1, 0x1}, {0x0, 0x0, 0x0}, ...}, ...)
        github.com/caddyserver/caddy/v2@v2.8.4/context.go:389 +0x695
github.com/caddyserver/caddy/v2.Context.App({{0x22f24b0, 0xc00068de00}, 0xc0004ae360, 0xc0000bed20, {0x0, 0x0, 0x0}, {0x0, 0x0, 0x0}, ...}, ...)
        github.com/caddyserver/caddy/v2@v2.8.4/context.go:465 +0xee
github.com/caddyserver/caddy/v2.provisionContext.func3(...)
        github.com/caddyserver/caddy/v2@v2.8.4/caddy.go:545
github.com/caddyserver/caddy/v2.provisionContext(0x1?, 0x0)
        github.com/caddyserver/caddy/v2@v2.8.4/caddy.go:550 +0x53c
github.com/caddyserver/caddy/v2.run(0x1942f40?, 0x0)
        github.com/caddyserver/caddy/v2@v2.8.4/caddy.go:400 +0x78
github.com/caddyserver/caddy/v2.Validate(0xc0000bed20)
        github.com/caddyserver/caddy/v2@v2.8.4/caddy.go:714 +0x5a
github.com/caddyserver/caddy/v2/cmd.cmdValidateConfig({0x0?})
        github.com/caddyserver/caddy/v2@v2.8.4/cmd/commandfuncs.go:551 +0x155
github.com/caddyserver/caddy/v2/cmd.init.1.func8.WrapCommandFuncForCobra.1(0xc000924908, {0x1d1de5b?, 0x4?, 0x1d1ddc7?})
        github.com/caddyserver/caddy/v2@v2.8.4/cmd/cobra.go:141 +0x2f
github.com/spf13/cobra.(*Command).execute(0xc000924908, {0xc0006a3660, 0x2, 0x2})
        github.com/spf13/cobra@v1.8.1/command.go:985 +0xaaa
github.com/spf13/cobra.(*Command).ExecuteC(0xc00098a608)
        github.com/spf13/cobra@v1.8.1/command.go:1117 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
        github.com/spf13/cobra@v1.8.1/command.go:1041
github.com/caddyserver/caddy/v2/cmd.Main()
        github.com/caddyserver/caddy/v2@v2.8.4/cmd/main.go:75 +0x1dd
main.main()
        caddy/main.go:26 +0xf
```



Discussion / hints:
> because missing :port (ie servers :80, servers :443 )

I don't think that's related. Can you share your full config file? I need to replicate it, because I don't see the same issue on my side.
> I don't think that's related. Can you share your full config file? I need to replicate it, because I don't see the same issue on my side.

Here is the configuration file that can be replicated.

```
{
        servers {
                metrics
        }

}

*.example.com {
        respond "fallback"
}

http://nossl.example.com {
        respond "nossl"
}

```

> [!NOTE]  
> I have the same panic while trying to test `v2.9.0-beta.2` with my existing Caddyfile from `v.2.8.4`.

```console
panic: duplicate metrics collector registration attempted
goroutine 1 [running]:
github.com/prometheus/client_golang/prometheus.(*Registry).MustRegister(0xc0003d39f0, {0xc0008d36f0?, 0x1b21117?, 0x4?})
        github.com/prometheus/client_golang@v1.19.1/prometheus/registry.go:405 +0x66
github.com/prometheus/client_golang/prometheus/promauto.Factory.NewGaugeVec({{0x1ffd3c0?, 0xc0003d39f0?}}, {{0x1b229f7, 0x5}, {0x1b21117, 0x4}, {0x1b450a6, 0x12}, {0x1b9>
        github.com/prometheus/client_golang@v1.19.1/prometheus/promauto/auto.go:308 +0x163
github.com/caddyserver/caddy/v2/modules/caddyhttp.initHTTPMetrics({{0x2001aa8, 0xc000643bc0}, 0xc0004e0810, 0xc000284840, {0xc00073c1f0, 0x1, 0x1}, {0x0, 0x0, 0x0}, ...}>
        github.com/caddyserver/caddy/v2@v2.9.0-beta.2/modules/caddyhttp/metrics.go:46 +0x130
github.com/caddyserver/caddy/v2/modules/caddyhttp.newMetricsInstrumentedHandler.func1()
        github.com/caddyserver/caddy/v2@v2.9.0-beta.2/modules/caddyhttp/metrics.go:121 +0x38
sync.(*Once).doSlow(0x1?, 0x3?)
        sync/once.go:76 +0xb4
sync.(*Once).Do(...)
        sync/once.go:67
github.com/caddyserver/caddy/v2/modules/caddyhttp.newMetricsInstrumentedHandler({{0x2001aa8, 0xc000643bc0}, 0xc0004e0810, 0xc000284840, {0xc00073c1f0, 0x1, 0x1}, {0x0, 0>
        github.com/caddyserver/caddy/v2@v2.9.0-beta.2/modules/caddyhttp/metrics.go:120 +0xb3
github.com/caddyserver/caddy/v2/modules/caddyhttp.wrapMiddleware({{0x2001aa8, 0xc000643bc0}, 0xc0004e0810, 0xc000284840, {0xc00073c1f0, 0x1, 0x1}, {0x0, 0x0, 0x0}, ...},>
        github.com/caddyserver/caddy/v2@v2.9.0-beta.2/modules/caddyhttp/routes.go:321 +0x99
github.com/caddyserver/caddy/v2/modules/caddyhttp.(*Route).ProvisionHandlers(0xc00062a288, {{0x2001aa8, 0xc000643bc0}, 0xc0004e0810, 0xc000284840, {0xc00073c1f0, 0x1, 0x>
...
github.com/spf13/cobra.(*Command).execute(0xc000980608, {0xc0008fee70, 0x3, 0x3})
        github.com/spf13/cobra@v1.8.0/command.go:983 +0xaaa
github.com/spf13/cobra.(*Command).ExecuteC(0xc000980008)
        github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
        github.com/spf13/cobra@v1.8.0/command.go:1039
github.com/caddyserver/caddy/v2/cmd.Main()
        github.com/caddyserver/caddy/v2@v2.9.0-beta.2/cmd/main.go:75 +0x1dd
main.main()
        caddy/main.go:20 +0xf
```
Okay, I figured the root cause. This is hairy. The shared `caddy.Context` applies to the entire process, while the `metrics` key may exist in multiple servers within the same configuration object, so they both try to register themselves and end up registering the same collectors.

I need to control the registration flow within the caddy context. I wonder if it's better to swap the control and have the context register the collectors instead of the registry being given to the HTTP app.

Fix patch:
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index 4dacd9058d0..a238a33b42f 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -15,6 +15,7 @@
 package httpcaddyfile
 
 import (
+	"cmp"
 	"encoding/json"
 	"fmt"
 	"net"
@@ -186,12 +187,25 @@ func (st ServerType) Setup(
 		return nil, warnings, err
 	}
 
+	// hoist the metrics config from per-server to global
+	metrics, _ := options["metrics"].(*caddyhttp.Metrics)
+	for _, s := range servers {
+		if s.Metrics != nil {
+			metrics = cmp.Or[*caddyhttp.Metrics](metrics, &caddyhttp.Metrics{})
+			metrics = &caddyhttp.Metrics{
+				PerHost: metrics.PerHost || s.Metrics.PerHost,
+			}
+			s.Metrics = nil // we don't need it anymore
+		}
+	}
+
 	// now that each server is configured, make the HTTP app
 	httpApp := caddyhttp.App{
 		HTTPPort:      tryInt(options["http_port"], &warnings),
 		HTTPSPort:     tryInt(options["https_port"], &warnings),
 		GracePeriod:   tryDuration(options["grace_period"], &warnings),
 		ShutdownDelay: tryDuration(options["shutdown_delay"], &warnings),
+		Metrics:       metrics,
 		Servers:       servers,
 	}
 
diff --git a/caddyconfig/httpcaddyfile/options.go b/caddyconfig/httpcaddyfile/options.go
index 9bae760c01a..03b9ba230a6 100644
--- a/caddyconfig/httpcaddyfile/options.go
+++ b/caddyconfig/httpcaddyfile/options.go
@@ -24,6 +24,7 @@ import (
 	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/caddy/v2/caddyconfig"
 	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
+	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
 	"github.com/caddyserver/caddy/v2/modules/caddytls"
 )
 
@@ -53,6 +54,7 @@ func init() {
 	RegisterGlobalOption("local_certs", parseOptTrue)
 	RegisterGlobalOption("key_type", parseOptSingleString)
 	RegisterGlobalOption("auto_https", parseOptAutoHTTPS)
+	RegisterGlobalOption("metrics", parseMetricsOptions)
 	RegisterGlobalOption("servers", parseServerOptions)
 	RegisterGlobalOption("ocsp_stapling", parseOCSPStaplingOptions)
 	RegisterGlobalOption("cert_lifetime", parseOptDuration)
@@ -446,6 +448,24 @@ func parseOptAutoHTTPS(d *caddyfile.Dispenser, _ any) (any, error) {
 	return val, nil
 }
 
+func unmarshalCaddyfileMetricsOptions(d *caddyfile.Dispenser) (any, error) {
+	d.Next() // consume option name
+	metrics := new(caddyhttp.Metrics)
+	for d.NextBlock(0) {
+		switch d.Val() {
+		case "per_host":
+			metrics.PerHost = true
+		default:
+			return nil, d.Errf("unrecognized servers option '%s'", d.Val())
+		}
+	}
+	return metrics, nil
+}
+
+func parseMetricsOptions(d *caddyfile.Dispenser, _ any) (any, error) {
+	return unmarshalCaddyfileMetricsOptions(d)
+}
+
 func parseServerOptions(d *caddyfile.Dispenser, _ any) (any, error) {
 	return unmarshalCaddyfileServerOptions(d)
 }
diff --git a/caddyconfig/httpcaddyfile/serveroptions.go b/caddyconfig/httpcaddyfile/serveroptions.go
index b05af47f547..40a8af20962 100644
--- a/caddyconfig/httpcaddyfile/serveroptions.go
+++ b/caddyconfig/httpcaddyfile/serveroptions.go
@@ -240,6 +240,7 @@ func unmarshalCaddyfileServerOptions(d *caddyfile.Dispenser) (any, error) {
 			}
 
 		case "metrics":
+			caddy.Log().Warn("The nested 'metrics' option inside `servers` is deprecated and will be removed in the next major version. Use the global 'metrics' option instead.")
 			serverOpts.Metrics = new(caddyhttp.Metrics)
 			for nesting := d.Nesting(); d.NextBlock(nesting); {
 				switch d.Val() {
diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go
index 7dc2bee72fa..c57e14e9992 100644
--- a/modules/caddyhttp/app.go
+++ b/modules/caddyhttp/app.go
@@ -15,6 +15,7 @@
 package caddyhttp
 
 import (
+	"cmp"
 	"context"
 	"crypto/tls"
 	"fmt"
@@ -142,6 +143,10 @@ type App struct {
 	// affect functionality.
 	Servers map[string]*Server `json:"servers,omitempty"`
 
+	// If set, metrics observations will be enabled.
+	// This setting is EXPERIMENTAL and subject to change.
+	Metrics *Metrics `json:"metrics,omitempty"`
+
 	ctx    caddy.Context
 	logger *zap.Logger
 	tlsApp *caddytls.TLS
@@ -184,6 +189,10 @@ func (app *App) Provision(ctx caddy.Context) error {
 		return err
 	}
 
+	if app.Metrics != nil {
+		app.Metrics.init = sync.Once{}
+		app.Metrics.httpMetrics = &httpMetrics{}
+	}
 	// prepare each server
 	oldContext := ctx.Context
 	for srvName, srv := range app.Servers {
@@ -196,6 +205,15 @@ func (app *App) Provision(ctx caddy.Context) error {
 		srv.errorLogger = app.logger.Named("log.error")
 		srv.shutdownAtMu = new(sync.RWMutex)
 
+		if srv.Metrics != nil {
+			srv.logger.Warn("per-server 'metrics' is deprecated; use 'metrics' in the root 'http' app instead")
+			app.Metrics = cmp.Or[*Metrics](app.Metrics, &Metrics{
+				init:        sync.Once{},
+				httpMetrics: &httpMetrics{},
+			})
+			app.Metrics.PerHost = app.Metrics.PerHost || srv.Metrics.PerHost
+		}
+
 		// only enable access logs if configured
 		if srv.Logs != nil {
 			srv.accessLogger = app.logger.Named("log.access")
@@ -342,16 +360,11 @@ func (app *App) Provision(ctx caddy.Context) error {
 				srv.listenerWrappers = append([]caddy.ListenerWrapper{new(tlsPlaceholderWrapper)}, srv.listenerWrappers...)
 			}
 		}
-
 		// pre-compile the primary handler chain, and be sure to wrap it in our
 		// route handler so that important security checks are done, etc.
 		primaryRoute := emptyHandler
 		if srv.Routes != nil {
-			if srv.Metrics != nil {
-				srv.Metrics.init = sync.Once{}
-				srv.Metrics.httpMetrics = &httpMetrics{}
-			}
-			err := srv.Routes.ProvisionHandlers(ctx, srv.Metrics)
+			err := srv.Routes.ProvisionHandlers(ctx, app.Metrics)
 			if err != nil {
 				return fmt.Errorf("server %s: setting up route handlers: %v", srvName, err)
 			}
@@ -370,7 +383,7 @@ func (app *App) Provision(ctx caddy.Context) error {
 
 		// provision the named routes (they get compiled at runtime)
 		for name, route := range srv.NamedRoutes {
-			err := route.Provision(ctx, srv.Metrics)
+			err := route.Provision(ctx, app.Metrics)
 			if err != nil {
 				return fmt.Errorf("server %s: setting up named route '%s' handlers: %v", name, srvName, err)
 			}
diff --git a/modules/caddyhttp/metrics.go b/modules/caddyhttp/metrics.go
index 947721429f4..9bb97e0b47b 100644
--- a/modules/caddyhttp/metrics.go
+++ b/modules/caddyhttp/metrics.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"net/http"
+	"strings"
 	"sync"
 	"time"
 
@@ -133,8 +134,8 @@ func (h *metricsInstrumentedHandler) ServeHTTP(w http.ResponseWriter, r *http.Re
 	statusLabels := prometheus.Labels{"server": server, "handler": h.handler, "method": method, "code": ""}
 
 	if h.metrics.PerHost {
-		labels["host"] = r.Host
-		statusLabels["host"] = r.Host
+		labels["host"] = strings.ToLower(r.Host)
+		statusLabels["host"] = strings.ToLower(r.Host)
 	}
 
 	inFlight := h.metrics.httpMetrics.requestInFlight.With(labels)
diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go
index 5aa7e0f6351..1b81bd98aa2 100644
--- a/modules/caddyhttp/server.go
+++ b/modules/caddyhttp/server.go
@@ -226,6 +226,7 @@ type Server struct {
 
 	// If set, metrics observations will be enabled.
 	// This setting is EXPERIMENTAL and subject to change.
+	// DEPRECATED: Use the app-level `metrics` field.
 	Metrics *Metrics `json:"metrics,omitempty"`
 
 	name string


Test patch:
diff --git a/caddytest/integration/caddyfile_adapt/metrics_merge_options.caddyfiletest b/caddytest/integration/caddyfile_adapt/metrics_merge_options.caddyfiletest
new file mode 100644
index 00000000000..946b3d0c83a
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/metrics_merge_options.caddyfiletest
@@ -0,0 +1,39 @@
+{
+	metrics
+	servers :80 {
+		metrics {
+			per_host
+		}
+	}
+}
+:80 {
+	respond "Hello"
+}
+
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":80"
+					],
+					"routes": [
+						{
+							"handle": [
+								{
+									"body": "Hello",
+									"handler": "static_response"
+								}
+							]
+						}
+					]
+				}
+			},
+			"metrics": {
+				"per_host": true
+			}
+		}
+	}
+}
\ No newline at end of file
diff --git a/caddytest/integration/caddyfile_adapt/metrics_perhost.caddyfiletest b/caddytest/integration/caddyfile_adapt/metrics_perhost.caddyfiletest
index 49921551573..e362cecc93e 100644
--- a/caddytest/integration/caddyfile_adapt/metrics_perhost.caddyfiletest
+++ b/caddytest/integration/caddyfile_adapt/metrics_perhost.caddyfiletest
@@ -26,11 +26,11 @@
 								}
 							]
 						}
-					],
-					"metrics": {
-						"per_host": true
-					}
+					]
 				}
+			},
+			"metrics": {
+				"per_host": true
 			}
 		}
 	}