Cleanup agent logging, increase default rdap retries

mrheinen commented 1 month ago

PR Type

Enhancement, Bug fix

Description

Implemented structured logging using slog throughout the agent and HTTP server, replacing fmt.Printf and log.Printf calls. This improves log consistency and provides better context for debugging.
Increased the default number of RDAP lookup attempts from 3 to 6 in both the Go configuration struct and the YAML configuration file. This change aims to improve reliability when dealing with flaky RDAP services.
Improved error reporting in test cases by replacing fmt.Printf with t.Errorf for better test output.
Removed unnecessary debug logging in the SendContext function of the agent.
Cleaned up imports by removing unused "fmt" import in the agent test file.

Changes walkthrough 📝

Relevant files

Cleanup

agent.go

Remove debug logging in SendContext function

pkg/agent/agent.go

Removed a debug print statement that was logging the context being
sent

+0/-2

Enhancement

agent_test.go `Improve test error reporting` pkg/agent/agent_test.go Replaced a `fmt.Printf` statement with `t.Errorf` in a test case Removed an unused import of "fmt"	+1/-3
http_server.go `Implement structured logging in HTTP server` pkg/agent/http_server.go Replaced `fmt.Printf` and `log.Printf` calls with structured logging using `slog` Improved error logging with more context Changed debug print to use `slog.Debug`	+4/-5

Configuration changes

config.go `Increase default RDAP lookup attempts` pkg/backend/config.go Increased the default value of `MaxAttempts` in the `WhoisManager` configuration from 3 to 6	+1/-1
backend-config.yaml `Update RDAP max attempts in configuration` config/backend-config.yaml - Updated the `max_attempts` value under `whois_manager` from 3 to 6	+1/-1

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

codiumai-pr-agent-pro[bot] commented 1 month ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review

Error Handling
The error handling for the request body parsing could be improved. Currently, it logs the error but continues execution, which might lead to unexpected behavior. Inconsistent Logging
There's an inconsistency in error logging. Some errors use `slog.Error` while others still use `log.Printf`. This should be standardized for better log consistency.

github-actions[bot] commented 1 month ago

Preparing PR description...

github-actions[bot] commented 1 month ago

Preparing review...

codiumai-pr-agent-pro[bot] commented 1 month ago

PR Code Suggestions ✨

Category	Suggestion	Score
Security	Add a size check for the request body to prevent potential out-of-memory issues ___ Consider adding a check for the length of the body before reading it entirely into memory. This can prevent potential out-of-memory issues for very large request bodies. [pkg/agent/http_server.go [177-184]](https://github.com/mrheinen/lophiid/pull/43/files#diff-bd72f74c6c9e289f9aa548543a745b6a7a821fa744c26d2ba9325c7ae9179cffR177-R184) ```diff if r.Body != nil { + if r.ContentLength > 1010241024 { // 10MB limit + slog.Warn("Request body too large", slog.Int64("content_length", r.ContentLength)) + w.WriteHeader(http.StatusRequestEntityTooLarge) + return + } b, err := io.ReadAll(r.Body) if err != nil { slog.Error("unable to parse body", slog.String("error", err.Error())) } else { pr.Request.Body = b } } ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 9 Why: This suggestion addresses a potential security vulnerability by preventing out-of-memory issues from large request bodies, which is crucial for system stability and security.	9
Enhancement	Enhance error logging for nil responses with more context and set an appropriate HTTP status code ___ Consider adding more context to the error log when the response or probe request is nil. This will help in debugging and understanding why the response is nil. [pkg/agent/http_server.go [197-200]](https://github.com/mrheinen/lophiid/pull/43/files#diff-bd72f74c6c9e289f9aa548543a745b6a7a821fa744c26d2ba9325c7ae9179cffR197-R200) ```diff if res == nil \|\| res.Response == nil { - slog.Error("got nil!!", slog.String("response", fmt.Sprintf("%+v", res)), slog.String("probe_request", fmt.Sprintf("%+v", pr))) + slog.Error("Received nil response", + slog.Any("response", res), + slog.String("request_uri", pr.RequestUri), + slog.String("method", r.Method), + slog.String("remote_addr", r.RemoteAddr)) + w.WriteHeader(http.StatusInternalServerError) return } ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion significantly improves error handling by adding more context to the log and setting an appropriate HTTP status code, which is valuable for debugging and proper client communication.	8
Enhancement	Improve error logging with more specific and structured information ___ Consider using a more specific error message that includes the request details. This will make debugging easier in case of issues. [pkg/agent/http_server.go [128-130]](https://github.com/mrheinen/lophiid/pull/43/files#diff-bd72f74c6c9e289f9aa548543a745b6a7a821fa744c26d2ba9325c7ae9179cffR128-R130) ```diff if err != nil { - slog.Error("Problem decoding requests", slog.String("error", err.Error()), slog.String("request", fmt.Sprintf("%+#v", r))) + slog.Error("Failed to dump HTTP request", + slog.String("error", err.Error()), + slog.String("method", r.Method), + slog.String("url", r.URL.String()), + slog.String("remote_addr", r.RemoteAddr)) } ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion improves error logging by providing more specific and structured information, which can aid in debugging and troubleshooting.	7

mrheinen / lophiid