fix(gcp/computeengine): normalize recommender savings by the projection duration

[cristim]

What

extractCostImpactFromRecommendation treated the Recommender CostProjection amount as monthly savings and never read CostProjection.Duration. It now scales the amount to a 730-hour month (monthly = amount x 730h / Duration; 730h = 8760/12, the month length the other providers use). A non-zero cost with a missing, zero or negative Duration returns an error wrapping errBadAmount, so GetRecommendations skips that recommendation with a log instead of reporting a window-less number as monthly. A zero cost needs no Duration.

Evidence is fixture-based (recommenderpb-shaped; the account has no live Recommender data). Existing cost fixtures gained a 730h Duration, including providers/gcp/recommendations_sdk_test.go (test file only; recommendations.go is untouched).

Tests

savings_window_test.go: 120 USD over 730h = 120/month; over 365 days = 10/month; over 15 days = 243.33/month; missing, zero and negative Duration error; zero cost with no Duration is fine; a rec without Duration is skipped by convertOrSkipBadAmount. Against main the new tests do not compile (the extractor returned nothing and ignored Duration, so a 365-day projection would be reported as 120 monthly); after the change GOWORK=off go vet/test ./... in providers/gcp passes.

Disjoint from go#307 (term resolution in convertGCPRecommendation): this PR touches only the cost-impact extractor and its call site.

Closes #384

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 4 billable files and costs up to $1.00.}

• 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 20 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: 16775463-d01f-4c54-8b58-0927c0a24f98

📥 Commits

Reviewing files that changed from the base of the PR and between 309cb5c and 586ad53.

📒 Files selected for processing: 4

• providers/gcp/recommendations_sdk_test.go
• providers/gcp/services/computeengine/client.go
• providers/gcp/services/computeengine/client_test.go
• providers/gcp/services/computeengine/savings_window_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 444f456: CHANGES REQUESTED

Normalising by CostProjection.Duration is the right fix. A missing, zero or negative Duration with a non-zero cost makes the rec go through convertOrSkipBadAmount, which logs and drops it. That is consistent with the existing errBadAmount handling and does not fabricate a figure, so it is fail-loud enough for this path.

Blocking

1. client.go const savingsWindow = 30 * 24 * time.Hour (720 h) does not match the month the rest of the repo uses: pkg/exchange and pkg/insurance use 730 h; AWS parser_sp.go hoursPerMonth = 730.0; GCP cloudstorage/client.go averageHoursPerMonth = 8760.0 / 12; Azure DaysPerMonth = 30.4375 (= 730.5 h); the platform's Archera DTO is monthly_730h_*. go#385 also sets RecurringMonthlyCost on the same rec to hourly x 730, so EstimatedSavings and MonthlyCost on one row would use different month lengths, and savings would be understated by 1.4%. The comment "GCP bills a month as 30 days" is not supported: GCP bills calendar months, and 30 days is the usual Recommender projection window, not a billing month. Suggested fix: const savingsWindow = 8760 * time.Hour / 12 (730 h, matching cloudstorage), and update the test expectations.

Non-blocking
2. Cloud SQL (cloudsql/client.go:489) and Memorystore (memorystore/client.go:457) read CostProjection the same way and also ignore Duration. Needs a follow-up issue.
3. A skipped rec shows up only as a log line. Fine for now; a count in the returned result would make it visible to callers.

Evidence

• Fail before: on main 856ccf9, a 120 USD projection over 365 days yields EstimatedSavings=120.
• Mutations: removing the Duration scaling is killed (TestExtractCostImpact_NormalizesToMonthly, 365-day and 15-day cases). Changing the window to 730 h is also killed by the 30-day expectations, so the fix above needs those expectations updated.
• GOWORK=off builds pass in all five modules, and vet and test pass in providers/gcp. Merges cleanly with go#381, fix(gcp/computeengine): report monthly CUD cost as recurring, not upfront #385, fix(gcp/computeengine): follow every page of the billing SKU listing #388 and fix(gcp): take the compute CUD term from the recommendation plan #307. CI on 444f456: all green.
• Fixture-based only, as the PR says; I had no live Recommender data either.

[cristim]

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

This re-review covers dc3ee68. The blocking item from my earlier review is fixed.

Checked

• savingsWindow = 730 * time.Hour (8760 / 12). That matches the pkg, AWS hoursPerMonth, GCP cloudstorage, and go#381's x730 for RecurringMonthlyCost and OnDemandCost, so savings and costs on one row use the same month. The "GCP bills a month as 30 days" comment is gone.
• Mutations: reverting to 30 * 24 * time.Hour is killed (TestComputeRecommendationsThroughSDK), and dropping the Duration scaling is killed (the 365-day and 15-day cases). The expected values are 120 x 730 / 8760 and 120 x 730 / 360.
• Fail before (earlier run on main): a 120 USD projection over 365 days was reported as 120/month.
• Skip-with-log through convertOrSkipBadAmount for a missing, zero or negative Duration: acceptable, and consistent with the existing errBadAmount handling.
• GOWORK=off build passes in all five modules; vet and tests pass in providers/gcp. CI on dc3ee68: 15 of 15 green. Merges cleanly onto current main and with go#381 (vet and tests pass on main + fix(gcp/computeengine): price CUDs per vCPU and GiB from the real SKU taxonomy #381 + fix(gcp/computeengine): normalize recommender savings by the projection duration #386 + fix(gcp/computeengine): follow every page of the billing SKU listing #388). No conflict with go#307.

Non-blocking

1. The SDK fixtures now use a 730 h Duration. Real Recommender CUD projections are usually 30 days (2592000s). A 30-day case (120 becomes 121.67) would show the effect on real data.
2. Cloud SQL and Memorystore still ignore Duration. Tracked in fix(gcp): Cloud SQL and Memorystore still use term-total cost fields and ignore the savings Duration #399.

[cristim]

Independent review (claude-opus-5-5) of 586ad53: APPROVE
Update-branch of the approved dc3ee68: only main's commits came in, as clean merges with no conflict resolution; the PR's own diff content is unchanged; GOWORK=off vet and tests pass in: providers/gcp.