You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(providers/aws): fail loud on unparseable SP cost fields + RI utilization hours (COR-07 siblings to #1171) #18
PR LeanerCloud/cloud-commitments-cli#1236 (closes LeanerCloud/cloud-commitments-cli#1171, COR-07) made AWS RI cost-field parsing fail loud in parseAWSCostDetails (providers/aws/recommendations/parser_ri.go). The same silent-swallow pattern still exists in two sibling code paths in the same package, on equally money-affecting fields:
EstimatedAverageUtilization -> utilization signal (non-money, "0 = no signal" semantics documented, so the silent fallback is intentional here)
Same shape as COR-07: an all-upfront SP with an unparseable UpfrontCost would surface as $0 upfront -> wrong money figure feeding effective-savings math and purchase decisions, with only a WARNING log.
2. RI utilization parser (providers/aws/recommendations/utilization.go)
parseFloat (lines 142-149) silently returns 0 on parse failure for Utilization.PurchasedHours, Utilization.TotalActualHours, Utilization.UnusedHours (lines 131-138). These accumulate into riAccumulator, then buildUtilizations divides:
pct= (a.totalActualHours/a.purchasedHours) *100
A parse failure on purchasedHours quietly understates utilization or yields 0%; a failure on totalActualHours understates the numerator. This corrupts the --target-coverage sizing input directly.
Why this is the same bug as COR-07
Memory feedback_no_silent_fallbacks.md ("no fallbacks/defaults/fabricated values on money paths; return an explicit error") covers this exact pattern. PR LeanerCloud/cloud-commitments-cli#1236 fixed RI cost fields; SP cost fields and RI utilization hours are equivalent money-path inputs in the same package.
Suggested fix
Mirror the parser_ri.go pattern: parseOptionalFloat and parseFloat return (float64, error). Callers either propagate to parseSavingsPlanDetail / GetRIUtilization (which then skip-with-warning, matching the existing RI skip in parseRecommendations for parseRecommendationDetail failures), or for utilization-signal-only fields (EstimatedAverageUtilization, AverageNumberOfInstancesUsedPerHour, AverageUtilization) keep the log-and-default semantics with a docstring noting "zero means no signal".
Problem
PR LeanerCloud/cloud-commitments-cli#1236 (closes LeanerCloud/cloud-commitments-cli#1171, COR-07) made AWS RI cost-field parsing fail loud in
parseAWSCostDetails(providers/aws/recommendations/parser_ri.go). The same silent-swallow pattern still exists in two sibling code paths in the same package, on equally money-affecting fields:1. Savings Plans parser (
providers/aws/recommendations/parser_sp.go)parseOptionalFloat(lines 182-192) silently returns0whenstrconv.ParseFloatfails, then is called for every money field on every SP recommendation:Callers (
parseSavingsPlanDetail, lines 202-221):HourlyCommitmentToPurchase-> drivesRecurringMonthlyCost(× 730) andhourlyCommitmentfield for the recommendationEstimatedMonthlySavingsAmount->rec.EstimatedSavings(direct savings figure)EstimatedSavingsPercentage->rec.SavingsPercentageUpfrontCost->rec.CommitmentCostCurrentAverageHourlyOnDemandSpend->rec.OnDemandCost(× 730) — the denominator in effective-savings math (see issue fix(recommendations/aws): incorrect Effective Savings % — plumb on_demand_cost like Azure (#277) cloud-commitments-cli#303)EstimatedAverageUtilization-> utilization signal (non-money, "0 = no signal" semantics documented, so the silent fallback is intentional here)Same shape as COR-07: an all-upfront SP with an unparseable
UpfrontCostwould surface as$0 upfront-> wrong money figure feeding effective-savings math and purchase decisions, with only aWARNINGlog.2. RI utilization parser (
providers/aws/recommendations/utilization.go)parseFloat(lines 142-149) silently returns0on parse failure forUtilization.PurchasedHours,Utilization.TotalActualHours,Utilization.UnusedHours(lines 131-138). These accumulate intoriAccumulator, thenbuildUtilizationsdivides:A parse failure on
purchasedHoursquietly understates utilization or yields0%; a failure ontotalActualHoursunderstates the numerator. This corrupts the--target-coveragesizing input directly.Why this is the same bug as COR-07
Memory
feedback_no_silent_fallbacks.md("no fallbacks/defaults/fabricated values on money paths; return an explicit error") covers this exact pattern. PR LeanerCloud/cloud-commitments-cli#1236 fixed RI cost fields; SP cost fields and RI utilization hours are equivalent money-path inputs in the same package.Suggested fix
Mirror the parser_ri.go pattern:
parseOptionalFloatandparseFloatreturn(float64, error). Callers either propagate toparseSavingsPlanDetail/GetRIUtilization(which then skip-with-warning, matching the existing RI skip inparseRecommendationsforparseRecommendationDetailfailures), or for utilization-signal-only fields (EstimatedAverageUtilization,AverageNumberOfInstancesUsedPerHour,AverageUtilization) keep the log-and-default semantics with a docstring noting "zero means no signal".Acceptance
HourlyCommitmentToPurchase,EstimatedMonthlySavingsAmount,UpfrontCost,CurrentAverageHourlyOnDemandSpendfor SP; malformedPurchasedHours,TotalActualHoursfor utilization. Tests confirmed FAILING pre-fix, PASSING post-fix.strconv.ParseFloat(..., 64); err == nil { rec.<money field> = v }patterns left inproviders/aws/recommendations/for money paths.Related
feedback_no_silent_fallbacks.md,feedback_nullable_not_zero.md,feedback_strict_int_parse.md