From 0fcebc21dfc6da510dab3c0eefff3df6289a0890 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 13 Oct 2026 20:57:14 +0200 Subject: [PATCH] fix(gcp/computeengine): report monthly CUD cost as recurring, not upfront CommitmentCost is the upfront payment and is 0 for a monthly commitment; the committed price goes to RecurringMonthlyCost. The platform adds upfront plus monthly x term, so the term total in CommitmentCost was counted twice. BreakEvenMonths is dropped: with no upfront payment there is none. Closes #383 Refs platform#683 Co-Authored-By: Claude Sonnet 5.5 --- .../gcp/services/computeengine/client.go | 44 +++++-------------- .../gcp/services/computeengine/client_test.go | 10 +++-- 2 files changed, 19 insertions(+), 35 deletions(-) diff --git a/providers/gcp/services/computeengine/client.go b/providers/gcp/services/computeengine/client.go index db2c72dc..cb18257c 100644 --- a/providers/gcp/services/computeengine/client.go +++ b/providers/gcp/services/computeengine/client.go @@ -1067,8 +1067,8 @@ func (c *Client) getOrCreateBillingService(ctx context.Context) (BillingService, } // convertGCPRecommendation converts a GCP Recommender recommendation to common format. -// It also calls getComputePricing to fill CommitmentCost/OnDemandCost/SavingsPercentage/ -// BreakEvenMonths so the scorer can filter and rank GCP recommendations correctly +// It also calls getComputePricing to fill RecurringMonthlyCost/OnDemandCost/SavingsPercentage +// so the scorer can filter and rank GCP recommendations correctly // (issue #1022 C2). Pricing failures are logged but do not discard the recommendation: // EstimatedSavings from the Recommender payload is the authoritative savings signal. // Returns nil when the params.Term is unrecognized so the caller skips an @@ -1118,28 +1118,17 @@ func (c *Client) convertGCPRecommendation(ctx context.Context, gcpRec *recommend c.enrichRecWithPricing(ctx, rec) - // GCP Compute CUDs are monthly-payment commitments (PaymentOption is forced - // to "monthly" above), so the per-month charge is CommitmentCost / - // termMonths. enrichRecWithPricing performs the single billing lookup and - // populates CommitmentCost; reuse it here rather than issuing a second call. - // When the billing lookup failed CommitmentCost stays 0 and we leave - // RecurringMonthlyCost nil (unknown) so the frontend renders "—" rather than - // an incorrect explicit "$0" (nil means unavailable, 0 means known-zero fee). - if rec.CommitmentCost > 0 { - termYears := termYearsFromTerm(rec.Term) - monthly := rec.CommitmentCost / float64(termYears*12) - rec.RecurringMonthlyCost = &monthly - } - return rec, nil } -// enrichRecWithPricing fills CommitmentCost, OnDemandCost, SavingsPercentage, -// and BreakEvenMonths on rec by querying the billing catalog. Extracted from -// convertGCPRecommendation to keep that function's cyclomatic complexity under -// the gocyclo gate. A missing or invalid pricing entry is logged but does not -// discard the recommendation -- the Recommender-derived EstimatedSavings is -// still the authoritative savings signal (issue #1022 C2). +// enrichRecWithPricing fills OnDemandCost, SavingsPercentage and +// RecurringMonthlyCost on rec from the billing catalog. The recommendation is a +// monthly CUD with no upfront payment, so CommitmentCost (the upfront amount) +// stays 0 and the committed price is the monthly recurring cost. OnDemandCost is +// the on-demand price over the term. A missing or invalid pricing entry is logged +// and leaves RecurringMonthlyCost nil (unknown, not zero) without discarding the +// recommendation: the Recommender EstimatedSavings is still the authoritative +// savings signal (issue #1022 C2). func (c *Client) enrichRecWithPricing(ctx context.Context, rec *common.Recommendation) { if rec.ResourceType == "" { return @@ -1155,19 +1144,10 @@ func (c *Client) enrichRecWithPricing(ctx context.Context, rec *common.Recommend log.Printf("computeengine: pricing unavailable for %s in %s (issue #1020): %v", rec.ResourceType, c.region, err) return } - rec.CommitmentCost = pricing.CommitmentPrice + monthly := pricing.CommitmentPrice / float64(termYears*12) + rec.RecurringMonthlyCost = &monthly rec.OnDemandCost = pricing.OnDemandPrice rec.SavingsPercentage = pricing.SavingsPercentage - // BreakEvenMonths: months of accrued savings required to cover the - // commitment cost (conservative: treats the whole period cost as sunk, - // which is accurate for all-upfront and overestimates for monthly CUDs). - // monthlySavings = monthly spend difference between on-demand and CUD. - if pricing.OnDemandPrice > 0 && pricing.SavingsPercentage > 0 { - monthlySavings := pricing.OnDemandPrice * pricing.SavingsPercentage / 100.0 / float64(termYears*12) - if monthlySavings > 0 { - rec.BreakEvenMonths = pricing.CommitmentPrice / monthlySavings - } - } } // machineTypePathSegment precedes the machine type in a GCP resource path, e.g. diff --git a/providers/gcp/services/computeengine/client_test.go b/providers/gcp/services/computeengine/client_test.go index e91ddc58..53d5b8ef 100644 --- a/providers/gcp/services/computeengine/client_test.go +++ b/providers/gcp/services/computeengine/client_test.go @@ -1179,8 +1179,8 @@ func TestComputeEngineClient_ConvertGCPRecommendation(t *testing.T) { assert.Equal(t, 50.5, rec.EstimatedSavings) assert.Equal(t, "monthly", rec.PaymentOption, `GCP CUDs are billed monthly; PaymentOption must match ValidPaymentOptionsByProvider["gcp"]`) - // GCP Compute CUDs are monthly-billed and RecurringMonthlyCost is derived - // from CommitmentCost. No billing service is injected here, so the pricing + // GCP Compute CUDs are monthly-billed and RecurringMonthlyCost is priced from + // the billing catalog. No billing service is injected here, so the pricing // lookup fails and the field stays nil (unknown) rather than an incorrect // explicit 0 (nil means unavailable, 0 means a known-zero recurring fee). assert.Nil(t, rec.RecurringMonthlyCost) @@ -1348,7 +1348,11 @@ func TestConverterFillsPricingForScorer(t *testing.T) { // C2: SavingsPercentage must be > 0 so a MinSavingsPct filter doesn't silently drop the rec. require.Greater(t, rec.SavingsPercentage, float64(0), "convertGCPRecommendation must set SavingsPercentage > 0 from billing pricing (issue #1022 C2)") - assert.Greater(t, rec.CommitmentCost, float64(0), "CommitmentCost must be > 0") + assert.Zero(t, rec.CommitmentCost, "a monthly CUD has no upfront payment, so CommitmentCost is 0 (platform computes upfront + monthly x term)") + require.NotNil(t, rec.RecurringMonthlyCost) + // 4 vCPU and 6 GiB at the 1-year N1 commitment rates, per month. + assert.InDelta(t, (4*0.019915+6*0.002669)*8760/12, *rec.RecurringMonthlyCost, 1e-6) + assert.Zero(t, rec.BreakEvenMonths, "no upfront payment, so there is nothing to break even on") assert.Greater(t, rec.OnDemandCost, float64(0), "OnDemandCost must be > 0") // Apply a non-trivial MinSavingsPct filter and confirm the rec passes.