Skip to content
57 changes: 33 additions & 24 deletions cmd/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,10 @@
//
// Recs of any other CommitmentType are passed through unmodified (warned
// once per type per run).
func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64) []common.Recommendation {
// ApplyTargetCoverage applies the target coverage percentage to a slice of
// recommendations. drops accumulates per-reason drop counts for the
// end-of-run summary; pass nil to skip tracking.
func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64, drops *common.DropSummary) []common.Recommendation {
if targetPct <= 0 || targetPct > 100 {
// Validation ensures we never get here in production, but be defensive
// so a buggy caller doesn't divide by zero.
Expand All @@ -253,14 +256,15 @@
var skipped int
unsupportedSeen := make(map[common.CommitmentType]bool)

for _rvc := range recs {
rec := recs[_rvc]
adjusted, kept, missingSignal := applyTargetCoverageOne(rec, targetPct, unsupportedSeen)
for _, rec := range recs {

Check failure on line 259 in cmd/helpers.go

View workflow job for this annotation

GitHub Actions / Lint Code

rangeValCopy: each iteration copies 360 bytes (consider pointers or indexing) (gocritic)
adjusted, kept, missingSignal, dropReason := applyTargetCoverageOne(rec, targetPct, unsupportedSeen)
if missingSignal {
skipped++
}
if kept {
result = append(result, adjusted)
} else if dropReason != "" {
drops.Add(dropReason, 1)
}
}

Expand All @@ -273,51 +277,53 @@
}

// applyTargetCoverageOne dispatches a single recommendation through the
// appropriate branch. Returns (rec, kept, missingSignal):
// appropriate branch. Returns (rec, kept, missingSignal, dropReason):
// - kept=true → caller appends `rec` (the adjusted or pass-through value).
// - kept=false → caller drops the rec (only the RI "target unreachable"
// branch returns this; an INFO log already fired).
// branches return this; an INFO log already fired).
// - missingSignal=true → counted toward the end-of-run skip summary.
// - dropReason is non-empty when kept=false and the drop has a named category.
//
// Split out of ApplyTargetCoverage to keep that function under gocyclo's
// complexity threshold.
func applyTargetCoverageOne(rec common.Recommendation, targetPct float64, unsupportedSeen map[common.CommitmentType]bool) (common.Recommendation, bool, bool) {
func applyTargetCoverageOne(rec common.Recommendation, targetPct float64, unsupportedSeen map[common.CommitmentType]bool) (common.Recommendation, bool, bool, string) {

Check failure on line 289 in cmd/helpers.go

View workflow job for this annotation

GitHub Actions / Lint Code

unnamedResult: consider giving a name to these results (gocritic)
switch {
case common.IsSavingsPlan(rec.Service):
adjusted, ok := applyTargetCoverageSP(rec, targetPct)
if !ok {
// SP no-signal: pass through unchanged.
return rec, true, true
return rec, true, true, ""
}
return adjusted, true, false
return adjusted, true, false, ""
case rec.CommitmentType == common.CommitmentReservedInstance:
adjusted, ok := applyTargetCoverageRI(rec, targetPct)
adjusted, ok, dropReason := applyTargetCoverageRI(rec, targetPct)
if !ok {
// Distinguish "no signal" (pass through, count in summary) from
// "target unreachable" (drop with already-fired INFO log).
if rec.AverageInstancesUsedPerHour <= 0 {
return rec, true, true
return rec, true, true, ""
}
return rec, false, false
return rec, false, false, dropReason
}
return adjusted, true, false
return adjusted, true, false, ""
default:
if !unsupportedSeen[rec.CommitmentType] {
AppLogger.Printf("WARNING: --target-coverage not supported for CommitmentType=%q; passing recommendations through unchanged\n", rec.CommitmentType)
unsupportedSeen[rec.CommitmentType] = true
}
return rec, true, false
return rec, true, false, ""
}
}

// applyTargetCoverageRI is the RI branch of ApplyTargetCoverage. Returns
// (adjusted, true) on success, (rec, false) when the rec should be passed
// through unscaled (no signal) or dropped (target unreachable). Caller
// distinguishes the two via rec.AverageInstancesUsedPerHour.
func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common.Recommendation, bool) {
// (adjusted, true, "") on success, (rec, false, dropReason) when the rec
// should be passed through unscaled (no signal) or dropped (target
// unreachable). Caller distinguishes no-signal from drop via
// rec.AverageInstancesUsedPerHour and uses dropReason for the summary.
func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common.Recommendation, bool, string) {

Check failure on line 323 in cmd/helpers.go

View workflow job for this annotation

GitHub Actions / Lint Code

unnamedResult: consider giving a name to these results (gocritic)
if rec.AverageInstancesUsedPerHour <= 0 {
// No signal — caller will pass through and count in the summary.
return rec, false
return rec, false, ""
}

avg := rec.AverageInstancesUsedPerHour
Expand Down Expand Up @@ -346,7 +352,7 @@
// pass through".
AppLogger.Printf("INFO: --target-coverage=%.1f%% already met by existing coverage %.1f%% for %s/%s/%s; dropped recommendation\n",
targetPct, rec.ExistingCoveragePct, rec.Service, rec.Region, rec.ResourceType)
return rec, false
return rec, false, common.DropTargetAlreadyMet
}
// Floor so we never over-shoot the target on integer-arithmetic edges.
// Strict-target semantics: 80% means "at most 80% coverage", not "at
Expand All @@ -366,7 +372,7 @@
// Returning (_, false) with avg > 0 signals "drop, don't pass through".
// applyTargetCoverageRI's caller branches on
// rec.AverageInstancesUsedPerHour to distinguish drop vs no-signal.
return rec, false
return rec, false, common.DropTargetSizedToZero
}

// Cost-bearing fields scale by the ratio of sized-to-original count, so the
Expand Down Expand Up @@ -402,7 +408,7 @@
}
adjusted.ProjectedUtilization = projUtil
adjusted.ProjectedCoverage = projCov
return adjusted, true
return adjusted, true, ""
}

