Skip to content

Conversation

@github-actions
Copy link
Contributor

Q Workflow Optimization Report

Investigation Summary

User Question: "/q investigate the issue with firewall logs. Are they uploaded or is it an issue in logs?"

Answer:

  • ✓ Firewall logs ARE uploaded correctly
  • ✗ There IS an issue with parsing them in the logs command
  • Fix applied - Now parses directories with workflow-specific suffixes

Issues Found (from live data)

Firewall Log Analysis

Analyzed Runs: 18874197110, 18874317908, 18874452198

Evidence:

  • Firewall logs exist as artifacts: squid-logs-smoke-copilot-firewall/, squid-logs-changeset-generator/
  • All run_summary.json files show "firewall_analysis": null
  • Example log content found:
    1761653294.814 172.30.0.20:53082 api.github.com:443 140.82.112.5:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.github.com:443 "-"
    1761653295.337 172.30.0.20:53092 api.enterprise.githubcopilot.com:443 140.82.114.22:443 1.1 CONNECT 200 TCP_TUNNEL:HIER_DIRECT api.enterprise.githubcopilot.com:443 "-"
    

Root Cause:

The analyzeFirewallLogs() function in pkg/cli/firewall_log.go (lines 315-330) was looking for directories named exactly:

  • squid-logs
  • firewall-logs

But actual artifact directories have workflow-specific suffixes:

  • squid-logs-{workflow-name} (e.g., squid-logs-smoke-copilot-firewall)
  • firewall-logs-{workflow-name}

This directory name mismatch caused uploaded logs to be ignored during analysis.

Changes Made

pkg/cli/firewall_log.go

Lines modified: 316-343

Changes:

  1. Replaced hardcoded directory list with os.ReadDir() to scan all directories
  2. Changed from exact directory name matching to prefix matching using strings.HasPrefix()
  3. Added verbose logging when firewall logs directory is found
  4. Added comprehensive comments explaining workflow-specific suffix pattern

Before:

possibleDirs := []string{
    filepath.Join(runDir, "squid-logs"),
    filepath.Join(runDir, "firewall-logs"),
}
for _, logsDir := range possibleDirs {
    if stat, err := os.Stat(logsDir); err == nil && stat.IsDir() {
        return analyzeMultipleFirewallLogs(logsDir, verbose)
    }
}

After:

entries, err := os.ReadDir(runDir)
if err != nil {
    return nil, fmt.Errorf("failed to read run directory: %w", err)
}
for _, entry := range entries {
    if !entry.IsDir() {
        continue
    }
    name := entry.Name()
    if strings.HasPrefix(name, "squid-logs") || strings.HasPrefix(name, "firewall-logs") {
        logsDir := filepath.Join(runDir, name)
        if verbose {
            fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Found firewall logs directory: %s", name)))
        }
        return analyzeMultipleFirewallLogs(logsDir, verbose)
    }
}

pkg/cli/firewall_log_test.go

Added: TestAnalyzeFirewallLogsWithWorkflowSuffix() (60 lines)

Purpose: Validates that firewall log parsing works with workflow-specific directory suffixes

Test coverage:

  • Creates realistic directory structure: squid-logs-smoke-copilot-firewall/
  • Creates sample access.log with 3 requests (2 allowed, 1 denied)
  • Verifies analyzeFirewallLogs() finds and parses the logs correctly
  • Validates allowed/denied domain categorization
  • Ensures request counts are accurate

Expected Improvements

After this fix:

  • ✅ Firewall logs will be parsed correctly regardless of workflow-specific suffix
  • run_summary.json will include firewall analysis data
  • gh aw logs command will display firewall statistics in output
  • gh aw audit command will include firewall analysis in reports
  • ✅ Works with any workflow name suffix pattern

Validation

Unit Test Added:

  • TestAnalyzeFirewallLogsWithWorkflowSuffix() - Tests realistic workflow artifact structure

Manual Validation:

  • Verified firewall logs exist in downloaded artifacts
  • Confirmed directory names follow squid-logs-{workflow-name} pattern
  • Reviewed actual log content to ensure format compatibility

References

  • Investigation Report: /tmp/gh-aw/cache-memory/q-firewall-investigation.md
  • Log Evidence: /tmp/gh-aw/aw-mcp/logs/run-18874197110/squid-logs-smoke-copilot-firewall/
  • Related Documentation: FIREWALL_LOG_PARSER_IMPLEMENTATION.md
  • Discussion: Issue Daily Firewall Report - October 28, 2025 #2662

AI generated by Q

The analyzeFirewallLogs() function was looking for directories named
exactly 'squid-logs' or 'firewall-logs', but workflow artifacts are
uploaded with workflow-specific suffixes like:
- squid-logs-smoke-copilot-firewall
- squid-logs-changeset-generator
- firewall-logs-{workflow-name}

This caused firewall logs to be uploaded but not parsed, resulting in
'firewall_analysis: null' in run_summary.json files.

Changes:
- Modified directory discovery to use prefix matching instead of exact match
- Added verbose logging when firewall log directory is found
- Added comprehensive test case TestAnalyzeFirewallLogsWithWorkflowSuffix()
- Added detailed comments explaining workflow-specific suffix pattern

Impact:
- Firewall logs will now be parsed correctly regardless of suffix
- gh aw logs command will show firewall statistics
- gh aw audit command will include firewall analysis

Fixes: Firewall logs uploaded but not analyzed
Related: FIREWALL_LOG_PARSER_IMPLEMENTATION.md
@pelikhan pelikhan merged commit 834b4d2 into main Oct 28, 2025
4 checks passed
@pelikhan pelikhan deleted the q/fix-firewall-log-parsing-with-workflow-suffixes-307c8d3b304464a2 branch October 28, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant