[cristim]

Closes #74. This is PR 2. PR 1 (#689) added common.Commitment.CostKnown to pkg.

What

• Pin: providers/aws and providers/azure now pin pkg at 8d06fde (feat(common): add Commitment.CostKnown #689).
  • The azure re-pin moves aws-sdk-go-v2 to v1.47.5 and smithy-go to v1.28.7 transitively. Both are forced by MVS from pkg's go.mod, and azure reaches them only through pkg/provider.
  • A follow-up commit tidies the azure go.sum.
• AWS ladder: riToCommitment sets CostKnown only when the RI has a Duration. Without one, riHourlyCost can't amortize the upfront part, and the listing maps missing price pointers to 0. spToCommitment sets CostKnown: true, because the SP amount comes straight from the API, and a real 0 counts as known.
• Azure savings plans: a reported Commitment.Amount, including 0, is marked known. A nil amount stays unknown.
• Azure inventory: the cache, cosmosdb, database, search and synapse reservation converters stop inventing Region: c.region and State: Active. Consumption ReservationDetail carries neither field, so both are now "" (unknown). managedredis already reads real data since fix(azure): dedupe Redis recommendations against held reservations #706.
• Docs: the stale inventory.go comment now says Cost is unknown and CostKnown stays false, so a zero must not be read as free.

Behaviour notes

• Dedupe is unaffected for the five converters. Their StartDate is zero, so recfilter.isRecentActiveCommitment excludes them, before and after this change.
• Each listed commitment from those converters now logs one WARNING in recfilter, for its unrecognized empty State. The plan's change to silence it was dropped: fix(azure): dedupe Redis recommendations against held reservations #706's strict inventory path relies on "" meaning an unknown provisioning state. feat(common): add Commitment.CostKnown #689's squash message, which says empty State is "counted without logging", is inaccurate. No recfilter change has landed.
• Residual (follow-up): the ec2 listing turns nil pricing pointers into 0, so an RI with a Duration but a missing FixedPrice can't be told apart from a no-upfront RI.
• Residual (follow-up): Commitment has no currency field, so non-USD amounts aren't flagged.

Verification

• Tests first:
  • a zero-HourlyCommitmentUSD SP;
  • a fully-upfront RI with a Duration (known, Cost > 0) and one without a Duration (unknown);
  • an ec2 listing test pinning unknown cost;
  • savings plans with amount 0, nil amount and nil commitment;
  • a shared AssertUnknownInventory helper, reused by the 5 converter tests, run for both eastus and westeurope client regions.
• Deletion probes: putting back the invented Region or State fails each converter test. Setting CostKnown unconditionally fails the RI and savings plan tests.
• Rebased onto current main:
  • aws: go test -p 2 -short ./... gives 15 ok;
  • azure: 14 ok;
  • golangci-lint: 0 issues in both.
• Reviews: two adversarial task-review passes covering the whole branch. Pass 1 found that the RI with no Duration was wrongly marked known. That's fixed, and pass 2 was clean.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 26 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-go
• Branch: fix/74-cost-known-producers
• File: .coderabbit.yaml
• Commit: 93a29ee
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1c9c8614-2e80-436e-b7f5-a999f9a2ec01

Commits

Reviewing files that changed from the base of the PR and between 5dcf711 and 93a29ee.

Files ignored due to path filters: 2

• providers/aws/go.sum is excluded by !**/*.sum
• providers/azure/go.sum is excluded by !**/*.sum

Files selected for processing: 19

• providers/aws/go.mod
• providers/aws/ladder/commitments.go
• providers/aws/ladder/commitments_test.go
• providers/aws/services/ec2/client_test.go
• providers/azure/go.mod
• providers/azure/services/cache/client.go
• providers/azure/services/cache/client_test.go
• providers/azure/services/cosmosdb/client.go
• providers/azure/services/cosmosdb/client_test.go
• providers/azure/services/database/client.go
• providers/azure/services/database/client_test.go
• providers/azure/services/internal/reservations/inventory.go
• providers/azure/services/internal/reservations/reservationstest/inventory.go
• providers/azure/services/savingsplans/client.go
• providers/azure/services/savingsplans/client_test.go
• providers/azure/services/search/client.go
• providers/azure/services/search/client_test.go
• providers/azure/services/synapse/client.go
• providers/azure/services/synapse/client_test.go

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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn