Repository navigation
fix(gcp): avoid double counting monthly commitment costs - #408
Conversation
Derive monthly charges directly from catalog term totals and preserve zero upfront cost and break-even for Cloud SQL and Memorystore CUDs. Keep offering totals and unknown recurring pricing intact.
cristim
added
priority/p2
|
Warning Review limit reached
This review includes 4 billable files and costs up to $1.00.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 6 minutes for your next included review. View limit details
Comment |
|
Independent adversarial review and local verification cover exact head Two retained cold passes found no actionable findings. The reviewer attacked the public producer path, catalog validity, term division, payment aliases, independent offering totals, zero-upfront break-even, scorer/reporter behavior, count scaling, published dependency isolation and regression strength. Original parent and separate scratch mutations fail by runtime assertions. After rebase, the reviewer verified identical change bytes and unchanged affected dependencies, then reran public producer fixtures and the actual platform handoff at the new head. The actual platform scheduler converter and Commands passed: standalone Limitations: realistic synthetic Billing/Recommender fixtures, no live cloud economics or real-account acceptance; three-year arithmetic uses the existing SKU selection. The change preserves valid offering totals and unknown recurring pricing. Follow-up isolation, normalization and on-demand/savings work remains linked in #406, #407 and #399. Review is satisfied by independent exact-revision verification under the repository review gate. CodeRabbit has not been manually triggered for this PR; any automatic findings will still be checked and resolved. CI must be green on this exact SHA before merge. |
|
CodeRabbit automatic processing reported Review limit reached on this exact head and posted no review or inline findings. Its check is not evidence of a completed review. No manual paid review or quota retry was requested. The independent exact-revision review and local verification above cover the change under the repository review gate; merge remains blocked until CI is green on 83f0fb4. |
|
Final gate: exact reviewed head 83f0fb4, base 9511dcf, live CLEAN / MERGEABLE, zero review threads. All CI is green, including required CI Success and Run pre-commit hooks. Build/test workflow 38704321429 and pre-commit 38704321502 both completed SUCCESS; their independent background watchers terminated successfully. Independent two-pass review and actual producer-to-platform verification evidence is recorded above. CR waived: quota, adversarial review + local verification + green CI The automatic quota check performed no review. No manual paid review, quota trigger, or protection bypass is used. This PR is tracked for retrospective CodeRabbit review when quota returns; any actionable finding belongs in a follow-up PR. Follow-up issues filed: #406 and #407; remaining OnDemandCost and savings Duration normalization stays in #399. |
|
Merged PR #408 normally at main Post-merge consumer verification: the public GCP module resolves as Together with the independent parent RED / fixed GREEN through public producers and the actual platform scheduler converter plus Evidence uses realistic local SDK/catalog fixtures exercising production functions; ambient cloud credentials are disabled. No live economic acceptance, purchase or deployment is claimed. Recommendation: #391 is resolved by this merged fix. Follow-up issues filed: #406 for existing test credential isolation, #407 for payment-label normalization; #399 retains OnDemandCost and savings Duration normalization. Post-merge main CI build/test run |
|
Post-merge main verification is now terminal green at a3fb251: CI build/test run 38705503777 and pre-commit run 38705503654 both completed SUCCESS. Both main-run watchers and all earlier PR/merge watchers are reaped. Published standalone module build and fixture regression results remain recorded above; owner-safe merged worktree plus local/remote feature branch cleanup is complete. |
1 participant
Cloud SQL and Memorystore monthly CUD recommendations put the catalog term total in the upfront field and also emitted monthly charges. A catalog total near 1200 therefore became about 2400 in the platform's full-term commitment. A term-total-derived break-even override also caused no-upfront recommendations to be filtered out.
Derive recurring monthly charges directly from the positive catalog total and leave upfront cost and the override zero. Preserve offering totals and unknown recurring prices on catalog failure. This follows the existing Compute Engine contract.
Closes #391
Refs #399
Validation on
83f0fb40c0ffe9f91fc30ff584a9315558a7e305(tree53bc13f0ff247e620d6afcbde27a0b0b51a98d05):GetRecommendations, serialized into the actual platform scheduler converter andRecommendationRecord.TotalCommitment: parent fails near 2400 / 7200, fixed passes near 1200 / 3600 for one / three years. Default, monthly and no-upfront aliases and aggregate count resizing 3 to 2 covered.GOWORK=off. Fresh affected-service race tests pass after rebasing onto merged feat(ladder): expose scoped EC2 commitment rates #404. Normal local commit hooks passed, including tidy, vet, security and complexity checks.All subsequent local test and hook commands disable ambient cloud credentials; new tests inject billing fixtures. Evidence is synthetic SDK/catalog data exercising production functions, with no live purchase or deployment. Three-year tests cover current cashflow arithmetic, not independent term-specific vendor price acceptance. Aggregate count is seeded for the shared resizing path.
Remaining OnDemandCost term-basis and savings Duration work stays in #399. Separate follow-ups: #406 for existing fixture tests using ambient ADC, #407 for unsupported payment-label normalization. No unrelated calculation or module pin changes. Added comments: 2 / 136 lines (1.5%).