[cristim]

What

GCP Compute CUD pricing never resolved: skuMatchesMachineType required the machine type (e.g. n2-standard-8) in the SKU description, and 0 of the 33,453 live Compute Engine SKUs contain one. Commitment SKUs are per vCPU-hour and per GiB-hour ("Commitment v1: N2 Cpu in Americas for 1 Year", units h / GiBy.h).

• G1 (fix(gcp/computeengine): GCE CUD offering price per vCPU and per GB, not per machine type #321, refs fix(gcp): SKU lookup for Cloud SQL, Memorystore and Compute matches tier strings real descriptions may not contain #245): new pricing.go selects the family core and RAM SKUs by catalog family label, serviceRegions and term (Commit1Yr/Commit3Yr), checks the usage unit, and prices vCPUs x core rate + GiB x RAM rate for both commitment and on demand. vCPU and memory come from rec.Count and ComputeDetails.MemoryGB; GetOfferingDetails needs them too and now errors without them. Conflicting duplicate, mixed-currency, unit-mismatch and missing-currency SKUs error; the "USD" default is gone. The catalog is cached per client so N recommendations cost one fetch.
• G4 (fix(gcp/computeengine): monthly CUD recs report the term total as CommitmentCost #383, refs platform#683; absorbs the former go#385): cost fields follow the shared semantics. CommitmentCost is 0 (monthly, no upfront); RecurringMonthlyCost is the committed monthly amount (nil when pricing failed); OnDemandCost is the monthly on-demand price of the same resources (730 h/month, as in AWS, Azure and the platform baseline); BreakEvenMonths is no longer set (no upfront payment). The platform computes upfront + monthly x term (scheduler.go:1650), so the old term-total CommitmentCost was counted about 2x; platform owner to confirm on platform#683. Forced monthly payment stays (GCP CUDs bill monthly, https://cloud.google.com/docs/cuds).
• G3 (fix(gcp): Compute Engine commitment price is not scaled by hours in the term, so savings and break-even are wrong #271): CommitmentPrice and OnDemandPrice are both term totals and HourlyRate is the committed hourly cost, so savings are no longer about 99.99%.
• G2 (refs fix(gcp): billing SKU listing reads one page and drops the NextPageToken #92): SKU paging for this catalog (33,453 SKUs over 7 pages of 5000) is go#388, which uses the shared skulist.All helper. This PR's pricing needs that paging to take effect live.

Families whose catalog label differs from the machine prefix are mapped explicitly (n1, n2d, c2d, t2d, c4a, c2, m1/m2, m3), from the live catalog.

Live-catalog evidence

Fetched services/6F81-5844-456A/skus with gcloud auth: 7 pages, 33,453 SKUs. n2-standard-8 (8 vCPU, 32 GiB), us-central1, 1 year:

	core /h	RAM /GiB-h	hourly
on demand (N2 Instance Core/Ram running in Americas)	0.031611	0.004237	0.388472
commitment (Commitment v1: N2 Cpu/Ram in Americas for 1 Year)	0.019915	0.002669	0.244728

TestGetComputePricing_N2StandardFromLiveShapedCatalog (SKUs copied from the live catalog) yields HourlyRate 0.244728, on-demand 0.388472 x 8760, savings 37.0%, equal to the hand calculation.

Fail before, pass after

Before (main e9a33bb, live-shaped N2 SKUs through extractComputePricingFromSKUs):

--- FAIL: TestBeforeLiveShapedCatalog
    onDemand=0 commitment=0 err=<nil>
    pricing never resolves for n2-standard-8: onDemand=0 commitment=0

Cost semantics: TestConverterFillsPricingForScorer failed before with CommitmentCost 838.10 and BreakEvenMonths 20.43 where 0 was expected; TestEnrichRecWithPricing_MonthlyFiguresPerTerm pins the 1yr and 3yr monthly figures (n2-standard-8: hourly x 730).

After: GOWORK=off go build/vet/test ./... in providers/gcp passes. Existing fixtures with fabricated descriptions ("n1-standard-4 VM running in Americas") were replaced with live-shaped SKUs; usage_type_test.go and TestSkuMatchesMachineType tested the removed matcher and are gone.

Notes

• The savings window is go#386. Term from the recommendation plan is go#307; this PR edits disjoint lines.
• golangci-lint could not run locally (binary built with go1.26, module needs go1.27); the repo pre-commit hooks (gofmt, vet, gocyclo, gosec) passed.

Closes #271
Closes #321
Closes #383
Refs platform#683
Refs #92 #245

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 5 billable files and costs up to $1.25.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 30 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 61 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 4f497b27-06ae-4c4e-ae6a-a54d5eea001b

📥 Commits

Reviewing files that changed from the base of the PR and between 7eaf9c5 and 17a3d18.

📒 Files selected for processing: 5

• providers/gcp/services/computeengine/client.go
• providers/gcp/services/computeengine/client_test.go
• providers/gcp/services/computeengine/pricing.go
• providers/gcp/services/computeengine/pricing_test.go
• providers/gcp/services/computeengine/usage_type_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent review (claude-opus-5-5) of 1a25519: CHANGES REQUESTED (merge order only)

The pricing engine is correct against the live catalog. The one blocking item is about when it merges, not about pricing.go.

Blocking

1. Do not merge this ahead of go#385, and go#385 needs a fix first (see the review there). Before this PR, Compute pricing never resolved, so CommitmentCost, RecurringMonthlyCost and OnDemandCost were 0 or nil (unknown). With this PR alone, live recs carry CommitmentCost = the term total and RecurringMonthlyCost = total / months, which is the platform#683 2x overcount, and OnDemandCost = the on-demand price over the whole term, where the platform reads a monthly baseline (internal/config/types.go:592, platform 489f4bf). That turns "unknown" into wrong numbers. Land it together with a fixed go#385, or fold that fix in here.

Live re-derivation (read-only catalog GETs, 7 pages, 33,453 SKUs), n2-standard-8 (8 vCPU, 32 GiB), us-central1:

• on demand: BB77-5FDA-69D9 N2 Instance Core running in Americas 0.031611/h, 5B01-D157-A097 N2 Instance Ram 0.004237/GiB-h, so 8 x 0.031611 + 32 x 0.004237 = 0.388472/h
• 1yr: 672D-3FDD-7138 Commitment v1: N2 Cpu in Americas for 1 Year 0.019915, 2F6C-EB97-C718 ... Ram ... 0.002669, so 0.244728/h and 37.00% savings
• 3yr: FCA2-37F3-16BA 0.014225 and B638-CAE8-6E60 0.001907, so 0.174824/h and 55.00%
• These match the PR. The "Americas" SKUs carry explicit serviceRegions (us-central1, us-east1, us-west1), so region matching uses serviceRegions, not the description. A real-API end-to-end run (with go#388 merged locally) gave the same figures.

Live sweep of findFamilyRates (20 families x 47 regions x 2 terms against the full live catalog). No priced result had a commitment rate at or above the on-demand rate, and every non-price came back as an error, never as a guess. Coverage gaps, all non-blocking:
2. c2: 112 legacy SKUs are named Commitment: Compute optimized Core running in <X> for 1 Year, not Commitment v1:. c2 errors with "no commitment pricing" in 60 of 84 region-terms, including us-central1, so the override comment "verified against the live catalog" holds only for the v1 regions.
3. c4d: the RAM SKUs are priced per GBy.h (e.g. 159A-6F40-D544), so every c4d lookup fails with a unit-mismatch error (0 of 94). That is correct fail-loud behaviour, but c4d never prices.
4. m1/m2 return ambiguous catalog in asia-northeast1, us-east4 and asia-southeast1 (two Memory-optimized Instance Core SKUs with different prices). h4d on-demand resolves in only 12 of 94 region-terms.
5. Custom machine types (n2-custom-*) price at predefined rates. Live custom on-demand is 0.03319155/h against 0.031611, and the commitment adds a CmtCudPremium SKU, so both sides are understated by about 5%.
6. Fixtures: the N1 SKUs in pricing_test.go are exact live copies (IDs, groups, prices). The N2 entries have the live descriptions and prices but placeholder IDs (n2-od-core), and the decoy prices are invented (custom 0.033 vs live 0.03319155; sole-tenancy premium 0.001 vs 0.0031611). The catalogSKU comment ("prices are copied from real SKUs") overstates this.
7. GetOfferingDetails parses only "3yr"/"3" for the term, while termYearsFromTerm also accepts "36mo". This predates the PR.
8. Text conflict with go#388 in the import block. Keep skulist and drop skumatch.

Currency: every live SKU is USD (no currencyCode is requested). A missing or mixed currency errors, and the old "USD" default is gone. Correct.

Evidence

• Fail before: on main 856ccf9, getComputePricing("n2-standard-8", "us-central1", 1) fed the full live catalog returns no on-demand pricing found for machine type n2-standard-8.
• Mutations killed: commitment RAM read from the on-demand rate; the term check sub[2] != m.wantTerm dropped; term hours ignoring termYears.
• GOWORK=off build passes in all five modules, and vet and test pass in providers/gcp. CI on 1a25519: all green. No conflict with go#307.

[cristim]

Independent review (claude-opus-5-5) of 6102004: APPROVE

This re-review covers 6102004, which carries the go#381 pricing plus the former go#385 cost semantics. The blocking item from my earlier review is fixed.

Checked

• enrichRecWithPricing: CommitmentCost = 0, RecurringMonthlyCost = term total / (termYears x 12) = committed hourly x 730, and OnDemandCost = on-demand term total / (termYears x 12) = on-demand hourly x 730, now monthly like AWS, Azure and the platform baseline. BreakEvenMonths is no longer set. The 730 h month matches go#386's savingsWindow.
• Live end to end (real Cloud Billing API, read-only GETs, 33,453 SKUs, local merge of main + this PR + go#386 + go#388), n2-standard-8 in us-central1:
  • 1yr: CommitmentCost 0, Recurring 178.651440, OnDemand 283.584560 (= 0.388472 x 730), 37.00%
  • 3yr: Recurring 127.621520, OnDemand 283.584560, 55.00%
  • All of these match a hand calculation from the live SKUs.
• New TestEnrichRecWithPricing_MonthlyFiguresPerTerm kills both mutations: termMonths := 12 (fails the 3yr case) and OnDemandCost as the term total (fails both terms). Commitment RAM read from the on-demand rate is also killed.
• GOWORK=off build passes in all five modules; vet and tests pass in providers/gcp. CI on 6102004: 15 of 15 green.
• The body has Closes #271 #321 #383 and Refs platform#683. fix(gcp/computeengine): report monthly CUD cost as recurring, not upfront #385's scope (G4) is carried over.
• Merges cleanly onto current main, together with go#386. No conflict with go#307.

Non-blocking

1. Merging with go#388 still conflicts in the client.go import block. Keep skulist and drop skumatch; vet and tests pass on the resolved tree.
2. The N2 fixture SKU IDs are still placeholders (n2-od-core etc.; the live IDs are BB77-5FDA-69D9, 5B01-D157-A097, 672D-3FDD-7138, 2F6C-EB97-C718, FCA2-37F3-16BA, B638-CAE8-6E60). Tracked in test(gcp/computeengine): replace placeholder SKU IDs in the N2 pricing fixtures with live IDs #398.
3. The body's G2 paragraph still says paging is a future follow-up; it is go#388.
4. The coverage gaps from the earlier review are tracked as follow-ups: fix(gcp/computeengine): c2 pricing fails for legacy 'Commitment: Compute optimized' SKU names #393 (c2), fix(gcp/computeengine): c4d commitment RAM priced in GBy.h never prices #394 (c4d), fix(gcp/computeengine): m1 and m2 share one catalog label and are ambiguous in 3 regions #395 (m1/m2), fix(gcp/computeengine): h4d has on-demand SKUs in only 12 of 94 regions #396 (h4d), fix(gcp/computeengine): custom machine types are priced at predefined rates #397 (custom machine types), and fix(gcp): Cloud SQL and Memorystore still use term-total cost fields and ignore the savings Duration #399 / fix(gcp/cloudsql,memorystore): BreakEvenMonths treats the term total of a monthly CUD as upfront #391 (Cloud SQL and Memorystore).

[cristim]

Independent review (claude-opus-5-5) of 17a3d18: APPROVE
Covers go#381 after the author merged main (7eaf9c5, which includes #388): compared with the approved 6102004, the only change is merge 17a3d18, whose conflict resolution in providers/gcp/services/computeengine/client.go keeps main's skulist import and realBillingService ctx and drops the now-unused skumatch import (checked with git show --cc); the PR's own diff content vs main is identical to the approved diff; GOWORK=off vet and tests pass in: providers/gcp.