Skip to content

feat(monitor): add metrics, new features, and fix middleware bugs#1833

Merged
ReneWerner87 merged 15 commits into
mainfrom
copilot/add-monitor-middleware-absolute-counter
Jun 10, 2026
Merged

feat(monitor): add metrics, new features, and fix middleware bugs#1833
ReneWerner87 merged 15 commits into
mainfrom
copilot/add-monitor-middleware-absolute-counter

Conversation

@ReneWerner87

@ReneWerner87 ReneWerner87 commented Apr 25, 2026

Copy link
Copy Markdown
Member

Reopened from #1820 — original was closed due to a broken force-push that orphaned main's history (now restored).

All original changes preserved from the same branch.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added uptime metric to the monitoring dashboard.
    • Added Total Requests counter and Goroutines metric with dedicated chart displays.
  • Documentation

    • Enhanced middleware README with clarified configuration guidance for conditional request tracking and counting application-wide requests.
  • Tests

    • Improved test coverage for JSON response validation and request counting functionality.

Copilot AI and others added 13 commits April 18, 2026 10:00
…nic on process error, APIOnly override logic

Agent-Logs-Url: https://github.com/gofiber/contrib/sessions/6da9d492-f179-4c24-9e49-d2afbdb03db7

Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
…nil-safe process, APIOnly default, improved test

Agent-Logs-Url: https://github.com/gofiber/contrib/sessions/5107777e-20e7-4972-a097-544f30f8fda9

Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
…bal tests, typed JSON assertions, close resp.Body

Agent-Logs-Url: https://github.com/gofiber/contrib/sessions/50b76845-c4b0-4416-8204-1a577a58c8dc

Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Removed comments regarding APIOnly inheritance behavior.
Copilot AI review requested due to automatic review settings April 25, 2026 11:48
@ReneWerner87 ReneWerner87 requested a review from a team as a code owner April 25, 2026 11:48
@ReneWerner87 ReneWerner87 requested review from efectn, gaby and sixcolors and removed request for a team April 25, 2026 11:48
@coderabbitai

coderabbitai Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR refactors the v3 monitor middleware to replace mutex-protected shared data with atomic.Value storage, adds three new metrics (Requests, Goroutines, Uptime) with frontend visualization, improves test coverage for request counting, and documents the application-wide counting pattern via the Next configuration option.

Changes

Monitor Middleware: Atomic Metrics & Dashboard Expansion

Layer / File(s) Summary
Backend metrics data structures
v3/monitor/monitor.go
Expanded statsPID struct to track Goroutines, Requests (string-encoded), and Uptime; replaced shared mutex-protected data with per-metric atomic.Value globals including a new atomic.Uint64 request counter.
Initialization with safe defaults and process tracking
v3/monitor/monitor.go
Updated New() to pre-initialize all atomic metrics with zero defaults to prevent panics on early Load(); captured process start time from gopsutil with fallback to middleware init time and nil-process handling.
Request handling and metrics updates
v3/monitor/monitor.go
Incremented global request counter before Next filter to count all app-wide requests; refactored response to build on-demand snapshots by atomically loading metrics and computing Uptime; added goroutine counting to updateStatistics.
Frontend dashboard components and visualization
v3/monitor/index.go
Added HTML/CSS for uptime display and two new metric cards (Total Requests, Goroutines) with canvases; implemented formatUptime() helper; extended DOM wiring and chart management; reworked update() function with BigInt-based request delta calculation (handling restarts and precision limits) to populate all three new metrics.
Test coverage for request counting and JSON validation
v3/monitor/monitor_test.go, v3/monitor/config_test.go
Refactored Test_Monitor_JSON to decode JSON into typed struct and validate field ranges; added Test_Monitor_Requests and Test_Monitor_Requests_NextFiltered to validate request counter behavior across endpoint calls and filtered routes; added Test_Config_Default_APIOnly_Global for config inheritance.
Configuration and usage documentation
v3/monitor/README.md
Documented Next as a predicate to conditionally skip middleware; added "Counting all application requests" section with example of mounting app-wide with Next filtering to restrict dashboard to /metrics path.

Sequence Diagram

sequenceDiagram
  participant Client
  participant FiberApp
  participant MonitorHandler as Monitor Handler
  participant AtomicMetrics as Atomic Metrics
  participant Dashboard as Dashboard/API
  participant Frontend as Frontend JavaScript
  
  Client->>FiberApp: Request to any path
  FiberApp->>MonitorHandler: Invoke monitor middleware
  MonitorHandler->>AtomicMetrics: Increment request counter
  MonitorHandler->>MonitorHandler: Check Next filter
  alt Next returns true (skip)
    MonitorHandler->>FiberApp: Continue to next handler
  else Next returns false (show dashboard)
    MonitorHandler->>AtomicMetrics: Load PID metrics (requests, goroutines, uptime)
    MonitorHandler->>AtomicMetrics: Load OS metrics
    MonitorHandler->>Dashboard: Return JSON snapshot
    Dashboard->>Client: JSON response
    Client->>Frontend: Parse JSON data
    Frontend->>Frontend: Calculate request delta (BigInt)
    Frontend->>Frontend: Update metric displays & charts
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • gofiber/contrib#1172: Prior v3 monitor middleware migration that this PR extends with atomic metrics refactoring, request/goroutine/uptime field additions, and comprehensive test coverage improvements.

Suggested labels

🧹 Updates

Suggested reviewers

  • gaby
  • sixcolors
  • efectn

Poem

