Skip to content

Commit 66398ee

Browse files
authored
Consolidate dual logging in server package to internal logger (#3510)
`tool_registry.go` and `unified.go` had 20+ `log.Printf`/`log.Println` calls that either duplicated adjacent `logUnified.Printf`/`logger.LogXxx` calls or bypassed the internal logger entirely. This means log-format changes must be made in two places and bug fixes may only apply to one logger. ### Changes - **Remove duplicate `log.*` calls** adjacent to equivalent `logUnified`/`logger.LogXxx` calls (e.g., shutdown sequence, parallel registration results) - **Replace standalone `log.*` calls** with appropriate always-on or debug-gated loggers: - **Operational/lifecycle messages** (DIFC enable/disable status, server start, tool registration progress) → `logger.LogInfo("startup"/"backend", ...)` so they remain visible in default (non-debug) runs - **Security/audit events** (DIFC access denials, guard/response labeling failures, strict-mode blocks) → `logger.LogWarn("difc", ...)` for always-on auditability - **Verbose debug traces** (agent labels, resource details, filtering counts) → `logUnified.Printf` (DEBUG-gated) - **Remove unused `"log"` import** from both files Before: ```go log.Printf("Calling tool on %s: %s with DIFC enforcement", serverID, toolName) logUnified.Printf("callBackendTool: serverID=%s, toolName=%s, args=%+v", serverID, toolName, args) ``` After: ```go logUnified.Printf("callBackendTool: serverID=%s, toolName=%s, args=%+v", serverID, toolName, args) ``` The inline `sdk.CallToolResult{IsError:true}` pattern (sub-issue #3492) is already consolidated via the existing `newErrorCallToolResult()` helper — no production code instances remain.
2 parents f5c4b89 + 309dcfc commit 66398ee

3 files changed

Lines changed: 29 additions & 38 deletions

File tree

internal/launcher/launcher.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ func GetOrLaunch(l *Launcher, serverID string) (*mcp.Connection, error) {
143143
// Create an HTTP connection
144144
conn, err := mcp.NewHTTPConnection(l.ctx, serverID, serverCfg.URL, serverCfg.Headers, oidcProvider, oidcAudience, l.config.Gateway.HTTPKeepaliveInterval())
145145
if err != nil {
146+
log.Printf("FAILED to create HTTP connection for server %q: %v", serverID, err)
146147
logger.LogErrorWithServer(serverID, "backend", "Failed to create HTTP connection: %s, error=%v", serverID, err)
147148
l.recordError(serverID, err.Error())
148149
return nil, fmt.Errorf("failed to create HTTP connection: %w", err)

internal/server/tool_registry.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7-
"log"
87
"strings"
98
"sync"
109
"time"
@@ -57,18 +56,17 @@ func registerToolWithoutValidation(server *sdk.Server, tool *sdk.Tool, handler f
5756

5857
// registerAllTools fetches and registers tools from all backend servers
5958
func (us *UnifiedServer) registerAllTools() error {
60-
log.Println("Registering tools from all backends...")
61-
logUnified.Printf("Starting tool registration for %d backends", len(us.launcher.ServerIDs()))
59+
logger.LogInfo("backend", "Starting tool registration for %d backends", len(us.launcher.ServerIDs()))
6260

6361
// Only register sys tools if DIFC is enabled
6462
// When DIFC is disabled (default), sys tools are not needed
6563
if us.enableDIFC {
66-
log.Println("DIFC enabled: registering sys tools...")
64+
logger.LogInfo("backend", "DIFC enabled: registering sys tools...")
6765
if err := us.registerSysTools(); err != nil {
68-
log.Printf("Warning: failed to register sys tools: %v", err)
66+
logger.LogWarn("backend", "Failed to register sys tools: %v", err)
6967
}
7068
} else {
71-
log.Println("DIFC disabled: skipping sys tools registration")
69+
logger.LogInfo("backend", "DIFC disabled: skipping sys tools registration")
7270
}
7371

7472
serverIDs := us.launcher.ServerIDs()
@@ -89,7 +87,7 @@ func (us *UnifiedServer) registerAllToolsSequential(serverIDs []string) error {
8987
for _, serverID := range serverIDs {
9088
logUnified.Printf("Registering tools from backend: %s", serverID)
9189
if err := us.registerToolsFromBackend(serverID); err != nil {
92-
log.Printf("Warning: failed to register tools from %s: %v", serverID, err)
90+
logger.LogWarn("backend", "Failed to register tools from %s: %v", serverID, err)
9391
// Continue with other backends
9492
}
9593
}
@@ -132,7 +130,6 @@ func (us *UnifiedServer) registerAllToolsParallel(serverIDs []string) error {
132130
failureCount := 0
133131
for result := range results {
134132
if result.err != nil {
135-
log.Printf("Warning: failed to register tools from %s (took %v): %v", result.serverID, result.duration, result.err)
136133
logger.LogWarnWithServer(result.serverID, "backend", "Failed to register tools from %s (took %v): %v", result.serverID, result.duration, result.err)
137134
failureCount++
138135
} else {
@@ -142,14 +139,13 @@ func (us *UnifiedServer) registerAllToolsParallel(serverIDs []string) error {
142139
}
143140
}
144141

145-
log.Printf("Parallel tool registration complete: %d succeeded, %d failed, total tools=%d", successCount, failureCount, len(us.tools))
146-
logUnified.Printf("Tool registration complete: %d succeeded, %d failed, total tools=%d", successCount, failureCount, len(us.tools))
142+
logger.LogInfo("backend", "Tool registration complete: %d succeeded, %d failed, total tools=%d", successCount, failureCount, len(us.tools))
147143
return nil
148144
}
149145

150146
// registerToolsFromBackend registers tools from a specific backend with <server>___<tool> naming
151147
func (us *UnifiedServer) registerToolsFromBackend(serverID string) error {
152-
log.Printf("Registering tools from backend: %s", serverID)
148+
logUnified.Printf("Registering tools from backend: %s", serverID)
153149

154150
// Get connection to backend
155151
conn, err := launcher.GetOrLaunch(us.launcher, serverID)
@@ -291,10 +287,10 @@ func (us *UnifiedServer) registerToolsFromBackend(serverID string) error {
291287
Annotations: tool.Annotations,
292288
}, finalHandler)
293289

294-
log.Printf("Registered tool: %s", logName)
290+
logUnified.Printf("Registered tool: %s", logName)
295291
}
296292

297-
log.Printf("Registered %d tools from %s: %s", len(listResult.Tools), serverID, strings.Join(toolNames, ", "))
293+
logUnified.Printf("Registered %d tools from %s: %s", len(listResult.Tools), serverID, strings.Join(toolNames, ", "))
298294
return nil
299295
}
300296

@@ -370,7 +366,6 @@ func (us *UnifiedServer) registerSysTools() error {
370366
}
371367

372368
logger.LogInfo("client", "MCP session initialized successfully, session=%s, available_servers=%v", sessionID, us.launcher.ServerIDs())
373-
log.Printf("Initialized session: %s", sessionID)
374369

375370
// Call sys_init
376371
result, err := us.callSysServer("sys_init")
@@ -434,6 +429,6 @@ func (us *UnifiedServer) registerSysTools() error {
434429
sysListHandler,
435430
)
436431

437-
log.Println("Registered 2 sys tools")
432+
logUnified.Printf("Registered 2 sys tools")
438433
return nil
439434
}

internal/server/unified.go

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"encoding/json"
66
"fmt"
77
"io"
8-
"log"
98
"net/http"
109
"sync"
1110
"time"
@@ -184,9 +183,9 @@ func NewUnified(ctx context.Context, cfg *config.Config) (*UnifiedServer, error)
184183

185184
// Log guards status early (before backend launch which may take time)
186185
if us.enableDIFC {
187-
log.Printf("Guards enforcement enabled with mode: %s", cfg.DIFCMode)
186+
logger.LogInfo("startup", "Guards enforcement enabled with mode: %s", cfg.DIFCMode)
188187
} else {
189-
log.Println("Guards enforcement disabled (sessions auto-created for standard MCP client compatibility)")
188+
logger.LogInfo("startup", "Guards enforcement disabled (sessions auto-created for standard MCP client compatibility)")
190189
}
191190

192191
// Register aggregated tools from all backends
@@ -254,7 +253,7 @@ func (g *guardBackendCaller) CallTool(ctx context.Context, toolName string, args
254253

255254
// Make a read-only call to the backend for metadata
256255
// This bypasses DIFC checks since it's internal to the guard
257-
log.Printf("[DIFC] Guard calling backend %s tool %s for metadata", g.serverID, toolName)
256+
logUnified.Printf("[DIFC] Guard calling backend %s tool %s for metadata", g.serverID, toolName)
258257

259258
sessionID := SessionIDFromContext(g.ctx)
260259

@@ -413,7 +412,6 @@ func (us *UnifiedServer) isToolAllowed(serverID, toolName string) bool {
413412
func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName string, args interface{}) (*sdk.CallToolResult, interface{}, error) {
414413
// Note: Session validation happens at the tool registration level via closures
415414
// The closure captures the request and validates before calling this method
416-
log.Printf("Calling tool on %s: %s with DIFC enforcement", serverID, toolName)
417415
logUnified.Printf("callBackendTool: serverID=%s, toolName=%s, args=%+v", serverID, toolName, args)
418416

419417
// Start an OTEL span for the full tool call lifecycle (spans all phases 0–6)
@@ -470,7 +468,7 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
470468
// **Phase 0: Extract agent ID and get/create agent labels**
471469
agentID := guard.GetAgentIDFromContext(ctx)
472470
agentLabels := us.agentRegistry.GetOrCreate(agentID)
473-
log.Printf("[DIFC] Agent %s | Secrecy: %v | Integrity: %v",
471+
logUnified.Printf("[DIFC] Agent %s | Secrecy: %v | Integrity: %v",
474472
agentID, agentLabels.GetSecrecyTags(), agentLabels.GetIntegrityTags())
475473

476474
ctx = context.WithValue(ctx, mcp.AgentTagsSnapshotContextKey, &mcp.AgentTagsSnapshot{
@@ -487,12 +485,12 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
487485
// **Phase 1: Guard labels the resource**
488486
resource, operation, err := g.LabelResource(ctx, toolName, args, backendCaller, us.capabilities)
489487
if err != nil {
490-
log.Printf("[DIFC] Guard labeling failed: %v", err)
488+
logger.LogWarn("difc", "Guard labeling failed: %v", err)
491489
httpStatusCode = 500
492490
return newErrorCallToolResult(fmt.Errorf("guard labeling failed: %w", err))
493491
}
494492

495-
log.Printf("[DIFC] Resource: %s | Operation: %s | Secrecy: %v | Integrity: %v",
493+
logUnified.Printf("[DIFC] Resource: %s | Operation: %s | Secrecy: %v | Integrity: %v",
496494
resource.Description, operation, resource.Secrecy.Label.GetTags(), resource.Integrity.Label.GetTags())
497495

498496
// **Phase 2: Reference Monitor performs coarse-grained access check**
@@ -506,19 +504,19 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
506504
if isReadOperation {
507505
// Read operation in any mode - skip coarse-grained block
508506
// The guard will label response items and Phase 5 will enforce per-item policy
509-
log.Printf("[DIFC] Coarse-grained check failed for read in %s mode - proceeding to backend for response labeling", enforcementMode)
510-
log.Printf("[DIFC] Response items will be evaluated at Phase 5 based on per-item labels from LabelResponse()")
507+
logUnified.Printf("[DIFC] Coarse-grained check failed for read in %s mode - proceeding to backend for response labeling", enforcementMode)
508+
logUnified.Printf("[DIFC] Response items will be evaluated at Phase 5 based on per-item labels from LabelResponse()")
511509
} else {
512510
// Non-read operation - block the request
513-
log.Printf("[DIFC] Access DENIED for agent %s to %s: %s", agentID, resource.Description, result.Reason)
511+
logger.LogWarn("difc", "Access DENIED for agent %s to %s: %s", agentID, resource.Description, result.Reason)
514512
detailedErr := difc.FormatViolationError(result, agentLabels.Secrecy, agentLabels.Integrity, resource)
515513
toolSpan.RecordError(detailedErr)
516514
toolSpan.SetStatus(codes.Error, "access denied: "+result.Reason)
517515
httpStatusCode = 403
518516
return newErrorCallToolResult(detailedErr)
519517
}
520518
} else {
521-
log.Printf("[DIFC] Access ALLOWED for agent %s to %s", agentID, resource.Description)
519+
logUnified.Printf("[DIFC] Access ALLOWED for agent %s to %s", agentID, resource.Description)
522520
}
523521

524522
// **Phase 3: Execute the backend call**
@@ -549,12 +547,12 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
549547
if shouldCallLabelResponse {
550548
labeledData, err = g.LabelResponse(ctx, toolName, backendResult, backendCaller, us.capabilities)
551549
if err != nil {
552-
log.Printf("[DIFC] Response labeling failed: %v", err)
550+
logger.LogWarn("difc", "Response labeling failed: %v", err)
553551
httpStatusCode = 500
554552
return newErrorCallToolResult(fmt.Errorf("response labeling failed: %w", err))
555553
}
556554
} else {
557-
log.Printf("[DIFC] Skipping LabelResponse() for %s operation in %s mode", operation, enforcementMode)
555+
logUnified.Printf("[DIFC] Skipping LabelResponse() for %s operation in %s mode", operation, enforcementMode)
558556
}
559557

560558
// **Phase 5: Reference Monitor performs fine-grained filtering (if applicable)**
@@ -566,12 +564,12 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
566564
// Filter collection based on agent labels
567565
filtered := requestEvaluator.FilterCollection(agentLabels.Secrecy, agentLabels.Integrity, collection, operation)
568566

569-
log.Printf("[DIFC] Filtered collection: %d/%d items accessible",
567+
logUnified.Printf("[DIFC] Filtered collection: %d/%d items accessible",
570568
filtered.GetAccessibleCount(), filtered.TotalCount)
571569

572570
// **Strict mode: block entire response if ANY item is filtered**
573571
if enforcementMode == difc.EnforcementStrict && filtered.GetFilteredCount() > 0 {
574-
log.Printf("[DIFC] STRICT MODE: Blocking entire response - %d/%d items violate DIFC policy",
572+
logger.LogWarn("difc", "STRICT MODE: Blocking entire response - %d/%d items violate DIFC policy",
575573
filtered.GetFilteredCount(), filtered.TotalCount)
576574
blockErr := fmt.Errorf("DIFC policy violation: %d of %d items in response are not accessible to agent %s",
577575
filtered.GetFilteredCount(), filtered.TotalCount, agentID)
@@ -580,7 +578,7 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
580578
}
581579

582580
if filtered.GetFilteredCount() > 0 {
583-
log.Printf("[DIFC] Filtered out %d items due to DIFC policy", filtered.GetFilteredCount())
581+
logUnified.Printf("[DIFC] Filtered out %d items due to DIFC policy", filtered.GetFilteredCount())
584582
logFilteredItems(serverID, toolName, filtered)
585583
difcFiltered = filtered
586584
}
@@ -606,7 +604,7 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
606604
if !isPureWrite && enforcementMode == difc.EnforcementPropagate {
607605
overall := labeledData.Overall()
608606
agentLabels.AccumulateFromRead(overall)
609-
log.Printf("[DIFC] Agent %s accumulated labels (propagate mode) | Secrecy: %v | Integrity: %v",
607+
logUnified.Printf("[DIFC] Agent %s accumulated labels (propagate mode) | Secrecy: %v | Integrity: %v",
610608
agentID, agentLabels.GetSecrecyTags(), agentLabels.GetIntegrityTags())
611609
}
612610
} else {
@@ -616,7 +614,7 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
616614
// **Phase 6: Accumulate labels from resource (for reads in PROPAGATE mode only)**
617615
if !isPureWrite && enforcementMode == difc.EnforcementPropagate {
618616
agentLabels.AccumulateFromRead(resource)
619-
log.Printf("[DIFC] Agent %s accumulated labels (propagate mode) | Secrecy: %v | Integrity: %v",
617+
logUnified.Printf("[DIFC] Agent %s accumulated labels (propagate mode) | Secrecy: %v | Integrity: %v",
620618
agentID, agentLabels.GetSecrecyTags(), agentLabels.GetIntegrityTags())
621619
}
622620
}
@@ -643,7 +641,7 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
643641

644642
// Run starts the unified MCP server on the specified transport
645643
func (us *UnifiedServer) Run(transport sdk.Transport) error {
646-
log.Println("Starting unified MCP server...")
644+
logger.LogInfo("startup", "Starting unified MCP server...")
647645
return us.server.Run(us.ctx, transport)
648646
}
649647

@@ -736,7 +734,6 @@ func (us *UnifiedServer) InitiateShutdown() int {
736734
us.isShutdown = true
737735
us.shutdownMu.Unlock()
738736

739-
log.Println("Initiating gateway shutdown...")
740737
logger.LogInfo("shutdown", "Gateway shutdown initiated")
741738

742739
// Stop health monitor before closing connections
@@ -748,11 +745,9 @@ func (us *UnifiedServer) InitiateShutdown() int {
748745
serversTerminated = len(us.launcher.ServerIDs())
749746

750747
// Terminate all backend servers
751-
log.Printf("Terminating %d backend server(s)...", serversTerminated)
752748
logger.LogInfo("shutdown", "Terminating %d backend servers", serversTerminated)
753749
us.launcher.Close()
754750

755-
log.Println("Backend servers terminated")
756751
logger.LogInfo("shutdown", "Backend servers terminated successfully")
757752
})
758753
return serversTerminated

0 commit comments

Comments
 (0)