Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/cli/context_cancellation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TestDownloadWorkflowLogsWithCancellation(t *testing.T) {
cancel()

// Try to download logs with a cancelled context
err := DownloadWorkflowLogs(ctx, "", 10, "", "", "/tmp/test-logs", "", "", 0, 0, "", false, false, false, false, false, false, false, 0, "", "", false, false, "", nil)
err := DownloadWorkflowLogs(ctx, "", 10, "", "", "/tmp/test-logs", "", "", 0, 0, "", false, false, false, false, false, false, false, 0, "", "", false, false, "", nil, nil)

// Should return context.Canceled error
assert.ErrorIs(t, err, context.Canceled, "Should return context.Canceled error when context is cancelled")
Expand Down Expand Up @@ -111,7 +111,7 @@ func TestDownloadWorkflowLogsTimeoutRespected(t *testing.T) {

start := time.Now()
// Use a workflow name that doesn't exist to avoid actual network calls
_ = DownloadWorkflowLogs(ctx, "nonexistent-workflow-12345", 100, "", "", "/tmp/test-logs", "", "", 0, 0, "", false, false, false, false, false, false, false, 1, "", "", false, false, "", nil)
_ = DownloadWorkflowLogs(ctx, "nonexistent-workflow-12345", 100, "", "", "/tmp/test-logs", "", "", 0, 0, "", false, false, false, false, false, false, false, 1, "", "", false, false, "", nil, nil)
elapsed := time.Since(start)

// Should complete within reasonable time (give 5 seconds buffer for test overhead)
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/logs_ci_scenario_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func TestLogsJSONOutputWithNoRuns(t *testing.T) {
false, // train
"", // format
nil, // artifactSets
nil, // excludeWorkflows
)

// Restore stdout and read output
Expand Down
34 changes: 33 additions & 1 deletion pkg/cli/logs_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ Examples:
` + string(constants.CLIExtensionPrefix) + ` logs --filtered-integrity # Filter logs containing items that were filtered by gateway integrity checks
` + string(constants.CLIExtensionPrefix) + ` logs --no-staged # Exclude staged workflow runs from results

# Excluding specific workflows
` + string(constants.CLIExtensionPrefix) + ` logs --exclude ci-tests # Exclude a specific workflow from results
` + string(constants.CLIExtensionPrefix) + ` logs --exclude ci-tests,nightly-build # Exclude multiple workflows

# Run ID range filtering
` + string(constants.CLIExtensionPrefix) + ` logs --after-run-id 1000 # Filter runs after run ID 1000
` + string(constants.CLIExtensionPrefix) + ` logs --before-run-id 2000 # Filter runs before run ID 2000
Expand Down Expand Up @@ -155,6 +159,7 @@ Examples:
train, _ := cmd.Flags().GetBool("train")
format, _ := cmd.Flags().GetString("format")
artifacts, _ := cmd.Flags().GetStringSlice("artifacts")
excludeWorkflows, _ := cmd.Flags().GetStringSlice("exclude")

// Resolve relative dates to absolute dates for GitHub CLI
now := time.Now()
Expand Down Expand Up @@ -189,7 +194,33 @@ Examples:

logsCommandLog.Printf("Executing logs download: workflow=%s, count=%d, engine=%s, train=%v", workflowName, count, engine, train)

return DownloadWorkflowLogs(cmd.Context(), workflowName, count, startDate, endDate, outputDir, engine, ref, beforeRunID, afterRunID, repoOverride, verbose, toolGraph, noStaged, firewallOnly, noFirewall, parse, jsonOutput, timeout, summaryFile, safeOutputType, filteredIntegrity, train, format, artifacts)
return DownloadWorkflowLogsWithOptions(cmd.Context(), DownloadWorkflowLogsOptions{
WorkflowName: workflowName,
Count: count,
StartDate: startDate,
EndDate: endDate,
OutputDir: outputDir,
Engine: engine,
Ref: ref,
BeforeRunID: beforeRunID,
AfterRunID: afterRunID,
RepoOverride: repoOverride,
Verbose: verbose,
ToolGraph: toolGraph,
NoStaged: noStaged,
FirewallOnly: firewallOnly,
NoFirewall: noFirewall,
Parse: parse,
JSONOutput: jsonOutput,
Timeout: timeout,
SummaryFile: summaryFile,
SafeOutputType: safeOutputType,
FilteredIntegrity: filteredIntegrity,
Train: train,
Format: format,
ArtifactSets: artifacts,
ExcludeWorkflows: excludeWorkflows,
})
},
}

