fix(cost): one rate table, checked against the live AWS Pricing API - #220
Merged
Conversation
Five copies of the S3 rate card, two of which disagreed by a factor of ten. internal/storage/s3 stored the Standard PUT rate as 0.0005 — the per-1,000 price — in a field it then divided by 1,000 again, so every write on the default configuration was costed at a tenth of its price. internal/cost had the same rate right, which is the more instructive half: what an operation cost depended on which package the caller reached for. Correcting the value alone would leave the arrangement that produced it. So: internal/awsrates is now the only table. Every rate in it was read from the live AWS Pricing API, and a build-tagged test re-reads 23 of them and fails on any difference — the distinction between a table someone believed and a table something checked. The two conversions that went wrong are in the type rather than at the call sites: per-request fields hold the cost of one call, and GBFromBytes divides by 10^9. Verified against live AWS, which turned up two rates #209 had not recorded: Glacier IR PUT was 4x low and its GET 5x low. And a second unit error, in internal/cost: bytes were converted to GB by dividing by 2^30, so every storage cost was 7.4% low. Its comment asserted the binary reading was correct, which is why it survived — a wrong unit that looks considered rather than mistaken. The tests could not catch it either: they fed 2^30 bytes, called it "one GB", and asserted the per-GB rate came back, an expectation that holds under both divisors. Each new test was verified in both directions. Injecting the exact original defect — Standard PUT scaled down 10x — fails the published-price test with "ratio to expected: 0.1x" and fails the live drift test; it does *not* fail the tier-ordering test, because a uniformly-low rate inverts nothing, and that limitation is now stated on the test rather than left to be assumed. Reverting bytesPerGB to 1<<30 fails the decimal-GB test by name. Removing a tier's rate fires the init-time panic in withRates. pricing_drift_test.go compared DefaultPrices against its own literal copy of the rates, on the stated grounds that importing internal/storage/s3 would cycle. It does not — go list -deps confirms it — so the literal was a third copy of the rate card, added by the test meant to catch there being more than one, and it had no REDUCED_REDUNDANCY entry so that tier went unchecked. It now compares both tables to awsrates for every class the loader accepts. The prose rates in internal/storage/s3's package doc are removed rather than restated. A rate in a doc comment has no way to be told it is stale. What stays is what is S3 behavior rather than S3 price: minimum billable size, minimum duration, retrieval latency. Closes #209.
This was referenced Aug 3, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #209.
The defect, and the shape that produced it
Five copies of the S3 rate card.
internal/storage/s3/pricing_manager.gostored the Standard PUT rate as0.0005— which is what AWS charges per 1,000 requests — in a field it then divided by 1,000 again. Every write on the default configuration was costed at a tenth of its price.internal/cost/pricing.gohad the same rate right. That is the more instructive half: the two disagreed by 10×, so what an operation cost depended on which package the caller reached for, and nothing flagged it.Correcting the value alone leaves the arrangement that produced it, and #209 says so explicitly — the consolidation is the prerequisite that makes #183 a single-site change instead of five.
What changed
internal/awsratesis now the only table. Every rate in it was read from the live AWS Pricing API rather than from a pricing page, andrates_aws_test.go(build tagintegration) re-reads 23 of them and fails on any difference. That is the difference between a table someone believed and a table something checked.Two conversions are in the type rather than at the call sites, because both are where the errors came from:
0.0005ended up in a per-request field;GBFromBytesdivides by 10⁹ — AWS bills decimal GB.All five sites now read from it:
internal/cost/pricing.go,internal/cost/reporter.go,internal/storage/s3/tiers.go,internal/storage/s3/doc.go,internal/analytics/model.go.Two more defects, found by checking against live AWS rather than against another table
And a second unit error, in
internal/cost: bytes were converted to GB by dividing by 2³⁰, making every storage cost 7.4% low. Its comment asserted the binary reading was correct, which is why it survived — a wrong unit that looks considered rather than mistaken. The tests could not catch it either: they fed1024*1024*1024bytes, called it "one GB", and asserted the per-GB rate came back — an expectation that holds under both divisors.Verification, in both directions
A test that cannot fail proves nothing, so each was checked by injection:
ratio to expected: 0.1xinternal/storage/s3testratio to expected: 0.1xinternal/costawsrates.All()awsratesbytesPerGBreverted to1<<30should cost $0.023, got $0.0214…The fourth row is the point of doing this:
TestRateOrderingIsEconomicallySanelooks like it would catch a bad PUT rate and does not, because a uniformly-low rate inverts no ordering. That limitation is now stated on the test, verified by injection, rather than left to be assumed — and the published-price test exists because of it.pricing_drift_test.goIt compared
DefaultPricesagainst its own literal copy of the rates, on the stated grounds that importinginternal/storage/s3would create a cycle. There is no cycle —go list -depsconfirms s3 does not import cost — so the literal was a third copy of the rate card, added by the test whose purpose was to catch there being more than one. It also had noREDUCED_REDUNDANCYentry, so that tier went unchecked by the drift test that existed to check tiers.It now compares both tables directly against
awsrates, for every class the config loader accepts.Prose rates removed, not restated
internal/storage/s3's package doc quoted a per-GB price for each tier and had a Cost/GB column in its summary table. Both are gone. A rate in a doc comment has no way to be told it is stale, so the only question is when it starts lying. What stays is what is S3 behavior rather than S3 price — minimum billable size, minimum storage duration, retrieval latency — which changes when AWS changes the product, not when AWS changes a number.Gates
Coverage:
internal/awsrates100.0%,internal/cost98.8%,internal/analytics96.8%,internal/storage/s384.8%.Note for #183
internal/awsratesis now the single site that issue's premise assumed. The drift test also documents why it shells out to the AWS CLI rather than importingservice/pricing: that package forces smithy-go ≥1.26.0, and smithy-go sits under the S3 client serving every read and write — taking that bump for a drift check would put dependency risk on the data path to test a table. #183 owns adding the pricing client deliberately.