Warn users to remember adding endpoint to Checkmate Infrastructure Dashboard #59
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughA new helper function, Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
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. 🪧 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: 0
🧹 Nitpick comments (2)
cmd/capture/main.go (2)
25-42: ThegetLocalIP()function works well but has some limitations.The function correctly retrieves the first non-loopback IPv4 address. However, it has a couple of limitations:
- On machines with multiple network interfaces, it will always return the first non-loopback IPv4 address found, which might not be the desired one for connecting to the Checkmate dashboard.
- It doesn't support IPv6 addresses, which could be problematic in IPv6-only environments.
Consider enhancing the function to handle multiple network interfaces and IPv6 addresses:
func getLocalIP() string { addrs, err := net.InterfaceAddrs() if err != nil { return "<ip-address>" } // First try to find an IPv4 address for _, addr := range addrs { if ipnet, ok := addr.(*net.IPNet); ok && !ipnet.IP.IsLoopback() { if ipnet.IP.To4() != nil { return ipnet.IP.String() } } } + // If no IPv4 address is found, try IPv6 + for _, addr := range addrs { + if ipnet, ok := addr.(*net.IPNet); ok && !ipnet.IP.IsLoopback() { + if ipnet.IP.To4() == nil { + return "[" + ipnet.IP.String() + "]" // IPv6 addresses should be enclosed in brackets + } + } + } return "<ip-address>" }
75-75: The warning message is helpful but could be more informative.The log statement correctly warns users to add the endpoint to their Checkmate dashboard. However, it doesn't provide context about why this is important or what will happen if they don't add it.
Consider enhancing the log message to be more informative:
-log.Println("warning: Remember to add http://" + getLocalIP() + ":" + appConfig.Port + "/api/v1/metrics to your Checkmate dashboard") +log.Println("⚠️ WARNING: For proper monitoring, add http://" + getLocalIP() + ":" + appConfig.Port + "/api/v1/metrics to your Checkmate Infrastructure Dashboard. Without this endpoint, system metrics will not be displayed.")
…frastructure Dashboard Signed-off-by: Mert Şişmanoğlu <[email protected]>
mertssmnoglu
force-pushed
the
feat/logging-warning
branch
from
fb51757 to
1761440
Compare
Users may forgot to add
http://ip-address:port/api/v1/metricsendpoint to their Checkmate Infrastructure Dashboard.Created a function to retreive local ip address of the machine. Displaying a warning on startup.
Summary by CodeRabbit