Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
54 changes: 32 additions & 22 deletions cmd/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,10 @@ func ApplyCoverage(recs []common.Recommendation, coverage float64) []common.Reco
//
// 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 @@ -252,12 +255,14 @@ func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64) []comm
unsupportedSeen := make(map[common.CommitmentType]bool)

for _, rec := range recs {
adjusted, kept, missingSignal := applyTargetCoverageOne(rec, targetPct, unsupportedSeen)
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 @@ -270,51 +275,53 @@ func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64) []comm
}

// 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) {
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) {
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 @@ -343,7 +350,7 @@ func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common
// 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 @@ -363,7 +370,7 @@ func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common
// 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 @@ -399,7 +406,7 @@ func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common
}
adjusted.ProjectedUtilization = projUtil
adjusted.ProjectedCoverage = projCov
return adjusted, true
return adjusted, true, ""
}

// applyTargetCoverageSP is the SP branch of ApplyTargetCoverage. Returns
Expand Down Expand Up @@ -460,9 +467,12 @@ func applyTargetCoverageSP(rec common.Recommendation, targetPct float64) (common
// (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
26 changes: 13 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,7 +1374,7 @@ 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")
Expand Down
13 changes: 9 additions & 4 deletions cmd/multi_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,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 @@ -389,9 +393,10 @@ func filterAndAdjustRecommendations(recommendations []common.Recommendation, csv
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(recommendations)
recommendations = applyFilters(recommendations, cfg, instanceVersions, versionInfo, "")
recommendations = applyFilters(recommendations, cfg, instanceVersions, versionInfo, "", nil)
if len(recommendations) < originalCount {
AppLogger.Printf("🔍 After filters: %d recommendations (filtered out %d)\n", len(recommendations), originalCount-len(recommendations))
}
Expand All @@ -402,7 +407,7 @@ func filterAndAdjustRecommendations(recommendations []common.Recommendation, csv
// CSV-path short-circuit is conditional on TargetCoverage == 0.
if cfg.TargetCoverage > 0 || csvModeCoverage < 100 {
beforeSize := len(recommendations)
recommendations = applySizing(recommendations, cfg, csvModeCoverage)
recommendations = applySizing(recommendations, cfg, csvModeCoverage, nil)
if cfg.TargetCoverage > 0 {
AppLogger.Printf("🎯 Applying %.1f%% target-coverage: %d recommendations selected (from %d)\n", cfg.TargetCoverage, len(recommendations), beforeSize)
} else {
Expand Down
Loading
Loading