diff --git a/internal/database/postgres/migrations/000049_invalidate_aws_recs_missing_on_demand_cost.down.sql b/internal/database/postgres/migrations/000049_invalidate_aws_recs_missing_on_demand_cost.down.sql new file mode 100644 index 00000000..92ee8e93 --- /dev/null +++ b/internal/database/postgres/migrations/000049_invalidate_aws_recs_missing_on_demand_cost.down.sql @@ -0,0 +1,5 @@ +-- No rollback for the payload rewrite: the original AWS rows lacking +-- on_demand_cost were themselves stale (pre-PR-#312 data). Reverting +-- monthly_cost from null back to those stale values would re-introduce +-- the very bug this migration fixes. The correct values will be written +-- on the next scheduled collection. diff --git a/internal/database/postgres/migrations/000049_invalidate_aws_recs_missing_on_demand_cost.up.sql b/internal/database/postgres/migrations/000049_invalidate_aws_recs_missing_on_demand_cost.up.sql new file mode 100644 index 00000000..c8852664 --- /dev/null +++ b/internal/database/postgres/migrations/000049_invalidate_aws_recs_missing_on_demand_cost.up.sql @@ -0,0 +1,30 @@ +-- Rewrite stale pre-PR-#312 AWS recommendation payloads that lack +-- on_demand_cost in their JSONB payload. +-- +-- PR #312 added on_demand_cost population for AWS Savings Plans rows +-- (via CurrentAverageHourlyOnDemandSpend × 730). PR #277 added it for +-- Azure rows. Neither shipped a cache-invalidation migration for the +-- existing AWS rows already in the DB. +-- +-- Pre-#312 AWS rows have no on_demand_cost key, so the frontend falls +-- back to reconstructing the on-demand denominator from +-- monthly_cost + savings + amortized — a formula that double-counts +-- amortization vs. how AWS computes its savings amounts, producing +-- implausibly high Effective % values. +-- +-- Setting monthly_cost to null here causes the frontend to render "—" +-- instead, which is accurate ("data not yet collected") until the next +-- scheduler tick re-collects with correct on_demand_cost values. +-- +-- NOTE: We scope this strictly to AWS rows that lack on_demand_cost. +-- Azure rows were handled by migration 000046. GCP rows are not affected. +-- +-- NOTE: This UPDATE is idempotent — once a row has on_demand_cost +-- (written by the next scheduler tick), the NOT (payload ? 'on_demand_cost') +-- condition no longer matches it, so re-running this migration is safe. +-- +-- See GitHub issue #321 for the full root-cause investigation. +UPDATE recommendations + SET payload = jsonb_set(payload, '{monthly_cost}', 'null'::jsonb) + WHERE payload->>'provider' = 'aws' + AND NOT (payload ? 'on_demand_cost'); diff --git a/providers/aws/recommendations/parser_ri.go b/providers/aws/recommendations/parser_ri.go index 4d75f998..fe0ffbf7 100644 --- a/providers/aws/recommendations/parser_ri.go +++ b/providers/aws/recommendations/parser_ri.go @@ -2,6 +2,7 @@ package recommendations import ( "fmt" + "log" "math" "strconv" "time" @@ -127,6 +128,12 @@ func (c *Client) parseAWSCostDetails(rec *common.Recommendation, details *types. if onDemand, err := strconv.ParseFloat(*details.EstimatedMonthlyOnDemandCost, 64); err == nil { rec.OnDemandCost = onDemand } + } else { + // EstimatedMonthlyOnDemandCost absent from AWS CE response — OnDemandCost + // will be 0 and the scheduler's nonZeroPtr will store nil, causing the + // frontend to fall back to the reconstruction formula. Log so operators + // can detect when the API field is missing. See #321. + log.Printf("WARNING: EstimatedMonthlyOnDemandCost is nil for RI recommendation (service=%s, account=%s) — Effective %% will use reconstruction fallback", rec.Service, rec.Account) } // RecurringStandardMonthlyCost is the recurring charge per month for this RI. // It is distinct from CommitmentCost (upfront) and EstimatedMonthlySavingsAmount. diff --git a/providers/aws/recommendations/parser_sp.go b/providers/aws/recommendations/parser_sp.go index 6523caf2..283e969b 100644 --- a/providers/aws/recommendations/parser_sp.go +++ b/providers/aws/recommendations/parser_sp.go @@ -147,6 +147,13 @@ func (c *Client) parseSavingsPlanDetail( // where monthly_cost reflects only the no-upfront recurring charge, not // the full on-demand baseline). See #303. onDemandCost := parseOptionalFloat("CurrentAverageHourlyOnDemandSpend", detail.CurrentAverageHourlyOnDemandSpend) * hoursPerMonth + if detail.CurrentAverageHourlyOnDemandSpend == nil { + // CurrentAverageHourlyOnDemandSpend absent from AWS CE response — onDemandCost + // will be 0 and the scheduler's nonZeroPtr will store nil, causing the + // frontend to fall back to the reconstruction formula. Log so operators + // can detect when the API field is missing. See #321. + log.Printf("WARNING: CurrentAverageHourlyOnDemandSpend is nil for SP recommendation (planType=%s, account=%s) — Effective %% will use reconstruction fallback", planType, aws.ToString(detail.AccountId)) + } planTypeStr := string(planType) switch planType {