🐰 Hops into metrics with atomic grace,
Tracking requests in their rightful place,
Goroutines dance, uptime takes the stage,
Dashboard gleams on every page!
No mutex locks can slow this race—
Atoms keep the metrics safe!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: adding metrics (requests counter, goroutines, uptime), new features (Next filtering, uptime display), and fixing middleware bugs (atomic storage, panic handling, initialization).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch copilot/add-monitor-middleware-absolute-counter

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.12.2)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds new monitor metrics (requests, goroutines, uptime), improves JSON safety/precision, and extends UI/tests/workflow coverage for the monitor middleware.

Changes:

  • Add pid.requests (uint64 encoded as string), pid.goroutines, and pid.uptime to the monitor JSON payload.
  • Update the dashboard UI to display/chart requests and goroutines, and show uptime.
  • Extend monitor tests and CI matrix (adds Go 1.26.x) to cover the new fields and behaviors.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
v3/monitor/monitor.go Adds atomic-backed request counter, uptime/goroutine metrics, and snapshot-based JSON response generation.
v3/monitor/index.go Adds new UI elements and charts for requests/goroutines and an uptime display; updates JS sampling logic.
v3/monitor/monitor_test.go Adds typed JSON assertions, request counter test, and improves response body handling.
v3/monitor/config_test.go Adds coverage for ConfigDefault.APIOnly inheritance behavior.
.github/workflows/test-monitor.yml Expands Go test matrix to include 1.26.x.
v3/swaggerui/go.mod Removes trailing blank line at EOF.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread v3/monitor/config_test.go
Comment thread v3/monitor/index.go
Comment thread v3/monitor/monitor_test.go Outdated

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances the monitor middleware by adding Uptime, Total Requests, and Goroutine metrics, along with corresponding dashboard UI updates. It improves robustness by initializing atomic values to prevent panics and handling cases where process information is unavailable. Feedback indicates that the "Total Requests" counter is misleading as it only tracks requests to the monitor endpoint rather than the entire application, and notes potential performance concerns regarding frequent connection scanning.

Comment thread v3/monitor/monitor.go Outdated
Comment thread v3/monitor/monitor.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread v3/monitor/index.go Outdated
ReneWerner87 and others added 2 commits June 10, 2026 21:25
- Count requests before the Next check so the middleware can be mounted
  app-wide as a traffic collector; document the pattern in the README
- Use textContent instead of innerHTML for plain-text dashboard metrics
- Clamp the requests chart delta to [0, MAX_SAFE_INTEGER] to handle
  server restarts and uint64 precision loss in JavaScript
- Replace defer Body.Close() with t.Cleanup assertions, dropping the
  nolint suppression
- Add a test covering counting of Next-filtered requests
- Fix the Next description in the README config table

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
v3/monitor/monitor.go (1)

108-115: Next skips monitor rendering but still increments “Total Requests” (intentional, documented + tested)

  • monitTotalRequests.Add(1) runs before the cfg.Next short-circuit, so requests returning true are still counted; v3/monitor/README.md and Test_Monitor_Requests_NextFiltered describe this behavior.
  • The inline comment in v3/monitor/monitor.go (“Don’t execute middleware if Next returns true”) is slightly misleading because only the monitor response logic is skipped, not the counter increment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@v3/monitor/monitor.go` around lines 108 - 115, The inline comment in
monitor.go is misleading: monitTotalRequests.Add(1) intentionally runs before
the cfg.Next short-circuit so filtered requests are still counted; update the
comment near monitTotalRequests.Add(1) and the subsequent if cfg.Next != nil &&
cfg.Next(c) block to explicitly state that cfg.Next only prevents the middleware
response/rendering but does NOT prevent the total-request counter from
incrementing (reference symbols: monitTotalRequests.Add and cfg.Next).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@v3/monitor/monitor.go`:
- Around line 108-115: The inline comment in monitor.go is misleading:
monitTotalRequests.Add(1) intentionally runs before the cfg.Next short-circuit
so filtered requests are still counted; update the comment near
monitTotalRequests.Add(1) and the subsequent if cfg.Next != nil && cfg.Next(c)
block to explicitly state that cfg.Next only prevents the middleware
response/rendering but does NOT prevent the total-request counter from
incrementing (reference symbols: monitTotalRequests.Add and cfg.Next).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7b66004b-4e09-4e71-89fd-367de4bba70b

📥 Commits

Reviewing files that changed from the base of the PR and between 827ccac and e16dc1f.

⛔ Files ignored due to path filters (1)
  • .github/workflows/test-monitor.yml is excluded by !**/*.yml
📒 Files selected for processing (5)
  • v3/monitor/README.md
  • v3/monitor/config_test.go
  • v3/monitor/index.go
  • v3/monitor/monitor.go
  • v3/monitor/monitor_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). (1)
  • GitHub Check: Analyze (go)
🔇 Additional comments (13)
v3/monitor/README.md (2)

41-41: LGTM!


73-84: LGTM!

v3/monitor/monitor_test.go (4)

5-5: LGTM!

Also applies to: 9-9


127-157: LGTM!


226-271: LGTM!


273-304: LGTM!

v3/monitor/config_test.go (1)

165-184: LGTM!

v3/monitor/index.go (5)

96-102: LGTM!

Also applies to: 108-108, 146-163


178-186: LGTM!


216-218: LGTM!

Also applies to: 224-225, 231-232, 234-234


265-265: LGTM!

Also applies to: 285-295


267-268: LGTM!

Also applies to: 275-277, 296-296

v3/monitor/monitor.go (1)

69-103: LGTM!

Also applies to: 122-153, 160-195

@ReneWerner87 ReneWerner87 merged commit 5f1aef3 into main Jun 10, 2026
13 checks passed
@ReneWerner87 ReneWerner87 deleted the copilot/add-monitor-middleware-absolute-counter branch June 10, 2026 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✏️ Feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants