From 28f55a9d118ddfd2719115b13db45d93596ec2f4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 9 Feb 2026 12:44:07 +0000 Subject: [PATCH 1/3] Initial plan From e3d01c9ebed048860920af9a630619c793df8417 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 9 Feb 2026 13:02:42 +0000 Subject: [PATCH 2/3] Fix missing access and gateway logs by checking sandbox/firewall/logs and mcp-logs paths - Updated analyzeAccessLogs to also check sandbox/firewall/logs/ path - Updated analyzeFirewallLogs to prioritize sandbox/firewall/logs/ path - Updated parseGatewayLogs to check mcp-logs/gateway.jsonl path - Added tests for new log paths - All tests pass including integration tests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/access_log.go | 11 +++++- pkg/cli/access_log_test.go | 18 +++++++++ pkg/cli/firewall_log.go | 14 ++++++- pkg/cli/firewall_log_integration_test.go | 47 ++++++++++++++++++++++++ pkg/cli/gateway_logs.go | 27 ++++++++++++-- pkg/cli/gateway_logs_test.go | 33 +++++++++++++++++ 6 files changed, 145 insertions(+), 5 deletions(-) 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..e6becb91980 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..751ee3611a7 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") +} From 11b72a736d51b9565f34361b1019bf1c1f322808 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 9 Feb 2026 13:09:00 +0000 Subject: [PATCH 3/3] Complete fix for missing observability logs - all tests pass - All unit tests pass - Integration tests pass - Build succeeds - Linter passes - Ready for review Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/firewall_log_integration_test.go | 68 ++++++++++++------------ pkg/cli/gateway_logs_test.go | 50 ++++++++--------- 2 files changed, 59 insertions(+), 59 deletions(-) diff --git a/pkg/cli/firewall_log_integration_test.go b/pkg/cli/firewall_log_integration_test.go index e6becb91980..4e5dcbf7b4d 100644 --- a/pkg/cli/firewall_log_integration_test.go +++ b/pkg/cli/firewall_log_integration_test.go @@ -241,46 +241,46 @@ func TestFirewallLogSummaryBuilding(t *testing.T) { // 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 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 + // 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) -} + 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") -} + // 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) -} + // 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_test.go b/pkg/cli/gateway_logs_test.go index 751ee3611a7..5d90cc2fcc0 100644 --- a/pkg/cli/gateway_logs_test.go +++ b/pkg/cli/gateway_logs_test.go @@ -454,34 +454,34 @@ func TestGatewayLogsParsingIntegration(t *testing.T) { } func TestParseGatewayLogsFromMCPLogsSubdirectory(t *testing.T) { -// Create temp directory for test -tmpDir := t.TempDir() + // 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 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} + // 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") + 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") }