fix(attribution): priority meant the opposite of what it said - #118
Open
getnable wants to merge 1 commit into
Open
fix(attribution): priority meant the opposite of what it said#118getnable wants to merge 1 commit into
getnable wants to merge 1 commit into
Conversation
…pty results said nothing
Three things, all in the path that decides which team owns which dollar.
1. `priority` was inverted.
Documented as "lower number = higher priority". Implemented as rules sorted
ascending with every match overwriting the last, so the HIGHEST number won. Both
examples in the file write_example_rules() generates for users were wrong:
{team: frontend, costcenter: CC-9910} -> CC-9910 should be frontend
{team: infra-core} -> infra-core should be platform
The first is the expensive one. An org that tags properly got teams named after
cost centre codes, and since every dollar still lands in the correct total, the
symptom is not a broken report. It is a team disputing an invoice months later.
Now first-matching-rule-wins per field, which is what the docs describe and what
priority means everywhere else. Fields still resolve independently, and fallbacks
still fire when the higher-priority tag is absent, which is the thing an
over-correction would break.
This was already known. tests/attribution/test_mapper.py carried a
characterisation test pinning the buggy behaviour, with a comment saying the
disagreement was flagged and left alone because a perf-only refactor was the
wrong place to shift anyone's numbers. That was the right call then. This is the
right place, so the test now encodes the corrected contract.
NOTE: this changes attribution for anyone whose rules had a field collision.
That is the point, and it belongs in the release notes rather than a footnote.
2. Empty attribution now explains itself.
In AWS a tag does not reach Cost Explorer or the CUR until it is activated as a
cost allocation tag in Billing, which is a separate step in a different console.
Nothing errors when it is missing: every dollar simply lands in `unattributed`,
and a user who tagged their whole estate correctly concludes we are broken.
When a report comes back with nothing attributed, nable now says which configured
keys AWS is not carrying, and that activation is not retroactive.
Two rules it obeys. It never reports "inactive" when the check itself failed: a
guess about someone's billing configuration is worse than silence, because they
will act on it. And it goes through billing_access.ce_client like every other
Cost Explorer call rather than restating half the policy, so it is refused in
demo mode, in scheduled work, and under NABLE_NO_COST_EXPLORER=1. Cost Explorer
bills per request, and "surely one more call for a diagnostic" is exactly how a
per-request charge ends up on a timer. The ratchet in test_billing_access.py
would have caught a direct client here, and did.
3. docs/COST-ATTRIBUTION.md, because there was nothing.
Total prior documentation for this feature was one line in everything-nable.md
and a module docstring. An enterprise evaluator asking how spend maps to teams,
which is question two or three in any FinOps evaluation, had nothing to read.
Covers what it needs from you, that Terraform is optional and only for writing
tags back, the AWS activation step, the Azure inheritance policy, GCP labels, and
a plainly worded list of what it cannot do yet: tag-only matching, no shared cost
allocation, three fixed dimensions, no account hierarchy. Those limits are the
honest answer to a buyer comparing tools, and hiding them wastes everyone's time.
Mutation-tested, 7 of 7 caught. Suite 2739 passing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Three things in the path that decides which team owns which dollar.
1.
prioritymeant the opposite of what it saidDocumented as "lower number = higher priority". Implemented as rules sorted ascending with every match overwriting the last, so the highest number won. Both examples in the file
write_example_rules()generates for users were wrong:The first is the expensive one. An org that tags properly got teams named after cost centre codes. Every dollar still lands in the correct total, so this doesn't look like a broken report; it looks like a team disputing an invoice months later.
Now first-matching-rule-wins per field. Fields still resolve independently, and fallbacks still fire when the higher-priority tag is absent, which is what an over-correction would break.
This was already known.
test_mapper.pycarried a characterization test pinning the buggy behaviour, with a comment saying the disagreement was flagged and deliberately left alone because a perf-only refactor was the wrong place to shift anyone's numbers. Right call then. This is the right place, so that test now encodes the corrected contract.2. Empty attribution now explains itself
In AWS a tag does not reach Cost Explorer or the CUR until it's activated as a cost allocation tag in Billing. Separate step, different console. Nothing errors when it's missing: every dollar lands in
unattributed, and a user who tagged their whole estate correctly concludes we're broken.Two rules it obeys:
It never reports "inactive" when the check itself failed. A guess about someone's billing configuration is worse than silence, because they'll act on it.
It goes through
billing_access.ce_clientlike every other Cost Explorer call rather than restating half the policy, so it's refused in demo mode, in scheduled work, and underNABLE_NO_COST_EXPLORER=1. Cost Explorer bills per request, and "surely one more call for a diagnostic" is how a per-request charge ends up on a timer. The ratchet intest_billing_access.pywould have caught a direct client here, and did.3.
docs/COST-ATTRIBUTION.md, because there was nothingPrior documentation: one line in
everything-nable.mdand a module docstring. An enterprise evaluator asking how spend maps to teams had nothing to read.Covers what it needs from you, that Terraform is optional and only for writing tags back, the AWS activation step, Azure's inheritance policy, GCP labels, and a plainly worded list of what it cannot do yet: tag-only matching, no shared-cost allocation, three fixed dimensions, no account hierarchy. Those limits are the honest answer to a buyer comparing tools.
Mutation-tested, 7 of 7 caught. Suite 2739 passing.
🤖 Generated with Claude Code