diff --git a/pkg/cli/access_log.go b/pkg/cli/access_log.go index 7186a4d59fc..06991610537 100644 --- a/pkg/cli/access_log.go +++ b/pkg/cli/access_log.go @@ -157,13 +157,22 @@ func parseSquidLogLine(line string) (*AccessLogEntry, error) { func analyzeAccessLogs(runDir string, verbose bool) (*DomainAnalysis, error) { accessLogLog.Printf("Analyzing access logs in: %s", runDir) - // Check for access log files in access.log directory + // Check for access log files in access.log directory (legacy path) accessLogsDir := filepath.Join(runDir, "access.log") if _, err := os.Stat(accessLogsDir); err == nil { accessLogLog.Printf("Found access logs directory: %s", accessLogsDir) return analyzeMultipleAccessLogs(accessLogsDir, verbose) } + // Check for access logs in sandbox/firewall/logs/ directory (new path after artifact download) + // Firewall logs are uploaded from /tmp/gh-aw/sandbox/firewall/logs/ and the common parent + // /tmp/gh-aw/ is stripped during artifact upload, resulting in sandbox/firewall/logs/ after download + sandboxFirewallLogsDir := filepath.Join(runDir, "sandbox", "firewall", "logs") + if _, err := os.Stat(sandboxFirewallLogsDir); err == nil { + accessLogLog.Printf("Found firewall logs directory: %s", sandboxFirewallLogsDir) + return analyzeMultipleAccessLogs(sandboxFirewallLogsDir, verbose) + } + // No access logs found accessLogLog.Printf("No access logs directory found in: %s", runDir) if verbose { diff --git a/pkg/cli/access_log_test.go b/pkg/cli/access_log_test.go index 2e214aaf870..682acf1247d 100644 --- a/pkg/cli/access_log_test.go +++ b/pkg/cli/access_log_test.go @@ -116,6 +116,24 @@ func TestAnalyzeAccessLogsDirectory(t *testing.T) { require.NoError(t, err, "should not error when no logs present") assert.Nil(t, analysis, "should return nil when no logs found") }) + + t.Run("access logs in sandbox/firewall/logs/ (new path)", func(t *testing.T) { + // Test case 3: Access logs in sandbox/firewall/logs/ directory after artifact download + sandboxLogsDir := filepath.Join(tempDir, "run3", "sandbox", "firewall", "logs") + err := os.MkdirAll(sandboxLogsDir, 0755) + require.NoError(t, err, "should create sandbox/firewall/logs directory") + + fetchLogContent := `1701234567.123 180 192.168.1.100 TCP_MISS/200 1234 GET http://example.com/api/data - HIER_DIRECT/93.184.216.34 text/html +1701234568.456 250 192.168.1.100 TCP_HIT/200 5678 GET http://api.github.com/repos - HIER_DIRECT/140.82.112.6 application/json` + fetchLogPath := filepath.Join(sandboxLogsDir, "access-1.log") + err = os.WriteFile(fetchLogPath, []byte(fetchLogContent), 0644) + require.NoError(t, err, "should create test access log in sandbox path") + + analysis, err := analyzeAccessLogs(filepath.Join(tempDir, "run3"), false) + require.NoError(t, err, "should analyze access logs from sandbox path") + require.NotNil(t, analysis, "should return analysis for logs in sandbox path") + assert.Equal(t, 2, analysis.TotalRequests, "should count requests from sandbox path") + }) } func TestExtractDomainFromURL(t *testing.T) { diff --git a/pkg/cli/firewall_log.go b/pkg/cli/firewall_log.go index fcb93806383..449428fd5e2 100644 --- a/pkg/cli/firewall_log.go +++ b/pkg/cli/firewall_log.go @@ -311,7 +311,19 @@ func analyzeFirewallLogs(runDir string, verbose bool) (*FirewallAnalysis, error) // Look for firewall logs in the run directory // The logs could be in several locations depending on how they were uploaded - // First, check for directories starting with squid-logs or firewall-logs + // First, check for sandbox/firewall/logs/ directory (new path after artifact download) + // Firewall logs are uploaded from /tmp/gh-aw/sandbox/firewall/logs/ and the common parent + // /tmp/gh-aw/ is stripped during artifact upload, resulting in sandbox/firewall/logs/ after download + sandboxFirewallLogsDir := filepath.Join(runDir, "sandbox", "firewall", "logs") + if _, err := os.Stat(sandboxFirewallLogsDir); err == nil { + firewallLogLog.Printf("Found firewall logs directory: sandbox/firewall/logs") + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Found firewall logs directory: sandbox/firewall/logs")) + } + return analyzeMultipleFirewallLogs(sandboxFirewallLogsDir, verbose) + } + + // Second, check for directories starting with squid-logs or firewall-logs (legacy paths) // The actual directories may have workflow-specific suffixes like: // - squid-logs-smoke-copilot-firewall // - squid-logs-changeset-generator diff --git a/pkg/cli/firewall_log_integration_test.go b/pkg/cli/firewall_log_integration_test.go index 4364d0f4eec..4e5dcbf7b4d 100644 --- a/pkg/cli/firewall_log_integration_test.go +++ b/pkg/cli/firewall_log_integration_test.go @@ -237,3 +237,50 @@ func TestFirewallLogSummaryBuilding(t *testing.T) { t.Error("workflow-1 not found in ByWorkflow") } } + +// TestAnalyzeFirewallLogsFromSandboxPath tests that firewall logs can be found +// in sandbox/firewall/logs/ directory (the path after artifact download) +func TestAnalyzeFirewallLogsFromSandboxPath(t *testing.T) { + // Create temp directory for test + runDir := testutil.TempDir(t, "firewall-test-*") + + // Create sandbox/firewall/logs directory (path after artifact download) + firewallLogsDir := filepath.Join(runDir, "sandbox", "firewall", "logs") + err := os.MkdirAll(firewallLogsDir, 0755) + if err != nil { + t.Fatalf("should create sandbox/firewall/logs directory: %v", err) + } + + // Create test firewall log file + logContent := `# Firewall access log +1701234567.123 172.30.0.20:35288 api.github.com:443 140.82.112.22:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-" +1701234568.456 172.30.0.20:35289 api.npmjs.org:443 151.101.0.162:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.npmjs.org:443 "-" +1701234569.789 172.30.0.20:35290 blocked.example.com:443 0.0.0.0:0 1.1 CONNECT 403 NONE_NONE:HIER_NONE blocked.example.com:443 "-" +1701234570.012 172.30.0.20:35291 blocked.test.com:443 0.0.0.0:0 1.1 CONNECT 403 NONE_NONE:HIER_NONE blocked.test.com:443 "-" +` + logPath := filepath.Join(firewallLogsDir, "firewall-access.log") + err = os.WriteFile(logPath, []byte(logContent), 0644) + if err != nil { + t.Fatalf("should write test firewall log: %v", err) + } + + // Test the analysis + analysis, err := analyzeFirewallLogs(runDir, true) + if err != nil { + t.Fatalf("analyzeFirewallLogs should succeed: %v", err) + } + if analysis == nil { + t.Fatal("Analysis should not be nil") + } + + // Verify results + if analysis.TotalRequests != 4 { + t.Errorf("TotalRequests: got %d, want 4", analysis.TotalRequests) + } + if analysis.AllowedRequests != 2 { + t.Errorf("AllowedRequests: got %d, want 2", analysis.AllowedRequests) + } + if analysis.BlockedRequests != 2 { + t.Errorf("BlockedRequests: got %d, want 2", analysis.BlockedRequests) + } +} diff --git a/pkg/cli/gateway_logs.go b/pkg/cli/gateway_logs.go index 7731c9fa581..638900738a5 100644 --- a/pkg/cli/gateway_logs.go +++ b/pkg/cli/gateway_logs.go @@ -80,12 +80,21 @@ type GatewayMetrics struct { // parseGatewayLogs parses a gateway.jsonl file and extracts metrics func parseGatewayLogs(logDir string, verbose bool) (*GatewayMetrics, error) { + // Try root directory first (for older logs where gateway.jsonl was in the root) gatewayLogPath := filepath.Join(logDir, "gateway.jsonl") - // Check if gateway.jsonl exists + // Check if gateway.jsonl exists in root if _, err := os.Stat(gatewayLogPath); os.IsNotExist(err) { - gatewayLogsLog.Printf("gateway.jsonl not found at: %s", gatewayLogPath) - return nil, fmt.Errorf("gateway.jsonl not found") + // Try mcp-logs subdirectory (new path after artifact download) + // Gateway logs are uploaded from /tmp/gh-aw/mcp-logs/gateway.jsonl and the common parent + // /tmp/gh-aw/ is stripped during artifact upload, resulting in mcp-logs/gateway.jsonl after download + mcpLogsPath := filepath.Join(logDir, "mcp-logs", "gateway.jsonl") + if _, err := os.Stat(mcpLogsPath); os.IsNotExist(err) { + gatewayLogsLog.Printf("gateway.jsonl not found at: %s or %s", gatewayLogPath, mcpLogsPath) + return nil, fmt.Errorf("gateway.jsonl not found") + } + gatewayLogPath = mcpLogsPath + gatewayLogsLog.Printf("Found gateway.jsonl in mcp-logs subdirectory") } gatewayLogsLog.Printf("Parsing gateway.jsonl from: %s", gatewayLogPath) @@ -399,7 +408,19 @@ func extractMCPToolUsageData(logDir string, verbose bool) (*MCPToolUsageData, er } // Read gateway.jsonl again to get individual tool call records + // Try root directory first (for older logs where gateway.jsonl was in the root) gatewayLogPath := filepath.Join(logDir, "gateway.jsonl") + + // Check if gateway.jsonl exists in root + if _, err := os.Stat(gatewayLogPath); os.IsNotExist(err) { + // Try mcp-logs subdirectory (new path after artifact download) + mcpLogsPath := filepath.Join(logDir, "mcp-logs", "gateway.jsonl") + if _, err := os.Stat(mcpLogsPath); os.IsNotExist(err) { + return nil, fmt.Errorf("gateway.jsonl not found") + } + gatewayLogPath = mcpLogsPath + } + file, err := os.Open(gatewayLogPath) if err != nil { return nil, fmt.Errorf("failed to open gateway.jsonl: %w", err) diff --git a/pkg/cli/gateway_logs_test.go b/pkg/cli/gateway_logs_test.go index 06397130888..5d90cc2fcc0 100644 --- a/pkg/cli/gateway_logs_test.go +++ b/pkg/cli/gateway_logs_test.go @@ -452,3 +452,36 @@ func TestGatewayLogsParsingIntegration(t *testing.T) { assert.False(t, metrics.EndTime.IsZero()) assert.True(t, metrics.EndTime.After(metrics.StartTime)) } + +func TestParseGatewayLogsFromMCPLogsSubdirectory(t *testing.T) { + // Create temp directory for test + tmpDir := t.TempDir() + + // Create mcp-logs subdirectory (path after artifact download) + mcpLogsDir := filepath.Join(tmpDir, "mcp-logs") + err := os.MkdirAll(mcpLogsDir, 0755) + require.NoError(t, err, "should create mcp-logs directory") + + // Create test gateway.jsonl in mcp-logs subdirectory + testLogContent := `{"timestamp":"2024-01-15T10:00:00Z","level":"info","event":"tool_call","server_name":"github","tool_name":"search_code","duration":250} +{"timestamp":"2024-01-15T10:00:01Z","level":"info","event":"tool_call","server_name":"github","tool_name":"list_issues","duration":180} +{"timestamp":"2024-01-15T10:00:02Z","level":"error","event":"tool_call","server_name":"github","tool_name":"create_issue","duration":100} +` + gatewayLogPath := filepath.Join(mcpLogsDir, "gateway.jsonl") + err = os.WriteFile(gatewayLogPath, []byte(testLogContent), 0644) + require.NoError(t, err, "should write test gateway.jsonl in mcp-logs") + + // Test parsing from mcp-logs subdirectory + metrics, err := parseGatewayLogs(tmpDir, false) + require.NoError(t, err, "should parse gateway logs from mcp-logs subdirectory") + require.NotNil(t, metrics, "metrics should not be nil") + + // Verify results + assert.Equal(t, 3, metrics.TotalRequests, "should have 3 total requests") + assert.Len(t, metrics.Servers, 1, "should have 1 server") + + // Verify server metrics + githubMetrics, ok := metrics.Servers["github"] + require.True(t, ok, "should have github server metrics") + assert.Equal(t, 3, githubMetrics.RequestCount, "should have 3 total calls for github server") +}