fix(hubs): use max() when duplicate on-demand price rows collapse under one lookup key#2189
Merged
Merged
Conversation
…rreported savings When MCA price sheets emit a blank x_SkuOfferId, multiple Consumption price rows for one meter collapse under the same reservation price lookup key. The duplicates are identical in every identifier (including SkuPriceId and effective period), differing only in price, so the collision can't be resolved by refining the key. min() selected the anomalously low row, deflating the on-demand baseline and underreporting commitment discount savings (sometimes to zero). Switch the ListUnitPrice/ContractedUnitPrice aggregation from min() to max() in both v1_0 and v1_2 ingestion paths. For EA / populated-offer tenants the key already separates rows, so each group has a single row and min == max -- a no-op, making the change safe across billing account types. Fixes #2176 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
microsoft-github-policy-service
Bot
added
the
Needs: Review 👀
Contributor
There was a problem hiding this comment.
Pull request overview
Updates FinOps hubs ingestion KQL to prevent underreported reservation/commitment discount savings when multiple on-demand price rows collapse under the same reservation price lookup key.
Changes:
- Switched reservation reference price aggregation from
min()tomax()forListUnitPriceandContractedUnitPricein both v1.0 and v1.2 ingestion paths. - Added inline rationale comments near the updated
summarizestatements. - Added a FinOps hubs v15 changelog entry documenting the fix (Issue #2176).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_2.kql | Uses max() when summarizing on-demand prices by reservation lookup key to avoid deflated savings baselines. |
| src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_0.kql | Mirrors the v1.2 change for the v1.0 ingestion path to keep behavior consistent across supported transformations. |
| docs-mslearn/toolkit/changelog.md | Documents the fix under FinOps hubs v15 → Fixed with a link to #2176. |
Describe the general duplicate-key behavior instead of the MCA blank-offer example, which is filtered out by the isnotempty(x_SkuOfferId) gate earlier in the block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The min->max change does not resolve the MCA scenario in #2176: blank-offer cost rows are filtered out by the isnotempty(x_SkuOfferId) gate before this join, so they never reach the price collapse. Reframe the changelog entry as general hardening and reference the PR instead of #2176. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RolandKrummenacher
changed the title
flanakin
approved these changes
Jun 24, 2026
This was referenced Jul 16, 2026
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.
🛠️ Summary
General robustness fix for the FinOps hubs reservation price backfill. When duplicate on-demand price rows collapse under a single reservation price lookup key, prefer the highest price as the savings baseline instead of the lowest.
Note
This is a general hardening, not a complete fix for #2176 (MCA underreported savings). See "Scope / relationship to #2176" below.
🐛 Behavior change
Missing on-demand prices are backfilled via a leftouter join against the
Pricestable, aggregated by the reservation price lookup key (x_BillingProfileId+YYYY-MM+x_SkuMeterId+x_SkuOfferId). When multiple Consumption price rows collapse under one key and differ only in price,min()selects the lowest row — which can be an anomalously low value — deflating the on-demand baseline and understating commitment discount savings.Switching the
ListUnitPrice/ContractedUnitPriceaggregation frommin()tomax()selects the genuine on-demand price in those collision cases.IngestionSetup_v1_2.kqlIngestionSetup_v1_0.kqlFor tenants whose price rows carry distinct, populated offer IDs (e.g. EA), the lookup key already separates rows, so each
summarizegroup has a single row andmin == max— a no-op. The change therefore only affects genuine collision cases and is safe across billing account types.🔎 Scope / relationship to #2176
This PR does not close #2176. In the reporter's MCA scenario, the affected price rows carry a blank
x_SkuOfferId. The cost-sidetmp_MissingPricesgate requiresisnotempty(x_SkuOfferId), and the lookup key on both sides includes the offer ID — so blank-offer rows are filtered out before this join and never reach themin()/max()collapse. The MCA fix requires relaxing that offer-ID gate and/or keying on a field populated for MCA; tracked in #2176.📝 Notes
release/and fabric KQL copies are regenerated at release time and are intentionally not included here (consistent with prior KQL fixes).🤖 Generated with Claude Code