Expand Down Expand Up @@ -217,6 +248,7 @@ Examples:
logsCmd.Flags().String("format", "", "Output format for cross-run audit report: pretty, markdown (generates security audit report instead of default metrics table)")
logsCmd.Flags().Int("last", 0, "Alias for --count: number of recent runs to download")
logsCmd.Flags().StringSlice("artifacts", nil, "Artifact sets to download (default: all). Valid sets: "+strings.Join(ValidArtifactSetNames(), ", "))
logsCmd.Flags().StringSlice("exclude", nil, "Workflow names or IDs to exclude from results (comma-separated or repeated flag). Excluded runs are skipped before artifact download, saving API resources.")
logsCmd.MarkFlagsMutuallyExclusive("firewall", "no-firewall")

// Register completions for logs command
Expand Down
128 changes: 128 additions & 0 deletions pkg/cli/logs_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,3 +264,131 @@ func TestLogsCommandHelpText(t *testing.T) {
assert.Contains(t, safeOutputFlag.Usage, "noop", "safe-output flag help should mention noop")
assert.Contains(t, safeOutputFlag.Usage, "report-incomplete", "safe-output flag help should mention report-incomplete")
}

func TestLogsCommandExcludeFlag(t *testing.T) {
cmd := NewLogsCommand()
flags := cmd.Flags()

// Verify the --exclude flag exists
excludeFlag := flags.Lookup("exclude")
require.NotNil(t, excludeFlag, "Should have 'exclude' flag")
assert.Equal(t, "stringSlice", excludeFlag.Value.Type(), "exclude should be stringSlice type")
assert.Equal(t, "[]", excludeFlag.DefValue, "exclude should default to empty slice")
assert.Contains(t, excludeFlag.Usage, "exclude", "exclude flag usage should describe its purpose")
}

func TestIsWorkflowExcluded(t *testing.T) {
tests := []struct {
name string
workflowName string
excludes []string
expected bool
}{
{
name: "exact match",
workflowName: "Weekly Research",
excludes: []string{"Weekly Research"},
expected: true,
},
{
name: "case-insensitive match",
workflowName: "Weekly Research",
excludes: []string{"weekly research"},
expected: true,
},
{
name: "slug match - id matches display name",
workflowName: "Weekly Research",
excludes: []string{"weekly-research"},
expected: true,
},
{
name: "slug match - multi-word id",
workflowName: "CI Failure Doctor",
excludes: []string{"ci-failure-doctor"},
expected: true,
},
{
name: "no match",
workflowName: "Weekly Research",
excludes: []string{"Nightly Build"},
expected: false,
},
{
name: "empty excludes",
workflowName: "Weekly Research",
excludes: nil,
expected: false,
},
{
name: "multiple excludes, one matches",
workflowName: "CI Tests",
excludes: []string{"Weekly Research", "CI Tests", "Nightly Build"},
expected: true,
},
{
name: "multiple excludes, none match",
workflowName: "My Workflow",
excludes: []string{"Weekly Research", "CI Tests"},
expected: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := isWorkflowExcluded(tt.workflowName, tt.excludes)
assert.Equal(t, tt.expected, result, "isWorkflowExcluded(%q, %v) should be %v", tt.workflowName, tt.excludes, tt.expected)
})
}
}

