-
Notifications
You must be signed in to change notification settings - Fork 3
Closed as not planned
Closed as not planned
Copy link
Description
🔍 Duplicate Code Pattern: RPC Message Logging Functions
Part of duplicate code analysis: #316
Summary
Two functions (LogRPCRequest and LogRPCResponse) contain nearly identical code structure for logging RPC messages. Both follow the same pattern of formatting for debug logs, markdown logs, and JSONL logs, resulting in 22-28 lines of duplicated logic.
Duplication Details
Pattern: RPC Message Logging with Multiple Outputs
- Severity: MEDIUM
- Occurrences: 2 functions with identical structure
- Total Duplicated Lines: ~22-28 lines (11-14 per function)
- Location:
internal/logger/rpc_logger.go(lines 247-281, 284-324)
Code Sample:
// Pattern in both LogRPCRequest and LogRPCResponse:
func LogRPCRequest(serverID, method string, params json.RawMessage) {
// 1. Log to debug (text format)
infoText := &RPCMessageInfo{
Direction: "request",
MessageType: "method call",
ServerID: serverID,
Method: method,
Payload: params,
}
LogDebug("rpc", "%s", formatRPCMessage(infoText))
// 2. Log to markdown
infoMarkdown := &RPCMessageInfo{...} // Same data
globalMarkdownMu.RLock()
defer globalMarkdownMu.RUnlock()
if globalMarkdownLogger != nil {
globalMarkdownLogger.Log(LogLevelDebug, "rpc", "%s", formatRPCMessageMarkdown(infoMarkdown))
}
// 3. Log to JSONL
LogRPCMessageJSONL("request", serverID, method, params, nil)
}
// LogRPCResponse has identical structure with different direction/typeImpact Analysis
- Maintainability: Any enhancement to RPC logging (e.g., adding metrics, changing formats) requires identical changes in both functions
- Bug Risk: MEDIUM - If a logging bug or performance issue is found, both functions must be fixed
- Code Bloat: ~22-28 lines of duplicated logic in critical RPC communication paths
- Consistency Risk: Easy to update one function but forget the other, leading to inconsistent logging behavior
Refactoring Recommendations
1. Extract Common Logging Function (RECOMMENDED)
- Approach: Create single function that handles all RPC message logging
- Implementation:
// logRPCMessage handles logging to debug, markdown, and JSONL outputs func logRPCMessage(direction, messageType, serverID, method string, payload json.RawMessage, errorData *RPCError) { // 1. Log to debug (text format) infoText := &RPCMessageInfo{ Direction: direction, MessageType: messageType, ServerID: serverID, Method: method, Payload: payload, ErrorData: errorData, } LogDebug("rpc", "%s", formatRPCMessage(infoText)) // 2. Log to markdown globalMarkdownMu.RLock() defer globalMarkdownMu.RUnlock() if globalMarkdownLogger != nil { globalMarkdownLogger.Log(LogLevelDebug, "rpc", "%s", formatRPCMessageMarkdown(infoText)) } // 3. Log to JSONL var jsonlDirection string if direction == "request" { jsonlDirection = "request" } else { jsonlDirection = "response" } LogRPCMessageJSONL(jsonlDirection, serverID, method, payload, errorData) } // Simplified public functions: func LogRPCRequest(serverID, method string, params json.RawMessage) { logRPCMessage("request", "method call", serverID, method, params, nil) } func LogRPCResponse(serverID, method string, result json.RawMessage, rpcErr *RPCError) { logRPCMessage("response", "result", serverID, method, result, rpcErr) }
- Estimated effort: 2-3 hours
- Benefits:
- Single source of truth for RPC logging flow
- Easier to add new output formats (e.g., structured logging)
- Reduced code by ~15-20 lines
- Consistent logging behavior guaranteed
2. Consider Adding Logging Levels
- Enhancement: Allow different log levels for requests vs responses
- Benefit: More flexible logging configuration
- Estimated additional effort: 1 hour
Implementation Checklist
- Review current usage of LogRPCRequest and LogRPCResponse
- Create
logRPCMessagehelper function - Refactor LogRPCRequest to use helper
- Refactor LogRPCResponse to use helper
- Verify error handling for responses still works correctly
- Run full test suite, especially RPC-related tests
- Add unit tests for the helper function
- Verify JSONL output format remains unchanged
- Test with markdown logger enabled and disabled
- Consider documenting this pattern in AGENTS.md
Testing Considerations
- Test with all three logging outputs (debug, markdown, JSONL)
- Verify request logging with various payload sizes
- Verify response logging with and without errors
- Test concurrent RPC logging (multiple requests/responses)
- Ensure no performance regression in hot paths
Additional Notes
This refactoring aligns with the existing pattern in internal/logger/global_patterns.go where similar initialization/cleanup logic was successfully extracted. The same principle applies here for RPC message logging.
Parent Issue
See parent analysis report: #316
Related to #316
AI generated by Duplicate Code Detector