Skip to content

Commit 75b04c0

Browse files
Copilotlpcox
andauthored
Refactor: eliminate dual-logging and extract logger initialization fallback helper
- Remove duplicate log.Printf calls in http_helpers.go that echoed info already captured by structured logger.Log* calls (extractAndValidateSession, logHTTPRequestBody, setupSessionCallback, withResponseLogging) - Add shared logFallbackWarnings helper in internal/logger/init.go - Use logFallbackWarnings in file_logger.go, server_file_logger.go, and tools_logger.go to eliminate repeated WARNING log pairs Closes #4641 #4642 Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/7967a92b-368d-4295-a167-3cae7a5dfa0b Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
1 parent 8fdb15a commit 75b04c0

5 files changed

Lines changed: 11 additions & 18 deletions

File tree

internal/logger/file_logger.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ func setupFileLogger(file *os.File, logDir, fileName string) (*FileLogger, error
3737

3838
// handleFileLoggerError falls back to stdout when the log file cannot be opened.
3939
func handleFileLoggerError(err error, logDir, fileName string) (*FileLogger, error) {
40-
log.Printf("WARNING: Failed to initialize log file: %v", err)
41-
log.Printf("WARNING: Falling back to stdout for logging")
40+
logFallbackWarnings(err, "Failed to initialize log file", "Falling back to stdout for logging")
4241
fl := &FileLogger{
4342
logDir: logDir,
4443
fileName: fileName,

internal/logger/init.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,10 @@ func initWithWarning(err error, name string) {
2424
log.Printf("Warning: Failed to initialize %s: %v", name, err)
2525
}
2626
}
27+
28+
// logFallbackWarnings prints two WARNING lines for logger initialization failure with fallback:
29+
// the first includes the underlying error, the second describes the fallback behavior.
30+
func logFallbackWarnings(err error, errMsg, fallbackMsg string) {
31+
log.Printf("WARNING: %s: %v", errMsg, err)
32+
log.Printf("WARNING: %s", fallbackMsg)
33+
}

internal/logger/server_file_logger.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ var (
2828
func InitServerFileLogger(logDir string) error {
2929
// Create log directory if it doesn't exist
3030
if err := os.MkdirAll(logDir, 0755); err != nil {
31-
log.Printf("WARNING: Failed to create log directory for server logs: %v", err)
32-
log.Printf("WARNING: Falling back to unified logging only")
31+
logFallbackWarnings(err, "Failed to create log directory for server logs", "Falling back to unified logging only")
3332
// Create a fallback logger that won't create files
3433
sfl := &ServerFileLogger{
3534
logDir: logDir,

internal/logger/tools_logger.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ func setupToolsLogger(file *os.File, logDir, fileName string) (*ToolsLogger, err
6060

6161
// handleToolsLoggerError falls back to a no-op logger when the file cannot be opened.
6262
func handleToolsLoggerError(err error, logDir, fileName string) (*ToolsLogger, error) {
63-
log.Printf("WARNING: Failed to initialize tools log file: %v", err)
64-
log.Printf("WARNING: Tools logging disabled")
63+
logFallbackWarnings(err, "Failed to initialize tools log file", "Tools logging disabled")
6564
tl := &ToolsLogger{
6665
logDir: logDir,
6766
fileName: fileName,

internal/server/http_helpers.go

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func withResponseLogging(handler http.Handler) http.Handler {
7777
handler.ServeHTTP(lw, r)
7878
if len(lw.Body()) > 0 {
7979
sanitizedBody := sanitize.SanitizeString(string(lw.Body()))
80-
log.Printf("[%s] %s %s - Status: %d, Response: %s", r.RemoteAddr, r.Method, r.URL.Path, lw.StatusCode(), sanitizedBody)
80+
logHelpers.Printf("[%s] %s %s - Status: %d, Response: %s", r.RemoteAddr, r.Method, r.URL.Path, lw.StatusCode(), sanitizedBody)
8181
}
8282
})
8383
}
@@ -93,7 +93,6 @@ func extractAndValidateSession(r *http.Request) string {
9393
if sessionID == "" {
9494
logHelpers.Printf("Session extraction failed: no Authorization header, remote=%s", r.RemoteAddr)
9595
logger.LogError("client", "Rejected MCP client connection: no Authorization header, remote=%s, path=%s", r.RemoteAddr, r.URL.Path)
96-
log.Printf("[%s] %s %s - REJECTED: No Authorization header", r.RemoteAddr, r.Method, r.URL.Path)
9796
return ""
9897
}
9998

@@ -155,7 +154,6 @@ func logHTTPRequestBody(r *http.Request, sessionID, backendID string) {
155154
} else {
156155
logger.LogDebug("client", "MCP request body, session=%s, body=%s", sessionID, sanitizedBody)
157156
}
158-
log.Printf("Request body: %s", sanitizedBody)
159157
logHelpers.Print("Request body logged for debugging")
160158
}
161159

@@ -189,23 +187,14 @@ func setupSessionCallback(r *http.Request, backendID string) (string, bool) {
189187
if backendID != "" {
190188
logger.LogInfo("client", "New MCP client connection, remote=%s, method=%s, path=%s, backend=%s, session=%s",
191189
r.RemoteAddr, r.Method, r.URL.Path, backendID, sessionID)
192-
log.Printf("=== NEW STREAMABLE HTTP CONNECTION (ROUTED) ===")
193190
} else {
194191
logger.LogInfo("client", "MCP connection established, remote=%s, method=%s, path=%s, session=%s",
195192
r.RemoteAddr, r.Method, r.URL.Path, sessionID)
196-
log.Printf("=== NEW STREAMABLE HTTP CONNECTION ===")
197193
}
198-
log.Printf("[%s] %s %s", r.RemoteAddr, r.Method, r.URL.Path)
199-
if backendID != "" {
200-
log.Printf("Backend: %s", backendID)
201-
}
202-
log.Printf("Authorization (Session ID): %s", sanitize.TruncateSecret(sessionID))
203194

204195
logHTTPRequestBody(r, sessionID, backendID)
205196

206197
*r = *injectSessionContext(r, sessionID, backendID)
207-
log.Printf("✓ Injected session ID into context")
208-
log.Printf("===================================\n")
209198

210199
return sessionID, true
211200
}

0 commit comments

Comments
 (0)