func TestResolveExcludeWorkflows(t *testing.T) {
tests := []struct {
name string
excludes []string
expected []string
}{
{
name: "nil input returns nil",
excludes: nil,
expected: nil,
},
{
name: "empty slice returns nil",
excludes: []string{},
expected: nil,
},
{
name: "blank entries are dropped",
excludes: []string{" ", ""},
expected: []string{},
},
{
name: "unknown workflow kept as raw value",
excludes: []string{"no-such-workflow"},
expected: []string{"no-such-workflow"},
},
{
name: "whitespace is trimmed from raw values",
excludes: []string{" my-workflow "},
expected: []string{"my-workflow"},
},
{
name: "multiple unknown workflows all kept",
excludes: []string{"alpha", "beta"},
expected: []string{"alpha", "beta"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := resolveExcludeWorkflows(tt.excludes, false)
if tt.expected == nil {
assert.Nil(t, result, "resolveExcludeWorkflows(%v) should return nil", tt.excludes)
} else {
require.NotNil(t, result, "resolveExcludeWorkflows(%v) should not return nil", tt.excludes)
assert.Equal(t, tt.expected, result, "resolveExcludeWorkflows(%v) should return %v", tt.excludes, tt.expected)
}
})
}
}
4 changes: 2 additions & 2 deletions pkg/cli/logs_download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestDownloadWorkflowLogs(t *testing.T) {
// Test the DownloadWorkflowLogs function
// This should either fail with auth error (if not authenticated)
// or succeed with no results (if authenticated but no workflows match)
err := DownloadWorkflowLogs(context.Background(), "", 1, "", "", "./test-logs", "", "", 0, 0, "", false, false, false, false, false, false, false, 0, "summary.json", "", false, false, "", nil)
err := DownloadWorkflowLogs(context.Background(), "", 1, "", "", "./test-logs", "", "", 0, 0, "", false, false, false, false, false, false, false, 0, "summary.json", "", false, false, "", nil, nil)

// If GitHub CLI is authenticated, the function may succeed but find no results
// If not authenticated, it should return an auth error
Expand Down Expand Up @@ -393,7 +393,7 @@ func TestDownloadWorkflowLogsWithEngineFilter(t *testing.T) {
if !tt.expectError {
// For valid engines, test that the function can be called without panic
// It may still fail with auth errors, which is expected
err := DownloadWorkflowLogs(context.Background(), "", 1, "", "", "./test-logs", tt.engine, "", 0, 0, "", false, false, false, false, false, false, false, 0, "summary.json", "", false, false, "", nil)
err := DownloadWorkflowLogs(context.Background(), "", 1, "", "", "./test-logs", tt.engine, "", 0, 0, "", false, false, false, false, false, false, false, 0, "summary.json", "", false, false, "", nil, nil)

// Clean up any created directories
os.RemoveAll("./test-logs")
Expand Down
2 changes: 2 additions & 0 deletions pkg/cli/logs_json_stderr_order_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func TestLogsJSONOutputBeforeStderr(t *testing.T) {
false, // train
"", // format
nil, // artifactSets
nil, // excludeWorkflows
)

// Close writers first
Expand Down Expand Up @@ -187,6 +188,7 @@ func TestLogsJSONAndStderrRedirected(t *testing.T) {
false, // train
"", // format
nil, // artifactSets
nil, // excludeWorkflows
)

// Close the writer
Expand Down
124 changes: 121 additions & 3 deletions pkg/cli/logs_orchestrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,98 @@ func getMaxConcurrentDownloads() int {
return envutil.GetIntFromEnv("GH_AW_MAX_CONCURRENT_DOWNLOADS", MaxConcurrentDownloads, 1, 100, logsOrchestratorLog)
}

// DownloadWorkflowLogs downloads and analyzes workflow logs with metrics
func DownloadWorkflowLogs(ctx context.Context, workflowName string, count int, startDate, endDate, outputDir, engine, ref string, beforeRunID, afterRunID int64, repoOverride string, verbose bool, toolGraph bool, noStaged bool, firewallOnly bool, noFirewall bool, parse bool, jsonOutput bool, timeout int, summaryFile string, safeOutputType string, filteredIntegrity bool, train bool, format string, artifactSets []string) error {
logsOrchestratorLog.Printf("Starting workflow log download: workflow=%s, count=%d, startDate=%s, endDate=%s, outputDir=%s, summaryFile=%s, safeOutputType=%s, filteredIntegrity=%v, train=%v, format=%s, artifactSets=%v", workflowName, count, startDate, endDate, outputDir, summaryFile, safeOutputType, filteredIntegrity, train, format, artifactSets)
// DownloadWorkflowLogsOptions groups all configuration for DownloadWorkflowLogs.
// Using a struct avoids a long positional parameter list and makes future additions
// non-breaking at call sites.
type DownloadWorkflowLogsOptions struct {
WorkflowName string
Count int
StartDate string
EndDate string
OutputDir string
Engine string
Ref string
BeforeRunID int64
AfterRunID int64
RepoOverride string
Verbose bool
ToolGraph bool
NoStaged bool
FirewallOnly bool
NoFirewall bool
Parse bool
JSONOutput bool
Timeout int
SummaryFile string
SafeOutputType string
FilteredIntegrity bool
Train bool
Format string
ArtifactSets []string
ExcludeWorkflows []string
}

// DownloadWorkflowLogs downloads and analyzes workflow logs with metrics.
// It is a thin wrapper around DownloadWorkflowLogsWithOptions that maps the
// positional arguments into DownloadWorkflowLogsOptions.
func DownloadWorkflowLogs(ctx context.Context, workflowName string, count int, startDate, endDate, outputDir, engine, ref string, beforeRunID, afterRunID int64, repoOverride string, verbose bool, toolGraph bool, noStaged bool, firewallOnly bool, noFirewall bool, parse bool, jsonOutput bool, timeout int, summaryFile string, safeOutputType string, filteredIntegrity bool, train bool, format string, artifactSets []string, excludeWorkflows []string) error {
return DownloadWorkflowLogsWithOptions(ctx, DownloadWorkflowLogsOptions{
WorkflowName: workflowName,
Count: count,
StartDate: startDate,
EndDate: endDate,
OutputDir: outputDir,
Engine: engine,
Ref: ref,
BeforeRunID: beforeRunID,
AfterRunID: afterRunID,
RepoOverride: repoOverride,
Verbose: verbose,
ToolGraph: toolGraph,
NoStaged: noStaged,
FirewallOnly: firewallOnly,
NoFirewall: noFirewall,
Parse: parse,
JSONOutput: jsonOutput,
Timeout: timeout,
SummaryFile: summaryFile,
SafeOutputType: safeOutputType,
FilteredIntegrity: filteredIntegrity,
Train: train,
Format: format,
ArtifactSets: artifactSets,
ExcludeWorkflows: excludeWorkflows,
})
}

// DownloadWorkflowLogsWithOptions downloads and analyzes workflow logs with metrics.
func DownloadWorkflowLogsWithOptions(ctx context.Context, opts DownloadWorkflowLogsOptions) error {
workflowName := opts.WorkflowName
count := opts.Count
startDate := opts.StartDate
endDate := opts.EndDate
outputDir := opts.OutputDir
engine := opts.Engine
ref := opts.Ref
beforeRunID := opts.BeforeRunID
afterRunID := opts.AfterRunID
repoOverride := opts.RepoOverride
verbose := opts.Verbose
toolGraph := opts.ToolGraph
noStaged := opts.NoStaged
firewallOnly := opts.FirewallOnly
noFirewall := opts.NoFirewall
parse := opts.Parse
jsonOutput := opts.JSONOutput
timeout := opts.Timeout
summaryFile := opts.SummaryFile
safeOutputType := opts.SafeOutputType
filteredIntegrity := opts.FilteredIntegrity
train := opts.Train
format := opts.Format
artifactSets := opts.ArtifactSets
excludeWorkflows := opts.ExcludeWorkflows
logsOrchestratorLog.Printf("Starting workflow log download: workflow=%s, count=%d, startDate=%s, endDate=%s, outputDir=%s, summaryFile=%s, safeOutputType=%s, filteredIntegrity=%v, train=%v, format=%s, artifactSets=%v, excludeWorkflows=%v", workflowName, count, startDate, endDate, outputDir, summaryFile, safeOutputType, filteredIntegrity, train, format, artifactSets, excludeWorkflows)

// Validate and resolve artifact sets into a concrete filter (list of artifact base names).
if err := ValidateArtifactSets(artifactSets); err != nil {
Expand All @@ -50,6 +139,18 @@ func DownloadWorkflowLogs(ctx context.Context, workflowName string, count int, s
}
}

// Resolve excluded workflow names to display names (for matching against WorkflowRun.WorkflowName).
// Each entry in excludeWorkflows may be a workflow ID (e.g., "weekly-research") or a display name
// (e.g., "Weekly Research"). We try to resolve each to its canonical display name; if resolution
// fails (e.g., no .lock.yml files present), we fall back to case-insensitive matching of the raw value.
resolvedExcludes := resolveExcludeWorkflows(excludeWorkflows, verbose)
if len(resolvedExcludes) > 0 {
logsOrchestratorLog.Printf("Exclude filter active: %v", resolvedExcludes)
if verbose {
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Exclude filter: skipping workflows: "+strings.Join(resolvedExcludes, ", ")))
}
}

// Ensure .github/aw/logs/.gitignore exists on every invocation
if err := ensureLogsGitignore(); err != nil {
// Log but don't fail - this is not critical for downloading logs
Expand Down Expand Up @@ -193,6 +294,23 @@ func DownloadWorkflowLogs(ctx context.Context, workflowName string, count int, s
// forcing us to scan the entire batch.
batchProcessed := 0
runsRemaining := runs

// Apply exclude filter before chunking to avoid downloading artifacts for excluded workflows.
if len(resolvedExcludes) > 0 {
filteredRuns := make([]WorkflowRun, 0, len(runsRemaining))
for _, run := range runsRemaining {
if isWorkflowExcluded(run.WorkflowName, resolvedExcludes) {
logsOrchestratorLog.Printf("Skipping run %d: workflow '%s' is in exclude list", run.DatabaseID, run.WorkflowName)
if verbose {
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping run %d: workflow '%s' is excluded by --exclude", run.DatabaseID, run.WorkflowName)))
}
continue
}
filteredRuns = append(filteredRuns, run)
}
runsRemaining = filteredRuns
}

for len(runsRemaining) > 0 && len(processedRuns) < count {
remainingNeeded := count - len(processedRuns)
if remainingNeeded <= 0 {
Expand Down
Loading