feat: Migrate monitor middleware from Fiber v2 to v3 #3125
Conversation
Redish101
requested review from
gaby,
sixcolors,
ReneWerner87 and
efectn
and removed request for
a team
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
WalkthroughThe changes introduce a monitoring middleware for a Fiber web application, encompassing configuration management, a web-based interface, and real-time statistics retrieval. Key components include a configuration structure, an HTML dashboard for metrics visualization, and a middleware function that periodically updates system statistics. This implementation enhances the application's monitoring capabilities by providing insights into resource usage. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Middleware
participant StatsUpdater
participant HTMLGenerator
User->>Middleware: Request monitoring data
Middleware->>StatsUpdater: Fetch latest statistics
StatsUpdater-->>Middleware: Return statistics
Middleware->>HTMLGenerator: Generate HTML page with stats
HTMLGenerator-->>Middleware: Return HTML page
Middleware-->>User: Respond with monitoring interface
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
Files selected for processing (3)
- middleware/monitor/config.go (1 hunks)
- middleware/monitor/index.go (1 hunks)
- middleware/monitor/monitor.go (1 hunks)
Additional context used
golangci-lint
middleware/monitor/config.go
10-10: fieldalignment: struct with 96 pointer bytes could be 80
(govet)
middleware/monitor/index.go
9-9: fieldalignment: struct with 64 pointer bytes could be 56
(govet)
Additional comments not posted (3)
middleware/monitor/config.go (1)
66-132: Review ofconfigDefaultfunction: Well-implemented logic.The
configDefaultfunction is well-implemented, with clear handling of default configurations and conditional updates based on changes to the configuration. The logic to ensure that the refresh rate does not fall below the minimum value is particularly noteworthy.middleware/monitor/monitor.go (1)
18-35: Struct definitions forstats,statsPID, andstatsOS: Appropriately defined.The struct definitions are well-suited for their purpose in structuring the JSON response for the monitoring API. The use of appropriate data types for the metrics ensures that the data is accurately represented.
middleware/monitor/index.go (1)
17-270: Review ofnewIndexfunction and HTML template: Well-implemented and secure.The
newIndexfunction efficiently generates the HTML index page usingstrings.NewReplacer, which is a good practice for preventing templating errors. The HTML template is well-structured and appears secure against XSS vulnerabilities, as it does not directly embed user input.
| // Config defines the config for middleware. | ||
| type Config struct { | ||
| // Metrics page title | ||
| // | ||
| // Optional. Default: "Fiber Monitor" | ||
| Title string | ||
|
|
||
| // Refresh period | ||
| // | ||
| // Optional. Default: 3 seconds | ||
| Refresh time.Duration | ||
|
|
||
| // Whether the service should expose only the monitoring API. | ||
| // | ||
| // Optional. Default: false | ||
| APIOnly bool | ||
|
|
||
| // Next defines a function to skip this middleware when returned true. | ||
| // | ||
| // Optional. Default: nil | ||
| Next func(c *fiber.Ctx) bool | ||
|
|
||
| // Custom HTML Code to Head Section(Before End) | ||
| // | ||
| // Optional. Default: empty | ||
| CustomHead string | ||
|
|
||
| // FontURL for specify font resource path or URL . also you can use relative path | ||
| // | ||
| // Optional. Default: https://fonts.googleapis.com/css2?family=Roboto:wght@400;900&display=swap | ||
| FontURL string | ||
|
|
||
| // ChartJsURL for specify ChartJS library path or URL . also you can use relative path | ||
| // | ||
| // Optional. Default: https://cdn.jsdelivr.net/npm/[email protected]/dist/Chart.bundle.min.js | ||
| ChartJsURL string // TODO: Rename to "ChartJSURL" in v3 | ||
|
|
||
| index string | ||
| } |
There was a problem hiding this comment.
Optimize struct field alignment.
The Config struct could be optimized for memory usage by rearranging its fields. This change can potentially reduce the struct's memory footprint, improving performance especially in environments where many instances of this struct might be created.
Consider rearranging the fields to optimize alignment. Here's a suggested rearrangement:
type Config struct {
Next func(c *fiber.Ctx) bool // Move function pointers to the top
Title string
CustomHead string
FontURL string
ChartJsURL string
Refresh time.Duration
APIOnly bool
index string
}This rearrangement groups the larger string and time.Duration types together, placing the smaller bool and function pointer types appropriately to fit better in memory alignment slots.
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.
| // Config defines the config for middleware. | |
| type Config struct { | |
| // Metrics page title | |
| // | |
| // Optional. Default: "Fiber Monitor" | |
| Title string | |
| // Refresh period | |
| // | |
| // Optional. Default: 3 seconds | |
| Refresh time.Duration | |
| // Whether the service should expose only the monitoring API. | |
| // | |
| // Optional. Default: false | |
| APIOnly bool | |
| // Next defines a function to skip this middleware when returned true. | |
| // | |
| // Optional. Default: nil | |
| Next func(c *fiber.Ctx) bool | |
| // Custom HTML Code to Head Section(Before End) | |
| // | |
| // Optional. Default: empty | |
| CustomHead string | |
| // FontURL for specify font resource path or URL . also you can use relative path | |
| // | |
| // Optional. Default: https://fonts.googleapis.com/css2?family=Roboto:wght@400;900&display=swap | |
| FontURL string | |
| // ChartJsURL for specify ChartJS library path or URL . also you can use relative path | |
| // | |
| // Optional. Default: https://cdn.jsdelivr.net/npm/[email protected]/dist/Chart.bundle.min.js | |
| ChartJsURL string // TODO: Rename to "ChartJSURL" in v3 | |
| index string | |
| } | |
| // Config defines the config for middleware. | |
| type Config struct { | |
| Next func(c *fiber.Ctx) bool // Move function pointers to the top | |
| Title string | |
| CustomHead string | |
| FontURL string | |
| ChartJsURL string | |
| Refresh time.Duration | |
| APIOnly bool | |
| index string | |
| } |
Tools
golangci-lint
10-10: fieldalignment: struct with 96 pointer bytes could be 80
(govet)
| // New creates a new middleware handler | ||
| func New(config ...Config) fiber.Handler { | ||
| // Set default config | ||
| cfg := configDefault(config...) | ||
|
|
||
| // Start routine to update statistics | ||
| once.Do(func() { | ||
| p, _ := process.NewProcess(int32(os.Getpid())) //nolint:errcheck // TODO: Handle error | ||
| numcpu := runtime.NumCPU() | ||
| updateStatistics(p, numcpu) | ||
|
|
||
| go func() { | ||
| for { | ||
| time.Sleep(cfg.Refresh) | ||
|
|
||
| updateStatistics(p, numcpu) | ||
| } | ||
| }() | ||
| }) | ||
|
|
||
| // Return new handler | ||
| //nolint:errcheck // Ignore the type-assertion errors | ||
| return func(c fiber.Ctx) error { | ||
| // Don't execute middleware if Next returns true | ||
| if cfg.Next != nil && cfg.Next(&c) { | ||
| return c.Next() | ||
| } | ||
|
|
||
| if c.Method() != fiber.MethodGet { | ||
| return fiber.ErrMethodNotAllowed | ||
| } | ||
| if c.Get(fiber.HeaderAccept) == fiber.MIMEApplicationJSON || cfg.APIOnly { | ||
| mutex.Lock() | ||
| data.PID.CPU, _ = monitPIDCPU.Load().(float64) | ||
| data.PID.RAM, _ = monitPIDRAM.Load().(uint64) | ||
| data.PID.Conns, _ = monitPIDConns.Load().(int) | ||
|
|
||
| data.OS.CPU, _ = monitOSCPU.Load().(float64) | ||
| data.OS.RAM, _ = monitOSRAM.Load().(uint64) | ||
| data.OS.TotalRAM, _ = monitOSTotalRAM.Load().(uint64) | ||
| data.OS.LoadAvg, _ = monitOSLoadAvg.Load().(float64) | ||
| data.OS.Conns, _ = monitOSConns.Load().(int) | ||
| mutex.Unlock() | ||
| return c.Status(fiber.StatusOK).JSON(data) | ||
| } | ||
| c.Set(fiber.HeaderContentType, fiber.MIMETextHTMLCharsetUTF8) | ||
| return c.Status(fiber.StatusOK).SendString(cfg.index) | ||
| } | ||
| } |
There was a problem hiding this comment.
Review of New function: Implement error handling.
The New function correctly uses sync.Once to ensure that the statistics updating routine is started only once, which is a good practice. However, the error from process.NewProcess is not handled, which could lead to uncaught exceptions in production environments.
Consider adding error handling for the process creation to ensure stability:
p, err := process.NewProcess(int32(os.Getpid()))
if err != nil {
return nil, err // Handle the error appropriately
}This change will help prevent potential runtime errors and improve the robustness of the middleware.
Committable suggestion was skipped due to low confidence.
|
@Redish101 The monitor middleware was removed from v3 on purpose. The end goal is to migrate it to the gofiber/contrib repo. |
Well, I will move these changes to gofiber/contrib repo. |
Thanks, one thing. You have to fix the gopsutil version, it should be this one https://github.com/shirou/gopsutil/releases/tag/v4.24.8 in the |
I have moved these changes to gofiber/contrib#1172 and updated gopsutils to v4.24.8. |
Description
Migrate monitor middleware from fiber v2 to v3.
Type of change
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/directory for Fiber's documentation.