Add granularity and multi-dimension grouping to cost queries - #8
Merged
Conversation
CostQueryPayload hardcoded `granularity: 'None'` and a single grouping dimension, so a caller could only ever get one row per dimension value for the whole period. Attributing Azure spend to a day, or slicing it by resource group AND service at once, was not expressible. - `CostGranularity` enum (None/Daily). Monthly is deliberately absent: the query endpoint does not offer it, and a one-month period at None is how you ask for that. - `CostQueryPayload::$grouping` now accepts `string|list<string>`. The bare string is kept because it was the original signature and is the common case. - `CostQueryPayload::$granularity` defaults to None, so every existing caller gets byte-identical bodies. - `CostManagementResource::queryDaily()` as the named daily variant. Two ARM quirks are encoded rather than left to callers: - `grouping: []` is a 400, while omitting the key entirely is valid and means "one total for the whole period". Empty lists drop the key. - `array_filter` preserves keys, so a dropped empty dimension name would make the grouping encode as a JSON object and ARM would reject it. Reindexed. Daily changes the response SHAPE, not just its resolution — rows gain a `UsageDate` column (yyyyMMdd, as an integer) and the row count multiplies by the days in the period. That is why it is opt-in and why `queryDaily()` exists as a named method rather than a fourth argument callers pass without noticing. 14 tests covering every branch, including the two ARM quirks above and the request body actually sent. Co-Authored-By: Claude Opus 5 (1M context) <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
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.
Why
CostQueryPayloadhardcodedgranularity: 'None'and exactly one grouping dimension. A caller could only ever get one row per dimension value for the whole period — attributing Azure spend to a day, or slicing it by resource group and service at once, was not expressible.This came out of building per-workspace cost attribution in
flows-portal.docuhub, which had to settle for monthly totals as an interim.What
CostGranularityenum (None/Daily).Monthlyis deliberately absent — the query endpoint does not offer it, and a one-month period atNoneis how you ask for that.CostQueryPayload::$groupingnow acceptsstring|list<string>. The bare string is kept because it was the original signature and is the common case; the list matches ARM's owndataset.grouping, which is always an array.CostQueryPayload::$granularitydefaults toNone.CostManagementResource::queryDaily()as the named daily variant.Backwards compatibility
Non-breaking.
$granularitydefaults toNoneand$groupingstill accepts a string, so every existing caller produces a byte-identical request body — pinned by the first test inCostQueryPayloadTest.Two ARM quirks encoded rather than left to callers
grouping: []is a 400, while omitting the key entirely is valid and means "one total for the whole period". An empty list therefore drops the key instead of sending an empty array.array_filterpreserves keys, so dropping an empty dimension name from the middle of a list would make the grouping encode as a JSON object and ARM would reject it. Reindexed witharray_values().What callers need to know about
DailyIt changes the response shape, not just its resolution: rows gain a
UsageDatecolumn (yyyyMMdd, as an integer) and the row count multiplies by the days in the period. That is why it is opt-in, and whyqueryDaily()exists as a named method rather than something you pass as a fourth argument without noticing what comes back.Tests
14, covering every branch — both ARM quirks above, the string and list forms, the default body, and the request body actually sent through the connector.
composer testcomposer analysecomposer formatcomposer inventory:paritycomposer docs:apiCoverage was not verifiable locally (no xdebug/pcov on this machine — the 100% gate runs in CI); every branch has a test written for it deliberately.
One thing for the reviewer
queryDaily()does not appear in the generateddocs/api-reference.mdresource table. The generator's regex requiresnew SomeRequest(inside the method body, andqueryDaily()delegates toquery(). I chose not to duplicate the send logic just to satisfy the generator — happy to either inline it or drop the convenience method if you would rather the docs stay complete.🤖 Generated with Claude Code