Skip to content

fix(gcp): converter never sets Count/SavingsPct/Cost — CUD purchase requests 0 vCPU; scorer drops all GCP recs #1022

Description

@cristim

Problem. convertGCPRecommendation (all four GCP services) sets only EstimatedSavings and leaves Count, SavingsPercentage, CommitmentCost, OnDemandCost, BreakEvenMonths at zero.

  • C1: buildInsertRequest (computeengine/client.go:507-540) does Amount: int64(rec.Count) (and *4096 for memory) → a Recommender-driven purchase requests a 0 vCPU / 0 MB commitment. Green tests hide it (they hand-build Recommendation{Count:N}).
  • C2: pkg/scorer filters on SavingsPercentage (MinSavingsPct), BreakEvenMonths, Count (MinCount) — all zero for GCP → any non-zero filter silently drops all GCP recs; when they pass they rank to the bottom.

Evidence. computeengine/client.go:795-811 (converter), :507-540 (insert); pkg/scorer/scorer.go:80-90. GCP pricing fns compute the numbers but are only called from GetOfferingDetails, never the converter.

Impact. GCP CUD purchase happy-path is broken/useless; GCP recs silently filtered/mis-ranked. Money path.

Suggested fix. Extract vCPU count from the Recommender payload → set rec.Count; thread the pricing path into the converter to fill CommitmentCost/OnDemandCost/SavingsPercentage/BreakEvenMonths. Guard buildInsertRequest/PurchaseCommitment to refuse rec.Count <= 0. Also fix 10-M5 (per-service hardcoded PaymentOption — see #718/#829 for the monthly-stamp part). Regression test: realistic Recommender response → converter → Count > 0 and VCPU Amount set; through scorer with MinSavingsPct set, GCP recs survive.

References. Source: report 10 (C1, C2, M5) + architecture note on scorer contract. Related #264/PR #811 (RecurringMonthlyCost only), #718/PR #829 (payment-option monthly stamp).


Filed from automated adversarial code review (see docs/code-review/). Source finding(s): 10-C1, 10-C2, 10-M5.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingeffort/mDaysimpact/manyAffects most userspr-createdA PR has been opened for this issue (dedup guard for the auto-PR loop)pr-mergedThe PR for this issue has been mergedpriority/p1Next up; this sprintseverity/highSignificant harmtriagedItem has been triagedtype/bugDefecturgency/this-sprintWithin the current sprint

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions