diff --git a/cmd/helpers.go b/cmd/helpers.go index 40b0e56de..03251c62e 100644 --- a/cmd/helpers.go +++ b/cmd/helpers.go @@ -123,6 +123,12 @@ func CalculateTotalInstances(recs []common.Recommendation) int { // on-demand ratio) and stays unscaled. Pre-sizing values can still be // recovered: RecommendedCount holds AWS's pre-sized count for RIs. func ApplyCoverage(recs []common.Recommendation, coverage float64) []common.Recommendation { + return applyCoverage(recs, coverage, nil) +} + +// applyCoverage applies legacy percentage sizing and optionally records +// recommendations whose discrete RI count is reduced to zero. +func applyCoverage(recs []common.Recommendation, coverage float64, drops *common.DropSummary) []common.Recommendation { if coverage >= 100 { return recs } @@ -170,6 +176,8 @@ func ApplyCoverage(recs []common.Recommendation, coverage float64) []common.Reco adjusted = common.ScaleRecommendationCosts(adjusted, sizedRatio) adjusted.Count = newCount result = append(result, adjusted) + } else if drops != nil { + drops.Add(common.DropTargetSizedToZero, 1) } } return result @@ -241,7 +249,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. @@ -253,14 +264,15 @@ func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64) []comm var skipped int unsupportedSeen := make(map[common.CommitmentType]bool) - for _rvc := range recs { - rec := recs[_rvc] - adjusted, kept, missingSignal := applyTargetCoverageOne(rec, targetPct, unsupportedSeen) + for i := range recs { + adjusted, kept, missingSignal, dropReason := applyTargetCoverageOne(recs[i], targetPct, unsupportedSeen) if missingSignal { skipped++ } if kept { result = append(result, adjusted) + } else if dropReason != "" { + drops.Add(dropReason, 1) } } @@ -273,51 +285,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) (result common.Recommendation, kept, missingSignal bool, drop 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) (result common.Recommendation, ok bool, drop 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 @@ -346,7 +360,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 @@ -366,7 +380,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 @@ -402,7 +416,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 @@ -463,11 +477,14 @@ 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) + return applyCoverage(recs, coverage, drops) } // ApplyCountOverride overrides the count for all recommendations. diff --git a/cmd/helpers_test.go b/cmd/helpers_test.go index c2ab67f91..cc8deba76 100644 --- a/cmd/helpers_test.go +++ b/cmd/helpers_test.go @@ -383,6 +383,21 @@ func TestApplyCoverage_RICostScaling(t *testing.T) { assert.Equal(t, 60.0, monthly) } +func TestApplySizing_LegacyCoverageRecordsCountsFlooredToZero(t *testing.T) { + recs := []common.Recommendation{ + {Service: common.ServiceEC2, Count: 1}, + {Service: common.ServiceRDS, Count: 10}, + } + drops := common.NewDropSummary() + + out := applySizing(recs, Config{}, 10, drops) + + require.Len(t, out, 1) + assert.Equal(t, 1, out[0].Count) + assert.Equal(t, 1, drops.Total()) + assert.Contains(t, drops.FormatOneLine(), common.DropTargetSizedToZero) +} + func TestAdjustRecommendationsForExisting(t *testing.T) { ctx := context.Background() @@ -1028,7 +1043,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)) @@ -1079,7 +1094,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") @@ -1095,7 +1110,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") @@ -1107,7 +1122,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") }) @@ -1198,7 +1213,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 @@ -1233,7 +1248,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") @@ -1247,7 +1262,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) @@ -1260,7 +1275,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) @@ -1281,7 +1296,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. @@ -1291,7 +1306,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. @@ -1336,7 +1351,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 @@ -1360,7 +1375,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") @@ -1381,9 +1396,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") +} diff --git a/cmd/multi_service.go b/cmd/multi_service.go index 77c22c0aa..7c540fa66 100644 --- a/cmd/multi_service.go +++ b/cmd/multi_service.go @@ -131,27 +131,29 @@ 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) // Phase 2: score and display. scoredResult := scoreAndDisplay(allRecs, cfg) if len(scoredResult.Passed) == 0 { + printDropSummary(drops) AppLogger.Printf("\nℹ️ No recommendations passed filters. Nothing to purchase.\n") return } // Phases 3-4: confirm, purchase, and produce summary outputs. - runPurchaseAndReport(ctx, awsCfg, scoredResult, isDryRun, cfg) + runPurchaseAndReport(ctx, awsCfg, scoredResult, isDryRun, cfg, drops) } // runPurchaseAndReport handles the confirm, execute, and report phases of // the multi-service pipeline. It is a separate function to keep // runToolMultiService within the cyclomatic-complexity limit. -func runPurchaseAndReport(ctx context.Context, awsCfg aws.Config, scoredResult scorer.ScoredResult, isDryRun bool, cfg Config) { +func runPurchaseAndReport(ctx context.Context, awsCfg aws.Config, scoredResult scorer.ScoredResult, isDryRun bool, cfg Config, drops *common.DropSummary) { runID := uuid.New().String() if !isDryRun { totalInstances, totalSavings := sumPassedRecs(scoredResult.Passed) if !ConfirmPurchase(totalInstances, totalSavings, cfg.SkipConfirmation) { + printDropSummary(drops) AppLogger.Printf("\n❌ Purchase canceled.\n") return } @@ -159,14 +161,32 @@ 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, drops) +} + +// writeReportAndSummary writes the CSV report and prints the final summary. +func writeReportAndSummary(passed []common.Recommendation, allResults []common.PurchaseResult, isDryRun bool, cfg Config, drops *common.DropSummary) { + 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) + printDropSummary(drops) + printMultiServiceSummary(passed, allResults, serviceStats, isDryRun) +} + +// printDropSummary writes the accumulated drop reasons at the terminal summary +// boundary. A nil or empty summary produces no output. +func printDropSummary(drops *common.DropSummary) { + if drops == nil { + return + } + if line := drops.FormatOneLine(); line != "" { + AppLogger.Printf("\n%s\n", line) + } } // loadAWSConfig builds an aws.Config from the tool config. @@ -425,9 +445,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)) } @@ -438,7 +459,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 { diff --git a/cmd/multi_service_filters.go b/cmd/multi_service_filters.go index 66a26f7f9..ecf0edc58 100644 --- a/cmd/multi_service_filters.go +++ b/cmd/multi_service_filters.go @@ -12,7 +12,8 @@ import ( // applyFilters applies region, instance type, engine, and engine version filters to recommendations. // currentRegion is the region being processed in the current loop iteration; if non-empty, only // recommendations for that region are included. -func applyFilters(recs []common.Recommendation, cfg *Config, instanceVersions map[string][]InstanceEngineVersion, versionInfo map[string]MajorEngineVersionInfo, currentRegion string) []common.Recommendation { +// drops accumulates per-reason drop counts for the end-of-run summary; pass nil to skip tracking. +func applyFilters(recs []common.Recommendation, cfg *Config, instanceVersions map[string][]InstanceEngineVersion, versionInfo map[string]MajorEngineVersionInfo, currentRegion string, drops *common.DropSummary) []common.Recommendation { var filtered []common.Recommendation var poolDropCount int var poolDropInstances float64 @@ -23,11 +24,14 @@ func applyFilters(recs []common.Recommendation, cfg *Config, instanceVersions ma label := fmt.Sprintf("%s/%s/%s", recs[i].Service, recs[i].Region, recs[i].ResourceType) log.Printf("INFO: --min-pool-size=%.1f dropped %s (avg=%.2f < threshold)", cfg.MinPoolSize, label, recs[i].AverageInstancesUsedPerHour) poolDropCount++ + drops.Add(common.DropMinPoolSize, 1) continue } - adjusted, include := processRecommendation(&recs[i], cfg, instanceVersions, versionInfo, currentRegion) + adjusted, include, dropReason := processRecommendation(&recs[i], cfg, instanceVersions, versionInfo, currentRegion) if include { filtered = append(filtered, adjusted) + } else if dropReason != "" { + drops.Add(dropReason, 1) } } @@ -39,19 +43,27 @@ func applyFilters(recs []common.Recommendation, cfg *Config, instanceVersions ma } // processRecommendation applies all filters to a recommendation and returns -// the adjusted recommendation and whether to include it. The flat -// boolean-filter checks are delegated to passesDimensionFilters to keep -// this function under gocyclo's complexity threshold. -func processRecommendation(rec *common.Recommendation, cfg *Config, instanceVersions map[string][]InstanceEngineVersion, versionInfo map[string]MajorEngineVersionInfo, currentRegion string) (common.Recommendation, bool) { +// (adjusted, include, dropReason). dropReason is non-empty only when +// include is false and the drop is worth surfacing in the end-of-run +// summary (dimension-filter mismatches such as region/account/engine are +// expected exclusions and are not counted). The flat boolean-filter checks +// are delegated to passesDimensionFilters to keep this function under +// gocyclo's complexity threshold. +func processRecommendation(rec *common.Recommendation, cfg *Config, instanceVersions map[string][]InstanceEngineVersion, versionInfo map[string]MajorEngineVersionInfo, currentRegion string) (result common.Recommendation, include bool, dropReason string) { // Filter to only recommendations for the current region being processed. // This prevents duplicating recommendations across all regions. - // Skip this filter for Savings Plans as they are account-level, not regional. + // Skip for Savings Plans (account-level, not regional). No drop reason: + // same rec will be returned by its own region's pass. if currentRegion != "" && rec.Region != currentRegion && !common.IsSavingsPlan(rec.Service) { - return *rec, false + return *rec, false, "" } if !passesDimensionFilters(rec, cfg) { - return *rec, false + // Dimension mismatches (region/account/engine/instance-type) are expected + // operator-scoping choices, not drops worth surfacing in the summary. + // --min-pool-size drops are counted separately in applyFilters before + // this function runs, so no drop reason is surfaced here. + return *rec, false, "" } // Apply engine version filters - adjust instance count by subtracting extended support versions. @@ -59,12 +71,12 @@ func processRecommendation(rec *common.Recommendation, cfg *Config, instanceVers adjusted := adjustRecommendationForExcludedVersions(*rec, instanceVersions, versionInfo) // Skip if all instances were excluded (count reduced to 0). if adjusted.Count <= 0 { - return adjusted, false + return adjusted, false, common.DropExtendedSupport } - return adjusted, true + return adjusted, true, "" } - return *rec, true + return *rec, true, "" } // passesDimensionFilters runs the stateless include/exclude checks on diff --git a/cmd/multi_service_filters_test.go b/cmd/multi_service_filters_test.go index 4e912dda9..ded57e93b 100644 --- a/cmd/multi_service_filters_test.go +++ b/cmd/multi_service_filters_test.go @@ -1,9 +1,8 @@ package main import ( - "bytes" - "log" "testing" + "time" "github.com/LeanerCloud/CUDly/pkg/common" "github.com/stretchr/testify/assert" @@ -113,7 +112,7 @@ func TestApplyFilters(t *testing.T) { toolCfg.ExcludeInstanceTypes = tt.excludeInstanceTypes // Apply filters with Config (empty currentRegion for test) - result := applyFilters(tt.recommendations, &toolCfg, make(map[string][]InstanceEngineVersion), make(map[string]MajorEngineVersionInfo), "") + result := applyFilters(tt.recommendations, &toolCfg, make(map[string][]InstanceEngineVersion), make(map[string]MajorEngineVersionInfo), "", nil) // Check count assert.Equal(t, tt.expectedCount, len(result)) @@ -439,38 +438,93 @@ func TestShouldIncludePoolSize(t *testing.T) { } } -// TestApplyFilters_PoolSizeLogs verifies that applyFilters emits per-rec and -// summary log lines when --min-pool-size drops recommendations. -func TestApplyFilters_PoolSizeLogs(t *testing.T) { +// TestApplyFilters_DropMinPoolSize verifies that a recommendation whose +// AverageInstancesUsedPerHour is below --min-pool-size is recorded in a +// non-nil DropSummary under the DropMinPoolSize category. If the +// drops.Add call for that path were removed, d.Total() would stay at 0 +// and the first assertion below would fail. +func TestApplyFilters_DropMinPoolSize(t *testing.T) { origCfg := toolCfg defer func() { toolCfg = origCfg }() - // Capture log output. - var buf bytes.Buffer - origFlags := log.Flags() - origWriter := log.Writer() - log.SetOutput(&buf) - log.SetFlags(0) // strip timestamps so the assertions are stable - defer func() { - log.SetOutput(origWriter) - log.SetFlags(origFlags) - }() + // Pool with avg=1.5 is below min-pool-size=2; it should be dropped. + rec := common.Recommendation{ + Region: "us-east-1", + ResourceType: "db.t3.micro", + Count: 2, + AverageInstancesUsedPerHour: 1.5, + } + toolCfg.MinPoolSize = 2.0 + + d := common.NewDropSummary() + result := applyFilters( + []common.Recommendation{rec}, + &toolCfg, + make(map[string][]InstanceEngineVersion), + make(map[string]MajorEngineVersionInfo), + "", + d, + ) + + assert.Empty(t, result, "below-min-pool-size rec should be filtered out") + assert.Equal(t, 1, d.Total(), "drop summary should record 1 drop") + assert.Contains(t, d.FormatOneLine(), common.DropMinPoolSize, + "drop summary should name the --min-pool-size category") +} + +// TestApplyFilters_DropExtendedSupport verifies that a recommendation whose +// entire instance count is on an engine version in extended support is +// recorded in a non-nil DropSummary under DropExtendedSupport when +// --include-extended-support is false. If the drops.Add call for that +// path were removed, d.Total() would stay at 0 and the assertion below +// would fail. +func TestApplyFilters_DropExtendedSupport(t *testing.T) { + origCfg := toolCfg + defer func() { toolCfg = origCfg }() - recs := []common.Recommendation{ - {Service: "rds", Region: "us-east-1", ResourceType: "db.t3.micro", AverageInstancesUsedPerHour: 0.8, Count: 1}, - {Service: "rds", Region: "us-east-1", ResourceType: "db.r5.large", AverageInstancesUsedPerHour: 3.5, Count: 2}, + toolCfg.IncludeExtendedSupport = false + + // One RDS MySQL 5.7.42 instance in us-east-1; 5.7 is in extended support. + rec := common.Recommendation{ + Region: "us-east-1", + ResourceType: "db.t3.micro", + Count: 1, + Service: common.ServiceRDS, + Details: &common.DatabaseDetails{Engine: "mysql"}, } - cfg := Config{MinPoolSize: 2.0} - result := applyFilters(recs, &cfg, nil, nil, "") + instanceVersions := map[string][]InstanceEngineVersion{ + "db.t3.micro": { + {Engine: "mysql", EngineVersion: "5.7.42", InstanceClass: "db.t3.micro", Region: "us-east-1"}, + }, + } - // Only the rec above threshold passes through. - assert.Len(t, result, 1) - assert.Equal(t, "db.r5.large", result[0].ResourceType) + // Mark mysql 5.7 as in extended support (start date well in the past). + versionInfo := map[string]MajorEngineVersionInfo{ + "mysql:5.7": { + Engine: "mysql", + MajorEngineVersion: "5.7", + SupportedEngineLifecycles: []EngineLifecycleInfo{ + { + LifecycleSupportName: "open-source-rds-extended-support", + LifecycleSupportStartDate: time.Now().Add(-365 * 24 * time.Hour), + }, + }, + }, + } - output := buf.String() - // Per-rec line present. - assert.Contains(t, output, "--min-pool-size=2.0 dropped rds/us-east-1/db.t3.micro (avg=0.80 < threshold)") - // Summary line present. - assert.Contains(t, output, "--min-pool-size dropped 1 recommendation(s)") + d := common.NewDropSummary() + result := applyFilters( + []common.Recommendation{rec}, + &toolCfg, + instanceVersions, + versionInfo, + "", + d, + ) + + assert.Empty(t, result, "all-extended-support rec should be filtered out") + assert.Equal(t, 1, d.Total(), "drop summary should record 1 drop") + assert.Contains(t, d.FormatOneLine(), common.DropExtendedSupport, + "drop summary should name the --include-extended-support category") } diff --git a/cmd/multi_service_helpers.go b/cmd/multi_service_helpers.go index dd9e75542..b88feccf9 100644 --- a/cmd/multi_service_helpers.go +++ b/cmd/multi_service_helpers.go @@ -374,8 +374,8 @@ func processRegionRecommendations( // Populate account names populateRecommendationAccountNames(ctx, recs, accountCache) - // Apply filters - recs = applyRegionFilters(recs, engineData, region, cfg) + // Apply filters (drops not tracked on this legacy test-only path). + recs = applyRegionFilters(recs, engineData, region, cfg, nil) if len(recs) == 0 { AppLogger.Printf(" ℹ️ No recommendations after applying filters\n") return result @@ -384,8 +384,8 @@ func processRegionRecommendations( // Apply coverage and overrides. This legacy path (test-only) doesn't // fetch expiring commitments — expiry-aware sizing only runs via the // main pipeline in fetchAndFilterRegionRecs, which has access to the - // regional service client at the right moment. - filteredRecs := applyCoverageAndOverrides(recs, cfg, coverageMap, nil) + // regional service client at the right moment. Drop tracking skipped (nil). + filteredRecs := applyCoverageAndOverrides(recs, cfg, coverageMap, nil, nil) result.recommendations = filteredRecs @@ -400,8 +400,8 @@ func processRegionRecommendations( return result } - // Check for duplicate RIs and apply instance limit - adjustedRecs := checkDuplicatesAndApplyLimit(ctx, filteredRecs, serviceClient, cfg) + // Check for duplicate RIs and apply instance limit. Drop tracking skipped (nil). + adjustedRecs := checkDuplicatesAndApplyLimit(ctx, filteredRecs, serviceClient, cfg, nil) // Process purchases regionResults := processPurchaseLoop(ctx, adjustedRecs, region, isDryRun, serviceClient, cfg) @@ -462,9 +462,10 @@ func applyRegionFilters( engineData engineVersionData, region string, cfg Config, + drops *common.DropSummary, ) []common.Recommendation { originalCount := len(recs) - recs = applyFilters(recs, &cfg, engineData.instanceVersions, engineData.versionInfo, region) + recs = applyFilters(recs, &cfg, engineData.instanceVersions, engineData.versionInfo, region, drops) if len(recs) < originalCount { AppLogger.Printf(" 🔍 After filters: %d recommendations (filtered out %d)\n", len(recs), originalCount-len(recs)) @@ -485,7 +486,10 @@ func applyRegionFilters( // ExistingCoveragePct further by the share of pool demand attributable to // RIs expiring within the window, so --target-coverage recommends // replacements before the cliff. Doesn't run if either input is empty. -func applyCoverageAndOverrides(recs []common.Recommendation, cfg Config, coverageMap recommendations.PoolCoverageMap, expiringCommitments []common.Commitment) []common.Recommendation { +// +// drops accumulates per-reason drop counts for the end-of-run summary. +// Pass nil to skip tracking. +func applyCoverageAndOverrides(recs []common.Recommendation, cfg Config, coverageMap recommendations.PoolCoverageMap, expiringCommitments []common.Commitment, drops *common.DropSummary) []common.Recommendation { recommendations.ApplyCoverageMapToRecommendations(recs, coverageMap) if cfg.RebuyWindowDays > 0 && len(expiringCommitments) > 0 { n := recommendations.AdjustExistingCoverageForExpiringCommitments(recs, expiringCommitments, cfg.RebuyWindowDays) @@ -502,9 +506,13 @@ func applyCoverageAndOverrides(recs []common.Recommendation, cfg Config, coverag var sizedRDS []common.Recommendation rest := recs if cfg.TargetCoverage > 0 { - sizedRDS, rest = recommendations.ApplyFamilyNUSizingRDS(recs, coverageMap, cfg.TargetCoverage) + var familyDrops recommendations.FamilyDropCounts + sizedRDS, rest, familyDrops = recommendations.ApplyFamilyNUSizingRDS(recs, coverageMap, cfg.TargetCoverage) + drops.Add(common.DropFamilyAlreadyAtTarget, familyDrops.AlreadyAtTarget) + drops.Add(common.DropFamilyNoNUSignal, familyDrops.NoNUSignal) + drops.Add(common.DropFamilySizedToZero, familyDrops.SizedToZero) } - filteredRecs := applySizing(rest, cfg, cfg.Coverage) + filteredRecs := applySizing(rest, cfg, cfg.Coverage, drops) filteredRecs = append(filteredRecs, sizedRDS...) if cfg.TargetCoverage > 0 { AppLogger.Printf(" 🎯 Applying %.1f%% target-coverage: %d recommendations selected (%d via family-NU, %d via per-pool)\n", @@ -522,15 +530,17 @@ func applyCoverageAndOverrides(recs []common.Recommendation, cfg Config, coverag } // checkDuplicatesAndApplyLimit checks for duplicate RIs and applies instance limits. +// drops accumulates per-reason drop counts for the end-of-run summary; pass nil to skip. func checkDuplicatesAndApplyLimit( ctx context.Context, filteredRecs []common.Recommendation, serviceClient provider.ServiceClient, cfg Config, + drops *common.DropSummary, ) []common.Recommendation { // Check for duplicate RIs to avoid double purchasing duplicateChecker := NewDuplicateChecker(0) - adjustedRecs, _, err := duplicateChecker.AdjustRecommendationsForExistingRIs(ctx, filteredRecs, serviceClient) + adjustedRecs, dedupedOut, err := duplicateChecker.AdjustRecommendationsForExistingRIs(ctx, filteredRecs, serviceClient) if err != nil { AppLogger.Printf(" ⚠️ Warning: Could not check for existing RIs: %v\n", err) // Continue with original filteredRecs on error; adjustedRecs is not used in this branch. @@ -541,6 +551,7 @@ func checkDuplicatesAndApplyLimit( if originalInstances != adjustedInstances { AppLogger.Printf(" 🔍 Adjusted recommendations: %d instances → %d instances to avoid duplicate purchases\n", originalInstances, adjustedInstances) } + drops.Add(common.DropDuplicateDedup, len(dedupedOut)) filteredRecs = adjustedRecs } @@ -559,7 +570,8 @@ func checkDuplicatesAndApplyLimit( // fetchAndFilterRegionRecs fetches, filters, applies coverage, and deduplicates // recommendations for a single service+region. No purchases are made. // coverageMap (when non-nil) feeds existing-pool coverage into the sizing -// step for --target-coverage. +// step for --target-coverage. drops accumulates per-reason drop counts for +// the end-of-run summary; pass nil to skip tracking. func fetchAndFilterRegionRecs( ctx context.Context, awsCfg aws.Config, @@ -571,6 +583,7 @@ func fetchAndFilterRegionRecs( engineData engineVersionData, cfg Config, coverageMap recommendations.PoolCoverageMap, + drops *common.DropSummary, ) []common.Recommendation { AppLogger.Printf("\n 📍 [%d/%d] Region: %s\n", regionIndex, totalRegions, region) @@ -582,7 +595,7 @@ func fetchAndFilterRegionRecs( AppLogger.Printf(" ✅ Found %d recommendations\n", len(recs)) populateRecommendationAccountNames(ctx, recs, accountCache) - recs = applyRegionFilters(recs, engineData, region, cfg) + recs = applyRegionFilters(recs, engineData, region, cfg, drops) if len(recs) == 0 { AppLogger.Printf(" ℹ️ No recommendations after applying filters\n") return nil @@ -608,11 +621,11 @@ func fetchAndFilterRegionRecs( } } - recs = applyCoverageAndOverrides(recs, cfg, coverageMap, expiringCommitments) + recs = applyCoverageAndOverrides(recs, cfg, coverageMap, expiringCommitments, drops) // Deduplication: skip recs matching recently-purchased commitments. if serviceClient != nil { - recs = checkDuplicatesAndApplyLimit(ctx, recs, serviceClient, cfg) + recs = checkDuplicatesAndApplyLimit(ctx, recs, serviceClient, cfg, drops) } return recs @@ -621,7 +634,8 @@ func fetchAndFilterRegionRecs( // fetchAllRecs collects recommendations from all services and regions without // purchasing. coverageMap (when non-nil) populates Recommendation.ExistingCoveragePct // on each rec before sizing, so --target-coverage can subtract what's already -// owned in the same pool. +// owned in the same pool. The returned DropSummary accumulates per-reason +// drop counts across every service and region for the end-of-run summary. func fetchAllRecs( ctx context.Context, awsCfg aws.Config, @@ -631,8 +645,9 @@ func fetchAllRecs( engineData engineVersionData, cfg Config, coverageMap recommendations.PoolCoverageMap, -) []common.Recommendation { +) ([]common.Recommendation, *common.DropSummary) { all := make([]common.Recommendation, 0) + drops := common.NewDropSummary() for _, service := range servicesToProcess { AppLogger.Printf("\n━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n") AppLogger.Printf("🔍 Fetching %s recommendations\n", getServiceDisplayName(service)) @@ -644,9 +659,9 @@ func fetchAllRecs( continue } for i, region := range regions { - recs := fetchAndFilterRegionRecs(ctx, awsCfg, recClient, accountCache, service, region, i+1, len(regions), engineData, cfg, coverageMap) + recs := fetchAndFilterRegionRecs(ctx, awsCfg, recClient, accountCache, service, region, i+1, len(regions), engineData, cfg, coverageMap, drops) all = append(all, recs...) } } - return all + return all, drops } diff --git a/cmd/multi_service_test.go b/cmd/multi_service_test.go index 38b57358e..60046f682 100644 --- a/cmd/multi_service_test.go +++ b/cmd/multi_service_test.go @@ -1,16 +1,20 @@ package main import ( + "bytes" "context" "encoding/csv" "fmt" + "log" "os" "path/filepath" "strconv" + "strings" "testing" "time" "github.com/LeanerCloud/CUDly/pkg/common" + "github.com/LeanerCloud/CUDly/pkg/scorer" "github.com/aws/aws-sdk-go-v2/aws" rdstypes "github.com/aws/aws-sdk-go-v2/service/rds/types" "github.com/stretchr/testify/assert" @@ -558,6 +562,44 @@ func TestApplyCoverageToRecommendations(t *testing.T) { } } +func TestPrintDropSummaryImmediatelyPrecedesTerminalOutput(t *testing.T) { + for _, terminal := range []string{ + "ℹ️ No recommendations passed filters. Nothing to purchase.", + "FINAL SUMMARY", + } { + t.Run(terminal, func(t *testing.T) { + oldLogger := AppLogger + var output bytes.Buffer + AppLogger = log.New(&output, "", 0) + t.Cleanup(func() { AppLogger = oldLogger }) + + drops := common.NewDropSummary() + drops.Add(common.DropTargetSizedToZero, 1) + + printDropSummary(drops) + AppLogger.Printf("\n%s\n", terminal) + + assert.Contains(t, output.String(), + "Dropped 1 recs: target-sized-to-zero=1\n\n"+terminal) + assert.Equal(t, 1, strings.Count(output.String(), "Dropped 1 recs")) + }) + } +} + +func TestRunPurchaseAndReportDeclinedConfirmationPrintsDropsBeforeCancellation(t *testing.T) { + drops := common.NewDropSummary() + drops.Add(common.DropTargetSizedToZero, 1) + scored := scorer.ScoredResult{Passed: []common.Recommendation{{Count: 1, EstimatedSavings: 10}}} + + output := captureAppOutput(t, func() { + runPurchaseAndReport(context.Background(), aws.Config{}, scored, false, Config{}, drops) + }) + + assert.Contains(t, output, + "Dropped 1 recs: target-sized-to-zero=1\n\n❌ Purchase canceled.") + assert.Equal(t, 1, strings.Count(output, "Dropped 1 recs")) +} + func TestServiceProcessingOrder(t *testing.T) { // Test that services are processed in a consistent order services := []common.ServiceType{ @@ -839,7 +881,7 @@ func TestApplyFilters_RegionFiltering(t *testing.T) { ExcludeRegions: tt.excludeRegions, } - result := applyFilters(tt.recs, &cfg, map[string][]InstanceEngineVersion{}, map[string]MajorEngineVersionInfo{}, tt.currentRegion) + result := applyFilters(tt.recs, &cfg, map[string][]InstanceEngineVersion{}, map[string]MajorEngineVersionInfo{}, tt.currentRegion, nil) assert.Equal(t, tt.expectedCount, len(result), "Expected %d recommendations, got %d", tt.expectedCount, len(result)) }) } @@ -884,7 +926,7 @@ func TestApplyFilters_InstanceTypeFiltering(t *testing.T) { ExcludeInstanceTypes: tt.excludeInstanceTypes, } - result := applyFilters(tt.recs, &cfg, map[string][]InstanceEngineVersion{}, map[string]MajorEngineVersionInfo{}, "") + result := applyFilters(tt.recs, &cfg, map[string][]InstanceEngineVersion{}, map[string]MajorEngineVersionInfo{}, "", nil) assert.Equal(t, tt.expectedCount, len(result)) }) } @@ -959,7 +1001,7 @@ func TestApplyFilters_EngineFiltering(t *testing.T) { ExcludeEngines: tt.excludeEngines, } - result := applyFilters(tt.recs, &cfg, map[string][]InstanceEngineVersion{}, map[string]MajorEngineVersionInfo{}, "") + result := applyFilters(tt.recs, &cfg, map[string][]InstanceEngineVersion{}, map[string]MajorEngineVersionInfo{}, "", nil) assert.Equal(t, tt.expectedCount, len(result)) }) } @@ -1023,7 +1065,7 @@ func TestApplyFilters_AccountFiltering(t *testing.T) { ExcludeAccounts: tt.excludeAccounts, } - result := applyFilters(tt.recs, &cfg, map[string][]InstanceEngineVersion{}, map[string]MajorEngineVersionInfo{}, "") + result := applyFilters(tt.recs, &cfg, map[string][]InstanceEngineVersion{}, map[string]MajorEngineVersionInfo{}, "", nil) assert.Equal(t, tt.expectedCount, len(result)) }) } @@ -1044,7 +1086,7 @@ func TestApplyFilters_CombinedFilters(t *testing.T) { IncludeAccounts: []string{"prod", "dev"}, } - result := applyFilters(recs, &cfg, map[string][]InstanceEngineVersion{}, map[string]MajorEngineVersionInfo{}, "") + result := applyFilters(recs, &cfg, map[string][]InstanceEngineVersion{}, map[string]MajorEngineVersionInfo{}, "", nil) // Only the first two should pass all filters assert.Equal(t, 2, len(result)) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index adbfcc9e4..2064f1da4 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -8,38 +8,38 @@ "name": "cudly-frontend", "version": "1.0.0", "dependencies": { - "@types/qrcode": "^1.5.6", - "chart.js": "^4.4.0", - "qrcode": "^1.5.4" + "@types/qrcode": "1.5.6", + "chart.js": "4.5.1", + "qrcode": "1.5.4" }, "devDependencies": { - "@babel/core": "^7.23.0", - "@babel/preset-env": "^7.23.0", - "@babel/preset-typescript": "^7.23.0", - "@testing-library/dom": "^9.3.0", - "@testing-library/jest-dom": "^6.1.0", - "@types/chart.js": "^2.9.41", - "@types/jest": "^29.5.0", - "@types/jsdom": "^21.1.0", - "@typescript-eslint/eslint-plugin": "^8.0.0", - "@typescript-eslint/parser": "^8.0.0", - "@ungap/structured-clone": "^1.3.1", - "babel-loader": "^9.1.0", - "copy-webpack-plugin": "^14.0.0", - "css-loader": "^6.8.0", - "css-minimizer-webpack-plugin": "^8.0.0", - "eslint": "^8.50.0", - "html-webpack-plugin": "^5.5.0", - "jest": "^29.7.0", - "jest-environment-jsdom": "^29.7.0", - "jsdom": "^22.1.0", - "mini-css-extract-plugin": "^2.7.0", - "style-loader": "^3.3.0", - "ts-jest": "^29.1.0", - "ts-loader": "^9.5.0", - "typescript": "^5.3.0", - "webpack": "^5.88.0", - "webpack-cli": "^5.1.0" + "@babel/core": "7.29.7", + "@babel/preset-env": "7.28.5", + "@babel/preset-typescript": "7.28.5", + "@testing-library/dom": "9.3.4", + "@testing-library/jest-dom": "6.9.1", + "@types/chart.js": "2.9.41", + "@types/jest": "29.5.14", + "@types/jsdom": "21.1.7", + "@typescript-eslint/eslint-plugin": "8.62.1", + "@typescript-eslint/parser": "8.62.1", + "@ungap/structured-clone": "1.3.1", + "babel-loader": "9.2.1", + "copy-webpack-plugin": "14.0.0", + "css-loader": "6.11.0", + "css-minimizer-webpack-plugin": "8.0.0", + "eslint": "8.57.1", + "html-webpack-plugin": "5.6.5", + "jest": "29.7.0", + "jest-environment-jsdom": "29.7.0", + "jsdom": "22.1.0", + "mini-css-extract-plugin": "2.9.4", + "style-loader": "3.3.4", + "ts-jest": "29.4.6", + "ts-loader": "9.5.4", + "typescript": "5.9.3", + "webpack": "5.108.3", + "webpack-cli": "5.1.4" } }, "node_modules/@adobe/css-tools": { @@ -3837,9 +3837,9 @@ "license": "ISC" }, "node_modules/brace-expansion": { - "version": "1.1.15", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.15.tgz", - "integrity": "sha512-EwOCDEex4quD37XhqM3omwtMoJjr//isUZz1JopUNWms+4Z2ViyM/k1YIRePpoVNnQhENnxtFjLaxNHrT7xIUg==", + "version": "1.1.16", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", + "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", "dev": true, "license": "MIT", "dependencies": { diff --git a/pkg/common/drop_summary.go b/pkg/common/drop_summary.go new file mode 100644 index 000000000..165c5c1eb --- /dev/null +++ b/pkg/common/drop_summary.go @@ -0,0 +1,88 @@ +package common + +import ( + "fmt" + "sort" + "strings" +) + +// Drop reason keys used by all filter and sizing stages. Using typed constants +// avoids typos at call sites and lets tests assert on the exact key. +const ( + DropMinPoolSize = "--min-pool-size" + DropExtendedSupport = "--include-extended-support" + DropTargetAlreadyMet = "target-already-met" + DropTargetSizedToZero = "target-sized-to-zero" + DropFamilyAlreadyAtTarget = "family-nu-already-at-target" + DropFamilyNoNUSignal = "family-nu-no-nu-signal" + DropFamilySizedToZero = "family-nu-sized-to-zero" + DropDuplicateDedup = "duplicate-dedup" +) + +// DropSummary accumulates the count of recommendations dropped per reason +// across the full fetch-filter-size pipeline. It is not safe for concurrent +// use from multiple goroutines; the main pipeline is sequential per service +// and region so no synchronization is needed. +type DropSummary struct { + counts map[string]int +} + +// NewDropSummary returns a zero-valued DropSummary ready to record drops. +func NewDropSummary() *DropSummary { + return &DropSummary{counts: make(map[string]int)} +} + +// Add increments the drop count for the given reason by n. Safe to call on a +// zero-value DropSummary (e.g. var d DropSummary); the underlying map is +// lazily initialized on first use. +func (d *DropSummary) Add(reason string, n int) { + if d == nil || n == 0 { + return + } + if d.counts == nil { + d.counts = make(map[string]int) + } + d.counts[reason] += n +} + +// Total returns the sum of all drops recorded. +func (d *DropSummary) Total() int { + if d == nil { + return 0 + } + total := 0 + for _, n := range d.counts { + total += n + } + return total +} + +// IsEmpty reports whether no drops have been recorded. +func (d *DropSummary) IsEmpty() bool { + return d == nil || len(d.counts) == 0 +} + +// FormatOneLine returns a compact single-line summary such as: +// +// Dropped 14 recs: --min-pool-size=8, target-already-met=4, duplicate-dedup=2 +// +// Returns an empty string when no drops have been recorded. +func (d *DropSummary) FormatOneLine() string { + if d.IsEmpty() { + return "" + } + + // Sort reasons for deterministic output. + reasons := make([]string, 0, len(d.counts)) + for r := range d.counts { + reasons = append(reasons, r) + } + sort.Strings(reasons) + + parts := make([]string, 0, len(reasons)) + for _, r := range reasons { + parts = append(parts, fmt.Sprintf("%s=%d", r, d.counts[r])) + } + + return fmt.Sprintf("Dropped %d recs: %s", d.Total(), strings.Join(parts, ", ")) +} diff --git a/pkg/common/drop_summary_test.go b/pkg/common/drop_summary_test.go new file mode 100644 index 000000000..8aec84e3e --- /dev/null +++ b/pkg/common/drop_summary_test.go @@ -0,0 +1,100 @@ +package common + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDropSummary_Add_And_Total(t *testing.T) { + d := NewDropSummary() + assert.Equal(t, 0, d.Total()) + assert.True(t, d.IsEmpty()) + + d.Add(DropMinPoolSize, 8) + d.Add(DropTargetAlreadyMet, 4) + d.Add(DropDuplicateDedup, 2) + + assert.Equal(t, 14, d.Total()) + assert.False(t, d.IsEmpty()) +} + +func TestDropSummary_Add_Zero_NoOp(t *testing.T) { + d := NewDropSummary() + d.Add(DropMinPoolSize, 0) + assert.True(t, d.IsEmpty()) +} + +func TestDropSummary_NilReceiver_Safe(t *testing.T) { + var d *DropSummary + d.Add(DropMinPoolSize, 3) // must not panic + assert.Equal(t, 0, d.Total()) + assert.True(t, d.IsEmpty()) + assert.Equal(t, "", d.FormatOneLine()) +} + +// Regression: a zero-value DropSummary (declared without NewDropSummary) +// must not panic on Add because the internal map is lazily initialized. +func TestDropSummary_ZeroValue_Safe(t *testing.T) { + var d DropSummary + d.Add(DropMinPoolSize, 3) // must not panic on nil map + d.Add(DropTargetAlreadyMet, 2) + assert.Equal(t, 5, d.Total()) + assert.False(t, d.IsEmpty()) + assert.Equal(t, "Dropped 5 recs: --min-pool-size=3, target-already-met=2", d.FormatOneLine()) +} + +func TestDropSummary_FormatOneLine(t *testing.T) { + tests := []struct { + name string + setup func(*DropSummary) + expected string + }{ + { + name: "empty returns blank string", + setup: func(_ *DropSummary) {}, + expected: "", + }, + { + name: "single reason", + setup: func(d *DropSummary) { + d.Add(DropMinPoolSize, 3) + }, + expected: "Dropped 3 recs: --min-pool-size=3", + }, + { + name: "multiple reasons sorted deterministically", + setup: func(d *DropSummary) { + d.Add(DropMinPoolSize, 8) + d.Add(DropTargetAlreadyMet, 4) + d.Add(DropDuplicateDedup, 2) + }, + // Alphabetically: --min-pool-size, duplicate-dedup, target-already-met + expected: "Dropped 14 recs: --min-pool-size=8, duplicate-dedup=2, target-already-met=4", + }, + { + name: "all drop categories present", + setup: func(d *DropSummary) { + d.Add(DropMinPoolSize, 1) + d.Add(DropExtendedSupport, 2) + d.Add(DropTargetAlreadyMet, 3) + d.Add(DropTargetSizedToZero, 4) + d.Add(DropFamilyAlreadyAtTarget, 5) + d.Add(DropFamilyNoNUSignal, 6) + d.Add(DropFamilySizedToZero, 7) + d.Add(DropDuplicateDedup, 8) + }, + expected: "Dropped 36 recs: --include-extended-support=2, --min-pool-size=1, " + + "duplicate-dedup=8, family-nu-already-at-target=5, family-nu-no-nu-signal=6, " + + "family-nu-sized-to-zero=7, target-already-met=3, target-sized-to-zero=4", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := NewDropSummary() + tt.setup(d) + assert.Equal(t, tt.expected, d.FormatOneLine()) + }) + } +} diff --git a/providers/aws/recommendations/family_nu.go b/providers/aws/recommendations/family_nu.go index 475945f86..64270dd43 100644 --- a/providers/aws/recommendations/family_nu.go +++ b/providers/aws/recommendations/family_nu.go @@ -14,9 +14,12 @@ import ( // // Used by ApplyFamilyNUSizingRDS to translate AWS-rec-API's family-NU- // bundled buy recommendations into the family target NU need under -// --target-coverage. Sizes not in this map evaluate to 0 NU, which -// causes the family-NU step to leave the rec unchanged (the per-pool -// sizing path still handles it downstream). +// --target-coverage. Sizes not in this map evaluate to 0 NU. Recs +// with an empty family prefix (unknown size suffix) are routed to +// nonRDS by partitionRDSRecsByFamily and handled by the per-pool path. +// Recs with a known family prefix but an unrecognized size suffix reach +// sizeRDSFamilyRecs with 0 NU; if the whole family sums to zero +// (currentNU <= 0) they are recorded in drops.NoNUSignal and dropped. var rdsInstanceNU = map[string]float64{ "nano": 0.25, "micro": 0.5, @@ -53,8 +56,9 @@ func RDSFamilyFromType(instanceType string) string { // rdsInstanceNUFromType returns the NU value for an instance type like // "db.r7g.2xlarge", parsing out the size suffix ("2xlarge" → 16). Returns -// 0 when the size isn't recognised — callers treat that as "no family-NU -// signal" and fall back to per-pool sizing. +// 0 when the size isn't recognized. When a whole family's rec-NU sums to +// zero inside sizeRDSFamilyRecs (currentNU <= 0), those recs are recorded +// in drops.NoNUSignal and dropped rather than falling back to per-pool sizing. func rdsInstanceNUFromType(instanceType string) float64 { parts := strings.Split(instanceType, ".") if len(parts) < 3 { @@ -131,6 +135,23 @@ func AggregateRDSFamilyCoverage(coverage PoolCoverageMap) map[string]FamilyCover return out } +// FamilyDropCounts holds the counts of family-level drops from +// ApplyFamilyNUSizingRDS, split by the two distinct drop reasons so the +// caller can record them into a DropSummary without importing cmd. +type FamilyDropCounts struct { + // AlreadyAtTarget is the number of recs from families where the + // existing coverage already meets or exceeds the target (gap <= 0). + AlreadyAtTarget int + // NoNUSignal is the number of recs dropped because the family's + // AWS-recommended counts summed to zero NU (e.g. all recs at + // unknown/unrecognized sizes), so there is no scalable NU to apply + // the family target against. This is distinct from AlreadyAtTarget. + NoNUSignal int + // SizedToZero is the number of recs dropped because the family-wide + // scale factor produced a floor(0) count for that rec's size. + SizedToZero int +} + // ApplyFamilyNUSizingRDS replaces per-pool RI sizing with family-NU // sizing for RDS recommendations. AWS's GetReservationPurchaseRecommendation // already bundles size-flex demand within an instance family into a @@ -152,25 +173,31 @@ func AggregateRDSFamilyCoverage(coverage PoolCoverageMap) map[string]FamilyCover // 4. Non-RDS recs are returned unchanged so callers can continue them // through the per-pool sizing path. // -// Returns (sizedRDS, nonRDS). When targetPct is outside (0,100] or -// coverage has no family-NU signal for an RDS rec's family, the rec is -// passed through unchanged in sizedRDS (so per-pool sizing downstream -// doesn't re-process it — caller treats sizedRDS as already sized). +// Returns (sizedRDS, nonRDS, drops). When targetPct is outside (0,100] +// all recs are returned as nonRDS unchanged. When the CE coverage for +// a family has no NU signal (family.TotalNU <= 0), that family's recs +// are returned sized (as-is) in sizedRDS. When the family's +// AWS-recommended counts sum to zero NU (currentNU <= 0 — all recs at +// unrecognized sizes), those recs are dropped and recorded in +// drops.NoNUSignal; they are NOT passed through to the per-pool path. func ApplyFamilyNUSizingRDS( recs []common.Recommendation, coverage PoolCoverageMap, targetPct float64, -) (sizedRDS, nonRDS []common.Recommendation) { +) (sizedRDS, nonRDS []common.Recommendation, drops FamilyDropCounts) { if targetPct <= 0 || targetPct > 100 { - return nil, recs + return nil, recs, drops } familyCov := AggregateRDSFamilyCoverage(coverage) familyIdx, nonRDS := partitionRDSRecsByFamily(recs) for fk, indices := range familyIdx { - sized := sizeRDSFamilyRecs(recs, indices, familyCov[fk], targetPct) + sized, familyDrops := sizeRDSFamilyRecs(recs, indices, familyCov[fk], targetPct) sizedRDS = append(sizedRDS, sized...) + drops.AlreadyAtTarget += familyDrops.AlreadyAtTarget + drops.NoNUSignal += familyDrops.NoNUSignal + drops.SizedToZero += familyDrops.SizedToZero } - return sizedRDS, nonRDS + return sizedRDS, nonRDS, drops } // partitionRDSRecsByFamily splits recs into (a) an index map keyed by @@ -200,9 +227,17 @@ func partitionRDSRecsByFamily(recs []common.Recommendation) (map[string][]int, [ return familyIdx, nonRDS } -// sizeRDSFamilyRecs sizes the recs in one family-key group: returns the -// sized recs, drops empty/over-target families, and returns the -// unchanged AWS-recommended counts when there's no coverage signal. +// sizeRDSFamilyRecs sizes the recs in one family-key group. It returns +// the sized recs and a drop-count struct. Three distinct outcomes exist: +// - family.TotalNU <= 0: CE coverage has no NU signal for this family; +// recs are returned as-is (AWS-recommended counts unchanged). +// - gap <= 0: family already at or above target; all recs are dropped +// and recorded in drops.AlreadyAtTarget. +// - currentNU <= 0: rec counts sum to zero NU (all at unrecognized +// sizes); recs are dropped and recorded in drops.NoNUSignal — they +// are NOT passed through to the per-pool sizing path. +// +// The second return value reports how many recs were dropped and why. // // First pass scales each rec's Count and cost-bearing fields by the // family-wide scale factor; second pass sets the same family-level @@ -216,20 +251,22 @@ func sizeRDSFamilyRecs( indices []int, family FamilyCoverage, targetPct float64, -) []common.Recommendation { +) ([]common.Recommendation, FamilyDropCounts) { + var drops FamilyDropCounts if family.TotalNU <= 0 { // No coverage signal — keep AWS-recommended counts as-is. out := make([]common.Recommendation, 0, len(indices)) for _, i := range indices { out = append(out, recs[i]) } - return out + return out, drops } existingPct := family.CoveredNU / family.TotalNU * 100.0 gap := targetPct - existingPct if gap <= 0 { // Family already at-or-above target — drop the whole family's recs. - return nil + drops.AlreadyAtTarget = len(indices) + return nil, drops } targetNU := gap / 100.0 * family.TotalNU currentNU := 0.0 @@ -237,31 +274,37 @@ func sizeRDSFamilyRecs( currentNU += float64(recs[i].Count) * rdsInstanceNUFromType(recs[i].ResourceType) } if currentNU <= 0 { - // Recs sum to zero NU — nothing to scale. - return nil + // Recs sum to zero NU — family lookup returned no scalable signal. + // This is not the same as "already at target"; record separately. + drops.NoNUSignal = len(indices) + return nil, drops } scale := targetNU / currentNU - sized, totalNewNU := scaleFamilyRecs(recs, indices, scale) + sized, totalNewNU, zeroDrops := scaleFamilyRecs(recs, indices, scale) + drops.SizedToZero = zeroDrops annotateFamilyProjection(sized, existingPct, totalNewNU, family.TotalNU) - return sized + return sized, drops } // scaleFamilyRecs is the first pass of sizeRDSFamilyRecs: applies the // family-wide scale factor to each rec's count and cost-bearing fields, -// returning the surviving recs (newCount > 0) and the cumulative -// post-scaling NU across them. -func scaleFamilyRecs(recs []common.Recommendation, indices []int, scale float64) ([]common.Recommendation, float64) { +// returning the surviving recs (newCount > 0), the cumulative +// post-scaling NU across them, and the number of recs dropped because +// floor(count * scale) == 0. +func scaleFamilyRecs(recs []common.Recommendation, indices []int, scale float64) ([]common.Recommendation, float64, int) { sized := make([]common.Recommendation, 0, len(indices)) totalNewNU := 0.0 + zeroDrops := 0 for _, i := range indices { rec, kept := scaleRDSRecInFamily(recs[i], scale) if !kept { + zeroDrops++ continue } sized = append(sized, rec) totalNewNU += float64(rec.Count) * rdsInstanceNUFromType(rec.ResourceType) } - return sized, totalNewNU + return sized, totalNewNU, zeroDrops } // annotateFamilyProjection is the second pass: computes the cumulative diff --git a/providers/aws/recommendations/family_nu_test.go b/providers/aws/recommendations/family_nu_test.go index ce4000db9..576077a21 100644 --- a/providers/aws/recommendations/family_nu_test.go +++ b/providers/aws/recommendations/family_nu_test.go @@ -124,12 +124,13 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { Details: &common.DatabaseDetails{Engine: "aurora-mysql", AZConfig: "single-az"}, }, } - sized, nonRDS := ApplyFamilyNUSizingRDS(recs, cov, 80) + sized, nonRDS, drops := ApplyFamilyNUSizingRDS(recs, cov, 80) require.Len(t, sized, 1, "RDS rec kept (target NU > 0)") assert.Empty(t, nonRDS, "no non-RDS recs in this fixture") assert.Equal(t, 15, sized[0].Count, "AWS-rec NU already matches target → count preserved") // Costs unchanged (ratio = 1) assert.InDelta(t, 1500.0, sized[0].CommitmentCost, 0.01) + assert.Equal(t, 0, drops.AlreadyAtTarget+drops.NoNUSignal+drops.SizedToZero, "no drops expected") }) t.Run("RecurringMonthlyCost scales with count when populated", func(t *testing.T) { @@ -155,13 +156,14 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { Details: &common.DatabaseDetails{Engine: "mysql", AZConfig: "multi-az"}, }, } - sized, _ := ApplyFamilyNUSizingRDS(recs, cov, 80) + sized, _, drops := ApplyFamilyNUSizingRDS(recs, cov, 80) require.Len(t, sized, 1) // Scale 32/20 = 1.6 → newCount = 8 → monthly = 100 × 8/5 = 160 assert.Equal(t, 8, sized[0].Count) require.NotNil(t, sized[0].RecurringMonthlyCost) assert.InDelta(t, 160.0, *sized[0].RecurringMonthlyCost, 0.001, "monthly fee scales by 8/5 alongside other costs") assert.Equal(t, 100.0, monthly, "original target should not be mutated (new pointer)") + assert.Equal(t, 0, drops.AlreadyAtTarget+drops.NoNUSignal+drops.SizedToZero, "no drops expected") }) t.Run("AWS rec under-recommends → counts scale up", func(t *testing.T) { @@ -186,10 +188,11 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { Details: &common.DatabaseDetails{Engine: "mysql", AZConfig: "multi-az"}, }, } - sized, _ := ApplyFamilyNUSizingRDS(recs, cov, 80) + sized, _, drops := ApplyFamilyNUSizingRDS(recs, cov, 80) require.Len(t, sized, 1) assert.Equal(t, 8, sized[0].Count, "scale 32/20 = 1.6 × 5 = 8 RIs to deliver 32 NU at 80% target") assert.InDelta(t, 5000*8.0/5.0, sized[0].CommitmentCost, 0.01, "CommitmentCost scales by 8/5") + assert.Equal(t, 0, drops.AlreadyAtTarget+drops.NoNUSignal+drops.SizedToZero, "no drops expected") }) t.Run("AWS rec over-recommends → counts scale down", func(t *testing.T) { @@ -212,9 +215,10 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { Details: &common.DatabaseDetails{Engine: "mysql", AZConfig: "multi-az"}, }, } - sized, _ := ApplyFamilyNUSizingRDS(recs, cov, 80) + sized, _, drops := ApplyFamilyNUSizingRDS(recs, cov, 80) require.Len(t, sized, 1) assert.Equal(t, 8, sized[0].Count, "AWS over-proposed; scale down to family target") + assert.Equal(t, 0, drops.AlreadyAtTarget+drops.NoNUSignal+drops.SizedToZero, "no drops expected") }) t.Run("family at-or-above target → all recs dropped", func(t *testing.T) { @@ -234,8 +238,38 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { Details: &common.DatabaseDetails{Engine: "aurora-mysql", AZConfig: "single-az"}, }, } - sized, _ := ApplyFamilyNUSizingRDS(recs, cov, 80) + sized, _, drops := ApplyFamilyNUSizingRDS(recs, cov, 80) assert.Empty(t, sized, "family already covered → drop all recs") + assert.Equal(t, 1, drops.AlreadyAtTarget, "one rec dropped as family-at-target") + assert.Equal(t, 0, drops.NoNUSignal+drops.SizedToZero, "no other drop categories") + }) + + t.Run("scale produces floor(0) for rec → SizedToZero drop", func(t *testing.T) { + // Family db.r7g Aurora MySQL us-east-1; cov=0, target=80 → target_NU=32. + // AWS rec: 1 × db.r7g.xlarge = 8 NU → scale = 32/8 = 4 — wait, that + // would scale up. Use a scenario where scale < 1 and count = 1: + // TotalNU = 10*4 = 40, existing=79% → gap=1 → targetNU=0.4. + // AWS rec: 1 × db.r7g.large = 4 NU → scale = 0.4/4 = 0.1 → floor(0.1) = 0. + cov := PoolCoverageMap{ + rdsPoolKey("us-east-1", "db.r7g.large", "Aurora MySQL", "Single-AZ"): { + Pct: 79.0, AvgInstancesPerHour: 10, + }, + } + recs := []common.Recommendation{ + { + Service: common.ServiceRDS, + CommitmentType: common.CommitmentReservedInstance, + Region: "us-east-1", + ResourceType: "db.r7g.large", + Count: 1, + CommitmentCost: 1000, + Details: &common.DatabaseDetails{Engine: "aurora-mysql", AZConfig: "single-az"}, + }, + } + sized, _, drops := ApplyFamilyNUSizingRDS(recs, cov, 80) + assert.Empty(t, sized, "rec scaled to zero → dropped") + assert.Equal(t, 1, drops.SizedToZero, "rec dropped because floor(scale*count)==0") + assert.Equal(t, 0, drops.AlreadyAtTarget+drops.NoNUSignal, "no other drop categories") }) t.Run("no coverage signal → recs pass through unchanged", func(t *testing.T) { @@ -251,9 +285,10 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { Details: &common.DatabaseDetails{Engine: "aurora-mysql", AZConfig: "single-az"}, }, } - sized, _ := ApplyFamilyNUSizingRDS(recs, PoolCoverageMap{}, 80) + sized, _, drops := ApplyFamilyNUSizingRDS(recs, PoolCoverageMap{}, 80) require.Len(t, sized, 1, "rec preserved as-is when no family-NU signal") assert.Equal(t, 5, sized[0].Count) + assert.Equal(t, 0, drops.AlreadyAtTarget+drops.NoNUSignal+drops.SizedToZero, "no drops when coverage map is empty") }) t.Run("non-RDS recs flow through nonRDS partition", func(t *testing.T) { @@ -276,11 +311,12 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { Pct: 0.0, AvgInstancesPerHour: 10, }, } - sized, nonRDS := ApplyFamilyNUSizingRDS(recs, cov, 80) + sized, nonRDS, drops := ApplyFamilyNUSizingRDS(recs, cov, 80) require.Len(t, sized, 1, "only the RDS rec went through family-NU") require.Len(t, nonRDS, 2, "EC2 + SP recs left for per-pool sizing") assert.Equal(t, common.ServiceEC2, nonRDS[0].Service) assert.Equal(t, common.ServiceSavingsPlansAll, nonRDS[1].Service) + assert.Equal(t, 0, drops.AlreadyAtTarget+drops.NoNUSignal+drops.SizedToZero, "no drops expected") }) t.Run("ProjectedCoverage is cumulative across recs in a family", func(t *testing.T) { @@ -322,7 +358,7 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { Details: &common.DatabaseDetails{Engine: "aurora-postgresql", AZConfig: "single-az"}, }, } - sized, _ := ApplyFamilyNUSizingRDS(recs, cov, 80) + sized, _, drops := ApplyFamilyNUSizingRDS(recs, cov, 80) require.Len(t, sized, 2, "both recs kept after scaling") // Both recs see the SAME cumulative projection. assert.InDelta(t, sized[0].ProjectedCoverage, sized[1].ProjectedCoverage, 0.001, @@ -334,6 +370,7 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { // Both rec.Count values reflect the scale-down. assert.Equal(t, 4, sized[0].Count, "prod scaled 5→4") assert.Equal(t, 2, sized[1].Count, "staging scaled 3→2") + assert.Equal(t, 0, drops.AlreadyAtTarget+drops.NoNUSignal+drops.SizedToZero, "no drops expected") }) t.Run("multiple sizes in same family scale together", func(t *testing.T) { @@ -360,9 +397,10 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { Details: &common.DatabaseDetails{Engine: "aurora-mysql", AZConfig: "single-az"}, }, } - sized, _ := ApplyFamilyNUSizingRDS(recs, cov, 80) + sized, _, drops := ApplyFamilyNUSizingRDS(recs, cov, 80) require.Len(t, sized, 1) assert.Equal(t, 12, sized[0].Count, "family-NU need includes .xlarge demand even though AWS rec is at .large") + assert.Equal(t, 0, drops.AlreadyAtTarget+drops.NoNUSignal+drops.SizedToZero, "no drops expected") }) }