diff --git a/cmd/helpers.go b/cmd/helpers.go index 6a3a1af9..6ae9275d 100644 --- a/cmd/helpers.go +++ b/cmd/helpers.go @@ -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. @@ -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) } } @@ -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 @@ -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 @@ -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 @@ -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 @@ -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) } diff --git a/cmd/helpers_test.go b/cmd/helpers_test.go index afbc0084..d3792f06 100644 --- a/cmd/helpers_test.go +++ b/cmd/helpers_test.go @@ -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)) @@ -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") @@ -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") @@ -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") }) @@ -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 @@ -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") @@ -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) @@ -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) @@ -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. @@ -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. @@ -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 @@ -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") @@ -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") diff --git a/cmd/multi_service.go b/cmd/multi_service.go index 35f5c4e7..4a54c045 100644 --- a/cmd/multi_service.go +++ b/cmd/multi_service.go @@ -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) @@ -141,14 +145,19 @@ func runToolMultiService(ctx context.Context, cfg Config) { allResults := executePurchasePipeline(ctx, awsCfg, scoredResult.Passed, isDryRun, runID, cfg) // Produce summary outputs. - serviceStats := buildServiceStats(scoredResult.Passed, allResults) + 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. @@ -389,9 +398,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)) } @@ -402,7 +412,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 { diff --git a/cmd/multi_service_filters.go b/cmd/multi_service_filters.go index da3ffdad..07ec47a5 100644 --- a/cmd/multi_service_filters.go +++ b/cmd/multi_service_filters.go @@ -9,13 +9,15 @@ 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 { +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 for _, rec := range recs { - adjusted, include := processRecommendation(rec, cfg, instanceVersions, versionInfo, currentRegion) + adjusted, include, dropReason := processRecommendation(rec, cfg, instanceVersions, versionInfo, currentRegion) if include { filtered = append(filtered, adjusted) + } else if dropReason != "" { + drops.Add(dropReason, 1) } } @@ -23,19 +25,28 @@ func applyFilters(recs []common.Recommendation, cfg Config, instanceVersions map } // 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) { - // 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 +// (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) (common.Recommendation, bool, string) { + // Filter to only recommendations for the current region being processed. + // This prevents duplicating recommendations across all regions. + // 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. + // Only --min-pool-size is a sizing heuristic that operators may need to + // investigate (hence reported separately in passesDimensionFiltersWithReason). + _, reason := passesDimensionFiltersWithReason(rec, cfg) + return rec, false, reason } // Apply engine version filters - adjust instance count by subtracting extended support versions @@ -43,11 +54,11 @@ func processRecommendation(rec common.Recommendation, cfg Config, instanceVersio rec = adjustRecommendationForExcludedVersions(rec, instanceVersions, versionInfo) // Skip if all instances were excluded (count reduced to 0) if rec.Count <= 0 { - return rec, false + return rec, false, common.DropExtendedSupport } } - return rec, true + return rec, true, "" } // passesDimensionFilters runs the stateless include/exclude checks on @@ -57,22 +68,33 @@ func processRecommendation(rec common.Recommendation, cfg Config, instanceVersio // dimension filters here are pure functions of rec + cfg with no side // effects. func passesDimensionFilters(rec common.Recommendation, cfg Config) bool { + ok, _ := passesDimensionFiltersWithReason(rec, cfg) + return ok +} + +// passesDimensionFiltersWithReason is the reporting variant of +// passesDimensionFilters. It returns (false, dropReason) when the rec is +// excluded, where dropReason is non-empty only for drops that operators +// should see in the end-of-run drop summary (currently only +// --min-pool-size). Region, account, engine, and instance-type mismatches +// are expected operator-scoping choices and return an empty reason. +func passesDimensionFiltersWithReason(rec common.Recommendation, cfg Config) (bool, string) { if !shouldIncludeRegion(rec.Region, cfg) { - return false + return false, "" } if !shouldIncludeInstanceType(rec.ResourceType, cfg) { - return false + return false, "" } if !shouldIncludeEngine(rec, cfg) { - return false + return false, "" } if !shouldIncludeAccount(rec.AccountName, cfg) { - return false + return false, "" } if !shouldIncludePoolSize(rec, cfg) { - return false + return false, common.DropMinPoolSize } - return true + return true, "" } // shouldIncludePoolSize filters out RI recommendations for pools whose diff --git a/cmd/multi_service_filters_test.go b/cmd/multi_service_filters_test.go index 215d65ae..886e00aa 100644 --- a/cmd/multi_service_filters_test.go +++ b/cmd/multi_service_filters_test.go @@ -111,7 +111,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)) diff --git a/cmd/multi_service_helpers.go b/cmd/multi_service_helpers.go index 317fff5d..db362af2 100644 --- a/cmd/multi_service_helpers.go +++ b/cmd/multi_service_helpers.go @@ -372,8 +372,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 @@ -382,8 +382,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 @@ -398,8 +398,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) @@ -456,9 +456,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)) @@ -479,7 +480,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) @@ -496,9 +500,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", @@ -515,16 +523,18 @@ func applyCoverageAndOverrides(recs []common.Recommendation, cfg Config, coverag return filteredRecs } -// checkDuplicatesAndApplyLimit checks for duplicate RIs and applies instance limits +// 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) adjustedRecs = filteredRecs // Continue with original recommendations if check fails @@ -535,6 +545,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 } @@ -553,7 +564,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, @@ -565,6 +577,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) @@ -576,7 +589,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 @@ -602,11 +615,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 @@ -615,7 +628,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, @@ -625,8 +639,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)) @@ -638,9 +653,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 7187e623..a09cf82c 100644 --- a/cmd/multi_service_test.go +++ b/cmd/multi_service_test.go @@ -837,7 +837,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)) }) } @@ -882,7 +882,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)) }) } @@ -957,7 +957,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)) }) } @@ -1021,7 +1021,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)) }) } @@ -1042,7 +1042,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/pkg/common/drop_summary.go b/pkg/common/drop_summary.go new file mode 100644 index 00000000..0ea82cd9 --- /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 synchronisation 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 00000000..8aec84e3 --- /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 475945f8..7b12400c 100644 --- a/providers/aws/recommendations/family_nu.go +++ b/providers/aws/recommendations/family_nu.go @@ -131,6 +131,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/unrecognised 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,7 +169,7 @@ 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 +// Returns (sizedRDS, nonRDS, drops). 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). @@ -160,17 +177,20 @@ 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 @@ -203,6 +223,7 @@ func partitionRDSRecsByFamily(recs []common.Recommendation) (map[string][]int, [ // 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. +// 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 +237,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 +260,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 e2474069..9d2fa7b5 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.ServiceSavingsPlans, 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") }) }