🔥 feat: Prometheus middleware#1834
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gofiber/contrib/sessions/10c0c586-2ddf-40aa-a369-6310b4f7a70a Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
ReneWerner87
requested review from
efectn,
gaby and
sixcolors
and removed request for
a team
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a Prometheus middleware for Fiber v3, enabling the tracking of HTTP request metrics, durations, and sizes with support for custom registries and OpenMetrics. The review feedback points out a critical error regarding the Go version in go.mod and identifies major performance bottlenecks in the route-tracking logic. Specifically, the reviewer recommends utilizing Fiber's built-in ctx.Route() instead of manual stack scanning and advises against expensive metric deletions on unmatched requests to ensure the middleware remains performant under high traffic.
| @@ -0,0 +1,41 @@ | |||
| module github.com/gofiber/contrib/v3/prometheus | |||
|
|
|||
| go 1.25.0 | |||
There was a problem hiding this comment.
The Go version is set to 1.25.0, which is a future version that has not been released yet (the current stable versions are 1.23 and 1.24). This will prevent the module from being compiled or used by developers on current stable toolchains.
| go 1.25.0 | |
| go 1.23.0 |
| routePath := m.resolveRoutePath(ctx) | ||
| routeKey := method + " " + routePath | ||
|
|
||
| registered := m.refreshRoutes(ctx, routeKey) |
There was a problem hiding this comment.
The refreshRoutes function is called on every request and performs an stackSize (line 403). This introduces a significant performance bottleneck, especially for applications with many routes.
Furthermore, stackSize is an unreliable cache invalidation key because the total count of routes can remain the same even if routes are modified or replaced.
Since Fiber v3 populates ctx.Route() before executing middlewares, you can rely on ctx.Route() as the source of truth for whether a request matches a registered route and what its template path is. This eliminates the need for manual stack scanning, the registeredRoutes map, and the associated mutex locks.
| func (m *middleware) refreshRoutes(ctx fiber.Ctx, routeKey string) bool { | ||
| stack := ctx.App().Stack() | ||
| stackVersion := stackSize(stack) | ||
|
|
||
| m.routesMu.RLock() | ||
| currentVersion := m.routesVersion | ||
| _, registered := m.registeredRoutes[routeKey] | ||
| m.routesMu.RUnlock() | ||
|
|
||
| if registered && currentVersion == stackVersion { | ||
| return true | ||
| } | ||
|
|
||
| routes := make(map[string]struct{}) | ||
| for i := range stack { | ||
| routesList := stack[i] | ||
| for j := range routesList { | ||
| r := routesList[j] | ||
| if r == nil { | ||
| continue | ||
| } | ||
|
|
||
| path := utils.CopyString(r.Path) | ||
| if path == "" { | ||
| path = "/" | ||
| } else if path != "/" { | ||
| path = normalizePath(path) | ||
| } | ||
|
|
||
| routes[r.Method+" "+path] = struct{}{} | ||
| if r.Method == fiber.MethodGet { | ||
| routes[fiber.MethodHead+" "+path] = struct{}{} | ||
| } | ||
| } | ||
| } | ||
|
|
||
| m.routesMu.Lock() | ||
| m.registeredRoutes = routes | ||
| m.routesVersion = stackVersion | ||
| _, registered = routes[routeKey] | ||
| m.routesMu.Unlock() | ||
|
|
||
| return registered | ||
| } |
There was a problem hiding this comment.
As noted in the performance feedback for the instrument function, this entire route caching and stack scanning logic is redundant in Fiber v3. You can remove this function and the corresponding fields (registeredRoutes, routesVersion, routesMu) from the middleware struct to improve performance and simplify the codebase.
| defer func() { | ||
| m.requestInFlight.WithLabelValues(method, inflightPath).Dec() | ||
| if deleteGauge { | ||
| m.requestInFlight.DeleteLabelValues(method, inflightPath) |
There was a problem hiding this comment.
Calling DeleteLabelValues on every unmatched request (e.g., 404s) is computationally expensive because it involves acquiring global locks within the Prometheus registry and cleaning up internal metric maps. Under high load or a 404-based DDoS attack, this will cause significant performance degradation.
Instead of using the raw ctx.Path() and then deleting the series, it is better to use a fixed placeholder label (like /__unmatched__) for the in-flight gauge when ctx.Route() is nil. This avoids the expensive cleanup and protects against cardinality explosion.
There was a problem hiding this comment.
Pull request overview
Adds a new Prometheus instrumentation middleware module for Fiber v3, including configurable collectors/OpenMetrics options and a full test suite, and wires it into the repo (workspace + docs + CI).
Changes:
- Introduce
v3/prometheusmiddleware that instruments requests (counts, status class, durations, sizes, in-flight) and serves a Prometheus/OpenMetrics scrape endpoint. - Add comprehensive tests covering skip/ignore behavior, custom buckets/registry, HEAD behavior, OpenMetrics negotiation, and exemplars.
- Register the new module in workspace/docs and add a dedicated GitHub Actions workflow.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| v3/prometheus/prometheus.go | Implements the middleware, metrics registration, route tracking, and scrape handling. |
| v3/prometheus/config.go | Defines configuration options and defaulting/copying behavior. |
| v3/prometheus/prometheus_test.go | Adds test coverage for metrics emission and configuration toggles. |
| v3/prometheus/README.md | Documents usage and available metrics/options. |
| v3/prometheus/go.mod | New module definition and direct dependencies. |
| v3/prometheus/go.sum | New module dependency checksums. |
| .github/workflows/test-prometheus.yml | Adds CI job for the new module. |
| go.work | Adds ./v3/prometheus to the workspace. |
| README.md | Adds Prometheus to the v3 module list. |
| v3/README.md | Adds Prometheus to the v3 module list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| m.requestInFlight.WithLabelValues(method, inflightPath).Inc() | ||
| deleteGauge := false | ||
| defer func() { | ||
| m.requestInFlight.WithLabelValues(method, inflightPath).Dec() |
There was a problem hiding this comment.
The in-flight gauge uses WithLabelValues() again inside the defer and may also delete the label values. If another request deletes these label values before this defer runs, the deferred WithLabelValues() will create a fresh gauge at 0 and then Dec() it to -1, producing incorrect negative in-flight counts. Consider capturing the child gauge returned by WithLabelValues() once (use it for both Inc/Dec) and avoid calling WithLabelValues() in the defer; deletion can still be done after Dec if desired.
| m.requestInFlight.WithLabelValues(method, inflightPath).Inc() | |
| deleteGauge := false | |
| defer func() { | |
| m.requestInFlight.WithLabelValues(method, inflightPath).Dec() | |
| inflightGauge := m.requestInFlight.WithLabelValues(method, inflightPath) | |
| inflightGauge.Inc() | |
| deleteGauge := false | |
| defer func() { | |
| inflightGauge.Dec() |
| if _, ok := m.skipURIs[routePath]; ok { | ||
| deleteGauge = true | ||
| return err | ||
| } |
There was a problem hiding this comment.
SkipURIs are checked after ctx.Next() and after the in-flight gauge has already been incremented. This means "skipped" routes can still show up temporarily in http_requests_in_progress during the request (and adds avoidable overhead). If SkipURIs is intended to exclude all instrumentation including in-flight tracking, check SkipURIs before touching requestInFlight and return ctx.Next() early.
| app.Use("/metrics", fiberprometheus.New(fiberprometheus.Config{ | ||
| Service: "my-service-name", | ||
| SkipURIs: []string{"/ping"}, | ||
| IgnoreStatusCodes: []int{401, 403, 404}, | ||
| })) | ||
|
|
There was a problem hiding this comment.
The example mounts the middleware only at /metrics (app.Use("/metrics", ...)). With Fiber's path-based middleware, that would generally only run for /metrics requests, so it won't instrument other routes unless the handler is also registered as a global middleware. Consider updating the example to show registering the returned handler twice (global + metrics path), or clarify explicitly how to mount it so it both instruments traffic and serves the scrape endpoint.
| app.Use("/metrics", fiberprometheus.New(fiberprometheus.Config{ | |
| Service: "my-service-name", | |
| SkipURIs: []string{"/ping"}, | |
| IgnoreStatusCodes: []int{401, 403, 404}, | |
| })) | |
| prometheus := fiberprometheus.New(fiberprometheus.Config{ | |
| Service: "my-service-name", | |
| SkipURIs: []string{"/ping"}, | |
| IgnoreStatusCodes: []int{401, 403, 404}, | |
| }) | |
| app.Use(prometheus) | |
| app.Use("/metrics", prometheus) |
| - 'v3/prometheus/go.mod' | ||
| pull_request: | ||
| paths: | ||
| - 'v3/prometheus/**/*.go' | ||
| - 'v3/prometheus/go.mod' |
There was a problem hiding this comment.
The workflow only triggers on changes to Go files and go.mod, but dependency-only updates can be done by modifying go.sum. Consider including v3/prometheus/go.sum in the workflow paths filters so CI still runs when module dependencies change.
| - 'v3/prometheus/go.mod' | |
| pull_request: | |
| paths: | |
| - 'v3/prometheus/**/*.go' | |
| - 'v3/prometheus/go.mod' | |
| - 'v3/prometheus/go.mod' | |
| - 'v3/prometheus/go.sum' | |
| pull_request: | |
| paths: | |
| - 'v3/prometheus/**/*.go' | |
| - 'v3/prometheus/go.mod' | |
| - 'v3/prometheus/go.sum' |
WalkthroughThis pull request introduces a complete Prometheus middleware implementation for Fiber v3. It adds configuration structures, core middleware logic for collecting request metrics (counters, histograms, gauges), documentation, and a comprehensive test suite covering various configuration scenarios and OpenMetrics support. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Middleware as Prometheus Middleware
participant MetricsRegistry as Prometheus Registry
participant Handler as Next Handler/App
participant MetricsEndpoint as /metrics Endpoint
Client->>Middleware: HTTP Request
Note over Middleware: Check if metrics endpoint request
alt Is Metrics Endpoint Request
Middleware->>MetricsRegistry: Query registered metrics
MetricsRegistry-->>Middleware: Serialized metrics (Prometheus/OpenMetrics)
Middleware-->>Client: Metrics output
else Regular Request
Middleware->>Middleware: Increment in-flight gauge
Middleware->>Handler: Execute handler
Handler-->>Middleware: Response
Middleware->>MetricsRegistry: Record request counter
Middleware->>MetricsRegistry: Record request duration histogram
Middleware->>MetricsRegistry: Record request/response size histograms
Middleware->>MetricsRegistry: Record status-class counter
Middleware->>Middleware: Decrement in-flight gauge
Middleware-->>Client: HTTP Response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
v3/prometheus/README.md (1)
41-64: Mixed tabs/spaces in the example block.Lines 42, 50, 54, 58, 62 use 8-space indentation while lines 44–48 use tabs. Pick one (Go convention is tabs) so the rendered code block is uniform.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/prometheus/README.md` around lines 41 - 64, The README example mixes spaces and tabs; normalize the indentation to Go convention (tabs) so the code block renders uniformly: update the func main() example and its handlers (main(), app := fiber.New(), app.Use(fiberprometheus.Config{...}), app.Get("/", ...), app.Get("/ping", ...), app.Post("/some", ...), app.Listen(":3000")) to use tabs for all leading indentation and remove the 8-space prefixes on the listed lines so every line uses tabs consistently.v3/prometheus/prometheus.go (2)
403-409:stackSizeonly counts routes — it can miss a swap.The version is just the total route count. If a route is removed and another added (or a sub-app is mounted/replaced) without changing the count,
currentVersion == stackVersionstays true and the cachedregisteredRoutesmap silently goes stale. In practice Fiber rarely removes routes, but sub-app mounting viaUse(prefix, subApp)can rewrite the stack.A cheaper and more reliable signal would be
app.HandlersCount()(already exposed by Fiber) or hashingr.Method+r.Pathwhile iterating once. Optional — only worth doing if you expect dynamic mounts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/prometheus/prometheus.go` around lines 403 - 409, stackSize currently only sums route counts which can miss swaps and leave registeredRoutes stale when stack composition changes; update stackSize (or the versioning check that uses it) to use a more reliable signal such as Fiber's app.HandlersCount() or compute a cheap hash while iterating over routes (e.g., incorporate r.Method and r.Path) so swaps change the version; modify the logic that compares stackVersion/currentVersion to use the new handlers count or the hash, ensuring functions and variables like stackSize, registeredRoutes, stackVersion, currentVersion, and any code that mounts sub-apps via Use(prefix, subApp) reflect the new version source.
58-64:Servicesilently overridesLabels["service"].If a caller sets both
cfg.Labels["service"]andcfg.Service, the latter wins without any warning. That's probably fine (Service is the dedicated knob), but worth documenting in theServicefield doc inconfig.goso users aren't surprised when their custom label disappears.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/prometheus/prometheus.go` around lines 58 - 64, The code in prometheus.go silently lets cfg.Service override any existing cfg.Labels["service"] when building labels (see the labels map creation and the cfg.Service assignment); update the Service field's doc comment in the config struct (the Service field in config.go) to explicitly state that Service takes precedence and will overwrite any Labels["service"] value so users are not surprised, and mention that they should set Service only if they want to override the label or omit the key from Labels if they want the label retained.v3/prometheus/prometheus_test.go (1)
712-721: Exemplar bucket-label assertion depends on OpenMetrics float formatting.The check requires
le="256.0"(OpenMetrics float) rather thanle="256"(Prometheus text). That's correct given theAccept: application/openmetrics-textheader, but the brittle multi-Containschain will be hard to debug ifclient_golangever updates its sample/exemplar serialization. Consider either parsing the line viastrings.Fieldsand asserting on a structured form, or relaxing the bucket boundary to a regex (e.g.,le="256(\.0)?"). Optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/prometheus/prometheus_test.go` around lines 712 - 721, The test's exemplar bucket-label checks in prometheus_test.go use brittle strings.Contains chains against metrics and require the OpenMetrics float format le="256.0"; update the assertions around requestLineWithExemplar and responseLineWithExemplar to be robust by matching the bucket boundary with a regex that accepts either le="256" or le="256.0" (e.g., le="256(\\.0)?") or by tokenizing each metric line via strings.Fields and comparing structured label/key-value pairs instead of raw substring matches; update the checks that currently call strings.Contains(line, "...le=\"256.0\"...") to use the regex or parsed label comparison so they tolerate formatting changes from client_golang while still asserting the exemplar presence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@v3/prometheus/prometheus.go`:
- Around line 242-278: The deferred cleanup in the middleware uses
m.requestInFlight.DeleteLabelValues with a deleteGauge flag (controlled in the
request path where deleteGauge is set) which races under concurrent requests and
can produce negative in-flight counts; remove the DeleteLabelValues call and the
deleteGauge plumbing (keep the Inc()/Dec() calls on
m.requestInFlight.WithLabelValues(method, inflightPath)) so labels remain with
value 0 instead of being removed, and update tests (e.g.,
TestIgnoreStatusCodesRemovesInFlightGauge) to expect a 0-valued metric rather
than absence.
In `@v3/prometheus/README.md`:
- Line 91: The sentence "All of the options default to `false`" is incorrect —
update the README text to accurately state that some options default to false
while others have non-false defaults; explicitly mention the exceptions by name
(UnmatchedRouteLabel defaults to `/__unmatched__`, and RequestDurationBuckets,
RequestSizeBuckets, ResponseSizeBuckets have non-false default bucket values) or
reword to "Most options default to `false`; the following have non-false
defaults: UnmatchedRouteLabel, RequestDurationBuckets, RequestSizeBuckets,
ResponseSizeBuckets" so the statement no longer contradicts the bullets.
- Around line 41-64: The README example mounts the Prometheus middleware only at
"/metrics" using app.Use("/metrics", ...), which prevents instrumentation of
other routes; change the example to register the middleware globally (call
app.Use(handler) with the fiberprometheus middleware returned by
fiberprometheus.New(...)) and then separately mount the metrics endpoint
(app.Use("/metrics", handler) or similar) so application routes ("/", "/ping",
"/some") are instrumented and metrics are exposed; also update the prose that
currently claims the middleware "continues to instrument all routed traffic" to
reflect that instrumentation only occurs when the middleware is registered
globally and correct the statement that "All of the options default to `false`"
to list the actual numeric defaults for RequestDurationBuckets,
RequestSizeBuckets, and ResponseSizeBuckets as shown in the config.
---
Nitpick comments:
In `@v3/prometheus/prometheus_test.go`:
- Around line 712-721: The test's exemplar bucket-label checks in
prometheus_test.go use brittle strings.Contains chains against metrics and
require the OpenMetrics float format le="256.0"; update the assertions around
requestLineWithExemplar and responseLineWithExemplar to be robust by matching
the bucket boundary with a regex that accepts either le="256" or le="256.0"
(e.g., le="256(\\.0)?") or by tokenizing each metric line via strings.Fields and
comparing structured label/key-value pairs instead of raw substring matches;
update the checks that currently call strings.Contains(line,
"...le=\"256.0\"...") to use the regex or parsed label comparison so they
tolerate formatting changes from client_golang while still asserting the
exemplar presence.
In `@v3/prometheus/prometheus.go`:
- Around line 403-409: stackSize currently only sums route counts which can miss
swaps and leave registeredRoutes stale when stack composition changes; update
stackSize (or the versioning check that uses it) to use a more reliable signal
such as Fiber's app.HandlersCount() or compute a cheap hash while iterating over
routes (e.g., incorporate r.Method and r.Path) so swaps change the version;
modify the logic that compares stackVersion/currentVersion to use the new
handlers count or the hash, ensuring functions and variables like stackSize,
registeredRoutes, stackVersion, currentVersion, and any code that mounts
sub-apps via Use(prefix, subApp) reflect the new version source.
- Around line 58-64: The code in prometheus.go silently lets cfg.Service
override any existing cfg.Labels["service"] when building labels (see the labels
map creation and the cfg.Service assignment); update the Service field's doc
comment in the config struct (the Service field in config.go) to explicitly
state that Service takes precedence and will overwrite any Labels["service"]
value so users are not surprised, and mention that they should set Service only
if they want to override the label or omit the key from Labels if they want the
label retained.
In `@v3/prometheus/README.md`:
- Around line 41-64: The README example mixes spaces and tabs; normalize the
indentation to Go convention (tabs) so the code block renders uniformly: update
the func main() example and its handlers (main(), app := fiber.New(),
app.Use(fiberprometheus.Config{...}), app.Get("/", ...), app.Get("/ping", ...),
app.Post("/some", ...), app.Listen(":3000")) to use tabs for all leading
indentation and remove the 8-space prefixes on the listed lines so every line
uses tabs consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c33d0e7e-5d96-4b48-a332-f8555b2753ad
⛔ Files ignored due to path filters (4)
.github/workflows/test-prometheus.ymlis excluded by!**/*.ymlgo.workis excluded by!**/*.work,!**/*.workv3/prometheus/go.modis excluded by!**/*.modv3/prometheus/go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (6)
README.mdv3/README.mdv3/prometheus/README.mdv3/prometheus/config.gov3/prometheus/prometheus.gov3/prometheus/prometheus_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: lint / lint (v3/prometheus)
- GitHub Check: lint / lint (v3/otel/example)
- GitHub Check: lint / lint (v3/newrelic)
- GitHub Check: lint / lint (v3/loadshed)
- GitHub Check: lint / lint (v3/otel)
- GitHub Check: lint / lint (v3/opa)
- GitHub Check: lint / lint (v3/jwt)
- GitHub Check: lint / lint (v3/circuitbreaker)
- GitHub Check: lint / lint (v3/hcaptcha)
- GitHub Check: lint / lint (v3/i18n)
- GitHub Check: Agent
🧰 Additional context used
🪛 LanguageTool
v3/prometheus/README.md
[style] ~90-~90: Consider removing “of” to be more concise
Context: ...62144 524288 1048576 2097152 5242880] All of the options default tofalse` and can be e...
(ALL_OF_THE)
🔇 Additional comments (5)
README.md (1)
36-36: LGTM.Index entry follows the established pattern for link target and workflow badge.
v3/README.md (1)
34-34: LGTM.Mirrors the top-level README index update with consistent link/badge format.
v3/prometheus/config.go (1)
133-187: Defaults & deep-copy logic looks good.The dual-path defaulting (zero-config vs. caller-supplied) and the deep-copies of
Labels,RequestDurationBuckets,RequestSizeBuckets,ResponseSizeBuckets,SkipURIs, andIgnoreStatusCodescorrectly insulate the package-level defaults from later mutation and prevent callers from observing shared backing arrays.strings.CloneonUnmatchedRouteLabelis a nice touch.One small consideration (not a defect): a caller-supplied non-nil but empty bucket slice (
[]float64{}) bypasses the nil check and would produce a histogram with only the implicit+Infbucket. The doc says "Provide nil to use the defaults," so this is by design — flagging just so it's a conscious choice.v3/prometheus/prometheus_test.go (1)
56-729: Test coverage is thorough.Strong end-to-end coverage of metrics emission, configuration knobs, OpenMetrics negotiation/exemplars, runtime collector toggles, custom registries, and HEAD handling. The
defer recover()pattern inTestRegistererWithoutGathererPanicscorrectly asserts the documented panic path.Note: if you adopt the suggested fix for the
DeleteLabelValuesrace inprometheus.go,TestIgnoreStatusCodesRemovesInFlightGauge(lines 200–218) will need to change from "metric absent" to "metric present with value 0," since dropping the delete is the correctness-preserving choice.v3/prometheus/prometheus.go (1)
212-226: Verify whatroute.Pathreturns forapp.Use(prefix, handler)registrations in Fiber v3-rc.3.The
isMetricsRequestmethod performs an exact-string comparison betweenroute.Pathandctx.Path()after normalizing trailing slashes. In Fiber v3, middleware registered viaapp.Use("/metrics", handler)may haveroute.Pathset to "/metrics/*" (with a wildcard suffix for prefix matching) rather than "/metrics", which would cause the equality check to fail and prevent metrics endpoint detection.The tests register the handler twice—once globally (
app.Use(handler)) and once with a prefix (app.Use(metricsPath, handler))—and the tests pass, but this dual registration means the global handler always catches the metrics request, masking whether the prefix-specific detection actually works. Confirm the actual value ofctx.Route().Pathfor prefix-mounted middleware and adjust the comparison logic to handle potential wildcard suffixes if needed (e.g., use prefix matching, strip wildcards, or store the configured metrics path at construction time).
| inflightPath := routePath | ||
| m.requestInFlight.WithLabelValues(method, inflightPath).Inc() | ||
| deleteGauge := false | ||
| defer func() { | ||
| m.requestInFlight.WithLabelValues(method, inflightPath).Dec() | ||
| if deleteGauge { | ||
| m.requestInFlight.DeleteLabelValues(method, inflightPath) | ||
| } | ||
| }() | ||
|
|
||
| start := time.Now() | ||
|
|
||
| err := ctx.Next() | ||
|
|
||
| if !registered && !trackUnmatched { | ||
| deleteGauge = true | ||
| return err | ||
| } | ||
|
|
||
| if _, ok := m.skipURIs[routePath]; ok { | ||
| deleteGauge = true | ||
| return err | ||
| } | ||
|
|
||
| status := fiber.StatusInternalServerError | ||
| if err != nil { | ||
| if e, ok := err.(*fiber.Error); ok { | ||
| status = e.Code | ||
| } | ||
| } else { | ||
| status = ctx.Response().StatusCode() | ||
| } | ||
|
|
||
| if _, ok := m.ignoreStatusCode[status]; ok { | ||
| deleteGauge = true | ||
| return err | ||
| } |
There was a problem hiding this comment.
DeleteLabelValues in the deferred cleanup creates a race that yields negative in-flight values.
Under concurrent load with two requests sharing the same (method, inflightPath) label set, where one is destined for skip/ignore (deleteGauge=true):
T1: Inc -> 1
T2: Inc -> 2
T1: Dec -> 1 (T2 still in flight)
T1: Delete -> entry removed
T2: Dec -> WithLabelValues recreates at 0, then Dec -> -1
The in-flight gauge will start emitting negative numbers, and the cleanup also corrupts T2's count. The same hazard applies on lines 256–264 and 275–278 anywhere deleteGauge is set while another goroutine has the matching label set in flight.
The simplest correct fix is to drop the deletion entirely — letting the gauge sit at 0 between requests is harmless and matches what the duration/size histograms already do. If lifecycle cleanup of stale labels is desired, it needs to be coordinated (e.g., a guarded counter per label that only deletes on the 0→0 transition, or periodic reaping outside the request path).
🔒 Suggested fix: stop deleting labels in the request path
inflightPath := routePath
m.requestInFlight.WithLabelValues(method, inflightPath).Inc()
- deleteGauge := false
defer func() {
m.requestInFlight.WithLabelValues(method, inflightPath).Dec()
- if deleteGauge {
- m.requestInFlight.DeleteLabelValues(method, inflightPath)
- }
}()
@@
if !registered && !trackUnmatched {
- deleteGauge = true
return err
}
if _, ok := m.skipURIs[routePath]; ok {
- deleteGauge = true
return err
}
@@
if _, ok := m.ignoreStatusCode[status]; ok {
- deleteGauge = true
return err
}Note: TestIgnoreStatusCodesRemovesInFlightGauge will need to be updated to assert … 0 instead of complete absence (since the metric will now persist with value 0), or the test should remove that strict expectation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| inflightPath := routePath | |
| m.requestInFlight.WithLabelValues(method, inflightPath).Inc() | |
| deleteGauge := false | |
| defer func() { | |
| m.requestInFlight.WithLabelValues(method, inflightPath).Dec() | |
| if deleteGauge { | |
| m.requestInFlight.DeleteLabelValues(method, inflightPath) | |
| } | |
| }() | |
| start := time.Now() | |
| err := ctx.Next() | |
| if !registered && !trackUnmatched { | |
| deleteGauge = true | |
| return err | |
| } | |
| if _, ok := m.skipURIs[routePath]; ok { | |
| deleteGauge = true | |
| return err | |
| } | |
| status := fiber.StatusInternalServerError | |
| if err != nil { | |
| if e, ok := err.(*fiber.Error); ok { | |
| status = e.Code | |
| } | |
| } else { | |
| status = ctx.Response().StatusCode() | |
| } | |
| if _, ok := m.ignoreStatusCode[status]; ok { | |
| deleteGauge = true | |
| return err | |
| } | |
| inflightPath := routePath | |
| m.requestInFlight.WithLabelValues(method, inflightPath).Inc() | |
| defer func() { | |
| m.requestInFlight.WithLabelValues(method, inflightPath).Dec() | |
| }() | |
| start := time.Now() | |
| err := ctx.Next() | |
| if !registered && !trackUnmatched { | |
| return err | |
| } | |
| if _, ok := m.skipURIs[routePath]; ok { | |
| return err | |
| } | |
| status := fiber.StatusInternalServerError | |
| if err != nil { | |
| if e, ok := err.(*fiber.Error); ok { | |
| status = e.Code | |
| } | |
| } else { | |
| status = ctx.Response().StatusCode() | |
| } | |
| if _, ok := m.ignoreStatusCode[status]; ok { | |
| return err | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@v3/prometheus/prometheus.go` around lines 242 - 278, The deferred cleanup in
the middleware uses m.requestInFlight.DeleteLabelValues with a deleteGauge flag
(controlled in the request path where deleteGauge is set) which races under
concurrent requests and can produce negative in-flight counts; remove the
DeleteLabelValues call and the deleteGauge plumbing (keep the Inc()/Dec() calls
on m.requestInFlight.WithLabelValues(method, inflightPath)) so labels remain
with value 0 instead of being removed, and update tests (e.g.,
TestIgnoreStatusCodesRemovesInFlightGauge) to expect a 0-valued metric rather
than absence.
| func main() { | ||
| app := fiber.New() | ||
|
|
||
| app.Use("/metrics", fiberprometheus.New(fiberprometheus.Config{ | ||
| Service: "my-service-name", | ||
| SkipURIs: []string{"/ping"}, | ||
| IgnoreStatusCodes: []int{401, 403, 404}, | ||
| })) | ||
|
|
||
| app.Get("/", func(c fiber.Ctx) error { | ||
| return c.SendString("Hello World") | ||
| }) | ||
|
|
||
| app.Get("/ping", func(c fiber.Ctx) error { | ||
| return c.SendString("pong") | ||
| }) | ||
|
|
||
| app.Post("/some", func(c fiber.Ctx) error { | ||
| return c.SendString("Welcome!") | ||
| }) | ||
|
|
||
| app.Listen(":3000") | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Fiber v3, does app.Use("/prefix", handler) execute the handler for requests outside that prefix?
💡 Result:
No, in Fiber v3, app.Use("/prefix", handler) does not execute the handler for requests outside that prefix. The prefix limits the middleware/handler to paths that begin with the specified prefix, with stricter matching requiring an exact match or slash boundary (e.g., "/api" matches "/api" and "/api/users" but not "/apiv2").
Citations:
- 1: https://docs.gofiber.io/guide/routing/
- 2: https://docs.gofiber.io/whats_new/
- 3: https://docs.gofiber.io/next/guide/routing
- 4: https://docs.gofiber.io/blog/fiber-v3-adapter-pattern/
- 5: https://docs.gofiber.io/next/api/app/
🏁 Script executed:
# Find the Prometheus test files to verify the double-mount pattern claim
find . -path "*/prometheus/*" -name "*.go" -o -name "*.md" | head -20Repository: gofiber/contrib
Length of output: 522
🏁 Script executed:
# Also check the README.md file location and content
find . -path "*/prometheus/README.md"Repository: gofiber/contrib
Length of output: 85
🏁 Script executed:
# Search for newAppWithMiddleware to verify the double-mount pattern
rg "newAppWithMiddleware" --type goRepository: gofiber/contrib
Length of output: 2137
🏁 Script executed:
# Extract the newAppWithMiddleware function implementation
sed -n '/^func newAppWithMiddleware/,/^}/p' v3/prometheus/prometheus_test.goRepository: gofiber/contrib
Length of output: 315
🏁 Script executed:
# Now check the README.md file around line 91 and the example code
cat -n v3/prometheus/README.md | sed -n '85,100p'Repository: gofiber/contrib
Length of output: 1053
🏁 Script executed:
# Check the indentation of the README example (lines 41-64)
cat -n v3/prometheus/README.md | sed -n '41,65p'Repository: gofiber/contrib
Length of output: 836
Example only mounts at /metrics — other routes won't be instrumented.
The example uses app.Use("/metrics", handler) which, in Fiber v3, limits the middleware to requests under the /metrics prefix only. Requests to /, /ping, and /some never reach the middleware, so:
- No counters/histograms/in-flight metrics are recorded for any application route.
Service,SkipURIs, andIgnoreStatusCodesin the example are effectively dead options./metricswould expose only the Go/process collectors.
The test suite's newAppWithMiddleware (prometheus_test.go) registers the handler twice: app.Use(handler) for global instrumentation, then app.Use(metricsPath, handler) for the metrics endpoint. The README must follow the same pattern.
Additionally, lines 93–96 claim the middleware "continues to instrument all routed traffic," which is false given the single prefix-mount behavior. Line 91 also incorrectly states "All of the options default to false" when RequestDurationBuckets, RequestSizeBuckets, and ResponseSizeBuckets have explicit numeric array defaults (lines 86–89).
📝 Suggested example fix
func main() {
app := fiber.New()
- app.Use("/metrics", fiberprometheus.New(fiberprometheus.Config{
+ prom := fiberprometheus.New(fiberprometheus.Config{
Service: "my-service-name",
SkipURIs: []string{"/ping"},
IgnoreStatusCodes: []int{401, 403, 404},
- }))
+ })
+
+ // Instrument every request.
+ app.Use(prom)
+ // Expose the metrics endpoint at /metrics.
+ app.Use("/metrics", prom)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func main() { | |
| app := fiber.New() | |
| app.Use("/metrics", fiberprometheus.New(fiberprometheus.Config{ | |
| Service: "my-service-name", | |
| SkipURIs: []string{"/ping"}, | |
| IgnoreStatusCodes: []int{401, 403, 404}, | |
| })) | |
| app.Get("/", func(c fiber.Ctx) error { | |
| return c.SendString("Hello World") | |
| }) | |
| app.Get("/ping", func(c fiber.Ctx) error { | |
| return c.SendString("pong") | |
| }) | |
| app.Post("/some", func(c fiber.Ctx) error { | |
| return c.SendString("Welcome!") | |
| }) | |
| app.Listen(":3000") | |
| } | |
| ``` | |
| func main() { | |
| app := fiber.New() | |
| prom := fiberprometheus.New(fiberprometheus.Config{ | |
| Service: "my-service-name", | |
| SkipURIs: []string{"/ping"}, | |
| IgnoreStatusCodes: []int{401, 403, 404}, | |
| }) | |
| // Instrument every request. | |
| app.Use(prom) | |
| // Expose the metrics endpoint at /metrics. | |
| app.Use("/metrics", prom) | |
| app.Get("/", func(c fiber.Ctx) error { | |
| return c.SendString("Hello World") | |
| }) | |
| app.Get("/ping", func(c fiber.Ctx) error { | |
| return c.SendString("pong") | |
| }) | |
| app.Post("/some", func(c fiber.Ctx) error { | |
| return c.SendString("Welcome!") | |
| }) | |
| app.Listen(":3000") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@v3/prometheus/README.md` around lines 41 - 64, The README example mounts the
Prometheus middleware only at "/metrics" using app.Use("/metrics", ...), which
prevents instrumentation of other routes; change the example to register the
middleware globally (call app.Use(handler) with the fiberprometheus middleware
returned by fiberprometheus.New(...)) and then separately mount the metrics
endpoint (app.Use("/metrics", handler) or similar) so application routes ("/",
"/ping", "/some") are instrumented and metrics are exposed; also update the
prose that currently claims the middleware "continues to instrument all routed
traffic" to reflect that instrumentation only occurs when the middleware is
registered globally and correct the statement that "All of the options default
to `false`" to list the actual numeric defaults for RequestDurationBuckets,
RequestSizeBuckets, and ResponseSizeBuckets as shown in the config.
| - Request size: `[256 512 1024 2048 4096 8192 16384 32768 65536 131072 262144 524288 1048576 2097152 5242880]` | ||
| - Response size: `[256 512 1024 2048 4096 8192 16384 32768 65536 131072 262144 524288 1048576 2097152 5242880]` | ||
|
|
||
| All of the options default to `false` and can be enabled or disabled individually as needed. |
There was a problem hiding this comment.
Inaccurate defaults statement.
The bullets immediately above this line include UnmatchedRouteLabel (defaults to /__unmatched__), RequestDurationBuckets, RequestSizeBuckets, and ResponseSizeBuckets (all non-false defaults), so "All of the options default to false" is incorrect and contradicts the surrounding text.
📝 Proposed wording
-All of the options default to `false` and can be enabled or disabled individually as needed.
+All boolean toggles default to `false` and can be enabled individually as needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@v3/prometheus/README.md` at line 91, The sentence "All of the options default
to `false`" is incorrect — update the README text to accurately state that some
options default to false while others have non-false defaults; explicitly
mention the exceptions by name (UnmatchedRouteLabel defaults to
`/__unmatched__`, and RequestDurationBuckets, RequestSizeBuckets,
ResponseSizeBuckets have non-false default bucket values) or reword to "Most
options default to `false`; the following have non-false defaults:
UnmatchedRouteLabel, RequestDurationBuckets, RequestSizeBuckets,
ResponseSizeBuckets" so the statement no longer contradicts the bullets.
All original changes preserved from the same branch.
Summary by CodeRabbit