feat(metrics)!: add server label to per-worker series#2396
Open
nicolas-grekas wants to merge 1 commit intophp:mainfrom
Open
feat(metrics)!: add server label to per-worker series#2396nicolas-grekas wants to merge 1 commit intophp:mainfrom
nicolas-grekas wants to merge 1 commit intophp:mainfrom
Conversation
Workers declared in distinct php_server blocks today share a single "worker" Prometheus label dimension, so metric series collide. This adds a sibling "server" label so each (server, worker) pair stays on its own series. Plumbing: - New Scope opaque type + NextScope() in scope.go: a per-php_server identifier the embedder allocates once per declarative block. - ScopeLabel(s) / SetScopeLabel(s, label): registry mapping Scope to a human-readable label, falling back to the numeric id when none is set. Empty labels are ignored. - WithWorkerScope(Scope) tags a declared worker; worker.scope copies it through newWorker. - Caddy module: each FrankenPHPModule provisions its own scope and on the first ServeHTTP resolves a label via the cascade (route host matcher -> user-set caddy server name -> first listen addr) and registers it via SetScopeLabel. API change: - BREAKING: every Metrics interface method that took (name string) now takes (server, name string). Embedders implementing frankenphp.Metrics need to widen their signatures; PrometheusMetrics and the null impl are updated in-tree. Mirrors the shape of php#1376 (which introduced the "worker" label the same way). - basicLabels becomes {"server", "worker"}. The scope mechanism is independently reusable for other per-server features (future per-server isolation, dispatching, etc.).
Contributor
Author
|
I could also move caddy/scopelabel.go et al from #2393 here. For now, I organized these PRs so that they can be merged independently. |
henderkes
reviewed
May 7, 2026
Contributor
henderkes
left a comment
henderkes
left a comment
There was a problem hiding this comment.
Do we really need this? Worker names are already unique so where's the win here? If a user sets multiple workers per domain? But this is per-php_server so even for that I'm not sure.
If it's just to better distinguish between different workers without a specific name set, why not change the automatically generated prefix instead?
| // isAutoServerName reports whether name is one of Caddy's auto-assigned | ||
| // server names (srv0, srv1, ...). Anything else is treated as user-set. | ||
| func isAutoServerName(name string) bool { | ||
| if !strings.HasPrefix(name, "srv") || len(name) <= 3 { |
Contributor
There was a problem hiding this comment.
this seems like a very common thing to prefix servers with, likely not the best check
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Today's per-worker Prometheus metrics carry only a
workerlabel. Twophp_serverblocks declaring workers with the same name therefore collide on the same metric series. This PR adds a siblingserverlabel so each(server, worker)pair stays on its own series.What changes
scope.go):Scopeopaque type,NextScope()per-block allocator, and aScopeLabel/SetScopeLabelregistry that maps a Scope to a human-readable label.FrankenPHPModuleprovisions its own scope and, on the firstServeHTTP, resolves a label via cascade — first host of the route's host matcher → user-set Caddy server name → first listener address → numeric id fallback. Registered once viasync.Once.(name string)now takes(server, name string). Mirrors the shape of introduces worker name option, use label on worker metrics instead #1376 which introduced theworkerlabel the same way.PrometheusMetricsandnullMetricsare updated in-tree; thebasicLabelsbecomes{\"server\", \"worker\"}.docs/metrics.mdreflects the new label set + cascade.Breaking change
Embedders implementing
frankenphp.Metricsneed to widen their method signatures. No runtime behaviour change for users who don't implement that interface.Test plan