// applyTargetCoverageSP is the SP branch of ApplyTargetCoverage. Returns
Expand Down Expand Up @@ -463,9 +469,12 @@
// (the main path passes cfg.Coverage; the CSV path passes csvModeCoverage,
// which substitutes the default 80% with 100% so CSV-driven counts aren't
// silently dropped).
func applySizing(recs []common.Recommendation, cfg Config, coverage float64) []common.Recommendation {
//
// drops accumulates per-reason drop counts for the end-of-run summary.
// Pass nil to skip tracking.
func applySizing(recs []common.Recommendation, cfg Config, coverage float64, drops *common.DropSummary) []common.Recommendation {
if cfg.TargetCoverage > 0 {
return ApplyTargetCoverage(recs, cfg.TargetCoverage)
return ApplyTargetCoverage(recs, cfg.TargetCoverage, drops)
}
return ApplyCoverage(recs, coverage)
}
Expand Down
78 changes: 65 additions & 13 deletions cmd/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,7 @@ func TestApplyTargetCoverage_RI(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
recs := []common.Recommendation{tt.rec}
out := ApplyTargetCoverage(recs, tt.target)
out := ApplyTargetCoverage(recs, tt.target, nil)
if tt.wantDropped {
if len(out) != 0 {
t.Fatalf("expected drop; got %d recs", len(out))
Expand Down Expand Up @@ -1072,7 +1072,7 @@ func TestApplyTargetCoverage_RI_CostScaling(t *testing.T) {
// n = floor(8 * 80/100) = 6. Ratio = 6/10 = 0.6 (cost scaling still
// uses rec.Count to convert AWS's quoted cost-for-rec.Count into
// cost-for-nTarget).
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
require.Len(t, out, 1)
assert.Equal(t, 6, out[0].Count)
assert.InDelta(t, 600.0, out[0].CommitmentCost, 0.001, "CommitmentCost scales by nTarget/rec.Count")
Expand All @@ -1088,7 +1088,7 @@ func TestApplyTargetCoverage_RI_CostScaling(t *testing.T) {
monthly := 50.0
recWithMonthly := rec
recWithMonthly.RecurringMonthlyCost = &monthly
out := ApplyTargetCoverage([]common.Recommendation{recWithMonthly}, 80)
out := ApplyTargetCoverage([]common.Recommendation{recWithMonthly}, 80, nil)
require.Len(t, out, 1)
require.NotNil(t, out[0].RecurringMonthlyCost, "scaled pointer should be non-nil")
assert.InDelta(t, 30.0, *out[0].RecurringMonthlyCost, 0.001, "monthly cost scales by 6/10")
Expand All @@ -1100,7 +1100,7 @@ func TestApplyTargetCoverage_RI_CostScaling(t *testing.T) {
// AWS API didn't return RecurringStandardMonthlyCost (all-upfront,
// or field missing). The sized rec should also have nil so
// downstream renders "unknown" rather than zero.
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
require.Len(t, out, 1)
assert.Nil(t, out[0].RecurringMonthlyCost, "nil input → nil output")
})
Expand Down Expand Up @@ -1191,7 +1191,7 @@ func TestApplyTargetCoverage_RI_ExistingCoverage(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, tt.target)
out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, tt.target, nil)
if tt.wantDropped {
assert.Len(t, out, 0, "expected drop")
return
Expand Down Expand Up @@ -1226,7 +1226,7 @@ func TestApplyTargetCoverage_SP(t *testing.T) {
// flag's intent is "leave 20% headroom", so the commitment shrinks
// to 80% of AWS rec. All cost-bearing fields scale by 0.8.
// Projected util = 95/0.80 = 118.75 clamped to 100.
out := ApplyTargetCoverage([]common.Recommendation{mkSP(95)}, 80)
out := ApplyTargetCoverage([]common.Recommendation{mkSP(95)}, 80, nil)
require.Len(t, out, 1)
assert.InDelta(t, 1.6, out[0].Details.(*common.SavingsPlanDetails).HourlyCommitment, 0.001)
assert.InDelta(t, 800.0, out[0].CommitmentCost, 0.001, "CommitmentCost scales by target/100")
Expand All @@ -1240,7 +1240,7 @@ func TestApplyTargetCoverage_SP(t *testing.T) {
t.Run("AWS below target — scale down by target (under-buy)", func(t *testing.T) {
// RecUtil=50, target=80. All cost-bearing fields shrink to 80%.
// Projected util = 50/0.80 = 62.5 (no clamp needed).
out := ApplyTargetCoverage([]common.Recommendation{mkSP(50)}, 80)
out := ApplyTargetCoverage([]common.Recommendation{mkSP(50)}, 80, nil)
require.Len(t, out, 1)
details := out[0].Details.(*common.SavingsPlanDetails)
assert.InDelta(t, 1.6, details.HourlyCommitment, 0.001)
Expand All @@ -1253,7 +1253,7 @@ func TestApplyTargetCoverage_SP(t *testing.T) {
})

t.Run("no signal → passed through unchanged", func(t *testing.T) {
out := ApplyTargetCoverage([]common.Recommendation{mkSP(0)}, 80)
out := ApplyTargetCoverage([]common.Recommendation{mkSP(0)}, 80, nil)
require.Len(t, out, 1)
// Original recommendation values intact.
assert.Equal(t, 2.0, out[0].Details.(*common.SavingsPlanDetails).HourlyCommitment)
Expand All @@ -1274,7 +1274,7 @@ func TestApplySizing(t *testing.T) {

t.Run("TargetCoverage > 0 → ApplyTargetCoverage", func(t *testing.T) {
cfg := Config{TargetCoverage: 80, Coverage: 100}
out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage)
out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage, nil)
require.Len(t, out, 1)
// avg=8, target=80%, existing=0%. gap=80.
// n = floor(8 * 80/100) = floor(6.4) = 6. ProjUtil = 8/6 = 133% → 100.
Expand All @@ -1284,7 +1284,7 @@ func TestApplySizing(t *testing.T) {

t.Run("TargetCoverage == 0 → ApplyCoverage", func(t *testing.T) {
cfg := Config{TargetCoverage: 0, Coverage: 50}
out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage)
out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage, nil)
require.Len(t, out, 1)
// ApplyCoverage(50) on count=10 → 5. ProjectedUtilization NOT set
// (zero) because we took the coverage branch.
Expand Down Expand Up @@ -1329,7 +1329,7 @@ func TestApplyTargetCoverage_RI_Target100(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, 100)
out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, 100, nil)
if tt.wantDropped {
assert.Len(t, out, 0, "expected drop at target=100 for avg=%.3f", tt.rec.AverageInstancesUsedPerHour)
return
Expand All @@ -1353,7 +1353,7 @@ func TestApplyTargetCoverage_SP_NoSignalGuards(t *testing.T) {
RecommendedUtilization: 50,
Details: &common.SavingsPlanDetails{HourlyCommitment: 0},
}
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
require.Len(t, out, 1, "$0 SP rec should still be in output (pass-through)")
// Pass-through — projection fields must NOT be set, savings unchanged.
assert.Equal(t, 0.0, out[0].ProjectedUtilization, "ProjectedUtilization must NOT be set for $0-commitment pass-through")
Expand All @@ -1374,9 +1374,61 @@ func TestApplyTargetCoverage_SP_NoSignalGuards(t *testing.T) {
RecommendedUtilization: 50,
Details: common.ComputeDetails{Platform: "Linux/UNIX"}, // wrong type
}
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
require.Len(t, out, 1)
assert.Equal(t, 0.0, out[0].ProjectedUtilization, "must NOT set projection when scaling failed")
assert.Equal(t, 1500.0, out[0].EstimatedSavings, "EstimatedSavings must remain unscaled when scaling failed")
})
}

// TestApplyTargetCoverage_DropTargetAlreadyMet verifies that when existing
// coverage already meets or exceeds the target, the recommendation is
// dropped and the drop is recorded in a non-nil DropSummary under the
// DropTargetAlreadyMet category. If the drops.Add call for that branch
// were removed, d.Total() would stay at 0 and the assertion below would fail.
func TestApplyTargetCoverage_DropTargetAlreadyMet(t *testing.T) {
// ExistingCoveragePct=90 >= target=80: gapPct=80-90=-10 <= 0 -> drop.
rec := common.Recommendation{
Service: common.ServiceEC2,
Region: "us-east-1",
ResourceType: "t3.medium",
Count: 5,
CommitmentType: common.CommitmentReservedInstance,
AverageInstancesUsedPerHour: 8.0,
ExistingCoveragePct: 90.0,
}

d := common.NewDropSummary()
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, d)

assert.Empty(t, out, "already-covered rec should be dropped")
assert.Equal(t, 1, d.Total(), "drop summary should record 1 drop")
assert.Contains(t, d.FormatOneLine(), common.DropTargetAlreadyMet,
"drop summary should name the target-already-met category")
}

// TestApplyTargetCoverage_DropTargetSizedToZero verifies that when the
// floor(avg * gapPct / 100) formula produces 0, the recommendation is
// dropped and the drop is recorded in a non-nil DropSummary under the
// DropTargetSizedToZero category. If the drops.Add call for that branch
// were removed, d.Total() would stay at 0 and the assertion below would fail.
func TestApplyTargetCoverage_DropTargetSizedToZero(t *testing.T) {
// avg=0.4, target=80%, existing=0%: floor(0.4 * 80/100) = floor(0.32) = 0 -> drop.
rec := common.Recommendation{
Service: common.ServiceEC2,
Region: "us-east-1",
ResourceType: "t3.micro",
Count: 1,
CommitmentType: common.CommitmentReservedInstance,
AverageInstancesUsedPerHour: 0.4,
ExistingCoveragePct: 0.0,
}

d := common.NewDropSummary()
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, d)

assert.Empty(t, out, "floor-to-zero rec should be dropped")
assert.Equal(t, 1, d.Total(), "drop summary should record 1 drop")
assert.Contains(t, d.FormatOneLine(), common.DropTargetSizedToZero,
"drop summary should name the target-sized-to-zero category")
}
23 changes: 17 additions & 6 deletions cmd/multi_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,11 @@ func runToolMultiService(ctx context.Context, cfg Config) {

// Phase 1: collect all recommendations without purchasing.
AppLogger.Printf("\n📥 Fetching recommendations from all services...\n")
allRecs := fetchAllRecs(ctx, awsCfg, recClient, accountCache, servicesToProcess, engineData, cfg, coverageMap)
allRecs, drops := fetchAllRecs(ctx, awsCfg, recClient, accountCache, servicesToProcess, engineData, cfg, coverageMap)

if line := drops.FormatOneLine(); line != "" {
AppLogger.Printf("\n%s\n", line)
}

// Phase 2: score and display.
scoredResult := scoreAndDisplay(allRecs, cfg)
Expand Down Expand Up @@ -159,14 +163,20 @@ func runPurchaseAndReport(ctx context.Context, awsCfg aws.Config, scoredResult s

allResults := executePurchasePipeline(ctx, awsCfg, scoredResult.Passed, isDryRun, runID, cfg)

serviceStats := buildServiceStats(scoredResult.Passed, allResults)
// Produce summary outputs.
writeReportAndSummary(scoredResult.Passed, allResults, isDryRun, cfg)
}

// writeReportAndSummary writes the CSV report and prints the final summary.
func writeReportAndSummary(passed []common.Recommendation, allResults []common.PurchaseResult, isDryRun bool, cfg Config) {
serviceStats := buildServiceStats(passed, allResults)
finalCSVOutput := generateCSVFilename(isDryRun, cfg)
if err := writeMultiServiceCSVReport(allResults, finalCSVOutput); err != nil {
log.Printf("Warning: Failed to write CSV output: %v", err)
} else {
AppLogger.Printf("\n📋 CSV report written to: %s\n", finalCSVOutput)
}
printMultiServiceSummary(scoredResult.Passed, allResults, serviceStats, isDryRun)
printMultiServiceSummary(passed, allResults, serviceStats, isDryRun)
}

// loadAWSConfig builds an aws.Config from the tool config.
Expand Down Expand Up @@ -425,9 +435,10 @@ func filterAndAdjustRecommendations(recs []common.Recommendation, csvModeCoverag
log.Printf("✅ Found support information for %d major engine versions", len(versionInfo))
}

// Apply filters (empty currentRegion since we're processing from CSV, not iterating regions)
// Apply filters (empty currentRegion since we're processing from CSV, not iterating regions).
// Drop tracking is skipped on the CSV path (nil drops).
originalCount := len(recs)
recs = applyFilters(recs, &cfg, instanceVersions, versionInfo, "")
recs = applyFilters(recs, &cfg, instanceVersions, versionInfo, "", nil)
if len(recs) < originalCount {
AppLogger.Printf("🔍 After filters: %d recs (filtered out %d)\n", len(recs), originalCount-len(recs))
}
Expand All @@ -438,7 +449,7 @@ func filterAndAdjustRecommendations(recs []common.Recommendation, csvModeCoverag
// CSV-path short-circuit is conditional on TargetCoverage == 0.
if cfg.TargetCoverage > 0 || csvModeCoverage < 100 {
beforeSize := len(recs)
recs = applySizing(recs, cfg, csvModeCoverage)
recs = applySizing(recs, cfg, csvModeCoverage, nil)
if cfg.TargetCoverage > 0 {
AppLogger.Printf("🎯 Applying %.1f%% target-coverage: %d recs selected (from %d)\n", cfg.TargetCoverage, len(recs), beforeSize)
} else {
Expand Down
Loading
Loading