From 63536651af604fddabfd54e78f08eb77bbcf211f Mon Sep 17 00:00:00 2001 From: Michael Flanakin Date: Mon, 17 Aug 2026 00:46:12 -0700 Subject: [PATCH] fix(hubs): relax reservation price backfill gate to recover MCA rows The tmp_MissingPrices gate required a non-blank x_SkuOfferId, an EA-only field that MCA cost and price rows never populate (confirmed against reporter data in #2176). MCA rows were filtered out before the price recovery join even ran, so the Reservations tab in the Rate Optimization report loaded empty for MCA deployments. Drop the offer ID requirement from the gate; leave it in the lookup key unchanged, since it's a no-op suffix for MCA and still differentiates EA rows (e.g. Production vs Dev/Test offers). Fixes #1769 Fixes #2176 Co-Authored-By: Claude Sonnet 5 --- docs-mslearn/toolkit/changelog.md | 3 +- .../HubsReservationPriceBackfill.Tests.ps1 | 77 +++++++++++++++++++ .../assets/ReservationPriceBackfillKey.kql | 72 +++++++++++++++++ .../Analytics/scripts/IngestionSetup_v1_0.kql | 4 +- .../Analytics/scripts/IngestionSetup_v1_2.kql | 4 +- 5 files changed, 155 insertions(+), 5 deletions(-) create mode 100644 src/powershell/Tests/Unit/HubsReservationPriceBackfill.Tests.ps1 create mode 100644 src/powershell/Tests/assets/ReservationPriceBackfillKey.kql diff --git a/docs-mslearn/toolkit/changelog.md b/docs-mslearn/toolkit/changelog.md index 50bba2964..ab10dc73a 100644 --- a/docs-mslearn/toolkit/changelog.md +++ b/docs-mslearn/toolkit/changelog.md @@ -3,7 +3,7 @@ title: FinOps toolkit changelog description: Review the latest features and enhancements in the FinOps toolkit, including updates to FinOps hubs, Power BI reports, and more. author: MSBrett ms.author: brettwil -ms.date: 08/13/2026 +ms.date: 08/17/2026 ms.topic: reference ms.service: finops ms.subservice: finops-toolkit @@ -107,6 +107,7 @@ _Released June 2026_ - Fixed Data Factory ingestion memory pressure during emptiness filtering. - Replaced `isnotempty(strcat(x_SkuMeterId, x_SkuOfferId))` with separate `isnotempty()` checks in FinOps hub ingestion scripts to avoid temporary string allocation. - Hardened the reservation price backfill to select the highest on-demand price (`max()` instead of `min()`) when duplicate price rows collapse under a single reservation price lookup key, preventing understated commitment discount savings ([#2189](https://github.com/microsoft/finops-toolkit/pull/2189)). + - Fixed the Reservations tab in the Rate Optimization report showing no data for Microsoft Customer Agreement (MCA) deployments. The reservation price backfill gate required a non-blank offer ID, but MCA cost and price rows never populate an offer ID, so MCA rows never attempted the price recovery join ([#1769](https://github.com/microsoft/finops-toolkit/issues/1769), [#2176](https://github.com/microsoft/finops-toolkit/issues/2176)). ### [Power BI reports](power-bi/reports.md) v15 diff --git a/src/powershell/Tests/Unit/HubsReservationPriceBackfill.Tests.ps1 b/src/powershell/Tests/Unit/HubsReservationPriceBackfill.Tests.ps1 new file mode 100644 index 000000000..ae95df7f9 --- /dev/null +++ b/src/powershell/Tests/Unit/HubsReservationPriceBackfill.Tests.ps1 @@ -0,0 +1,77 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +<# + Regression coverage for the MCA reservation list-price backfill fix (#1769 / #2176): + the price backfill gate must not require x_SkuOfferId. x_SkuOfferId is EA-only -- for MCA it is blank on + both the cost and price side (confirmed against reporter data in #2176), so gating on it excluded every + MCA row from the price backfill before the join was even attempted. x_SkuOfferId stays in the lookup key + unchanged: for MCA it is blank on both sides (a no-op suffix), and for EA it keeps differentiating rows + (e.g. Production vs Dev/Test offers) exactly as before. + + NOTE: a separate, unconfirmed hypothesis -- that x_BillingProfileId also needs normalizing to match between + the Costs and Prices transforms, because MCA cost rows may carry a full ARM billing-profile path where the + Prices transform normalizes to a bare ID -- is intentionally NOT included here. It wasn't raised or needed + in the reporter-confirmed #2176 investigation, and normalizing away the ARM path may not be the desired + fix (Prices' bare-ID form could itself be a Price Sheet export limitation rather than the canonical shape). + Needs review before changing. + + Per-row behavioral coverage lives in the executable harness + Tests/assets/ReservationPriceBackfillKey.kql (PASS = 0 returned rows on any Kusto database). +#> + +Describe 'HubsReservationPriceBackfill' { + + BeforeDiscovery { + $repoRoot = (Resolve-Path "$PSScriptRoot/../../../..").Path + $scriptsPath = Join-Path $repoRoot 'src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts' + $ingestionFiles = @('IngestionSetup_v1_0.kql', 'IngestionSetup_v1_2.kql') | ForEach-Object { + @{ Name = $_; FullName = (Join-Path $scriptsPath $_) } + } + } + + BeforeAll { + $repoRoot = (Resolve-Path "$PSScriptRoot/../../../..").Path + $harnessPath = Join-Path $repoRoot 'src/powershell/Tests/assets/ReservationPriceBackfillKey.kql' + } + + Context 'Missing-price gate' { + + It 'Should not require x_SkuOfferId to attempt the backfill: ' -ForEach $ingestionFiles { + $content = Get-Content -Path $FullName -Raw + $content.Contains('isnotempty(x_SkuMeterId) and isnotempty(x_SkuOfferId)') | Should -BeFalse -Because 'x_SkuOfferId is EA-only and blank for every MCA row (cost and price side alike, confirmed in #2176); gating on it excludes every MCA row from the price backfill before the join is even attempted (#1769, #2176)' + } + + It 'Should still require x_SkuMeterId to attempt the backfill: ' -ForEach $ingestionFiles { + $content = Get-Content -Path $FullName -Raw + $content.Contains('isnotempty(x_SkuMeterId)') | Should -BeTrue -Because 'the meter ID is still required to identify the on-demand price to recover' + } + + It 'Should leave x_SkuOfferId in the reservation price lookup key: ' -ForEach $ingestionFiles { + $content = Get-Content -Path $FullName -Raw + $content.Contains('x_SkuMeterId, x_SkuOfferId))') | Should -BeTrue -Because 'the key is unchanged by this fix: for MCA x_SkuOfferId is blank on both sides (a no-op suffix), and for EA it keeps differentiating rows (e.g. Production vs Dev/Test offers) as before' + } + } + + Context 'Equivalence harness' { + + It 'Should have the reservation price backfill key harness asset' { + Test-Path $harnessPath | Should -BeTrue + } + + It 'Should assert expected divergence per row' { + $harness = Get-Content -Path $harnessPath -Raw + $harness | Should -Match '\| where \(old_matches != new_matches\) != expectedDivergent' -Because 'the harness must return only rows that violate the recorded old-vs-new expectation' + } + + It 'Should cover an MCA blank offer ID case' { + $harness = Get-Content -Path $harnessPath -Raw + $harness | Should -Match 'MCA' -Because 'the fixture set must exercise the blank-offer-ID case that silently broke MCA reservation pricing' + } + + It 'Should cover an EA offer ID differentiation case for regression safety' { + $harness = Get-Content -Path $harnessPath -Raw + $harness | Should -Match 'EA' -Because 'EA already matched before the fix; the fixture set must prove the fix does not regress it, including the case where offer ID legitimately differentiates two rows' + } + } +} diff --git a/src/powershell/Tests/assets/ReservationPriceBackfillKey.kql b/src/powershell/Tests/assets/ReservationPriceBackfillKey.kql new file mode 100644 index 000000000..bd3417d98 --- /dev/null +++ b/src/powershell/Tests/assets/ReservationPriceBackfillKey.kql @@ -0,0 +1,72 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//====================================================================================================================== +// Reservation price backfill gate regression harness (#1769 / #2176) +// +// Verifies, per row, whether a cost row backfills its list/contracted price from the Prices table, before and after +// the fix. A row backfills only if (a) the missing-price gate lets it attempt the join, and (b) its +// tmp_ReservationPriceLookupKey matches a price row's key: +// +// old gate: isnotempty(x_SkuMeterId) and isnotempty(x_SkuOfferId) +// new gate: isnotempty(x_SkuMeterId) -- x_SkuOfferId requirement dropped +// +// key (unchanged by this fix): tolower(strcat(x_BillingProfileId, month, x_SkuMeterId, x_SkuOfferId)) +// +// Confirmed against reporter data in #2176: for MCA, x_SkuOfferId is blank on BOTH the cost row (FOCUS +// pass-through) and the price row (Price Sheet pass-through) -- it was never a key-matching problem, only a +// gate problem. Relaxing the gate lets the (still offer-ID-suffixed) key match, because the blank offer ID is +// an identical no-op suffix on both sides. For EA, x_SkuOfferId stays populated and continues to differentiate +// rows sharing a profile/meter (e.g. Production vs Dev/Test offers) exactly as before. +// +// NOTE: this harness intentionally does not model x_BillingProfileId format (ARM path vs bare ID). Whether the +// Costs and Prices transforms need to agree on that format is a separate, unconfirmed question -- see the test +// file docstring. +// +// How to run: paste into any Kusto database (ADX or Fabric eventhouse; no table access required). +// - PASS = the query returns 0 rows. +// +// expectedDivergent = true rows are the intended behavior change: MCA rows that now backfill after the fix. +// expectedDivergent = false rows assert everything else is unchanged: EA rows backfilled before and still do +// (including where offer ID legitimately differentiates two rows), and rows with no corresponding price +// (different meter/profile) stay unmatched either way. +//====================================================================================================================== + +let cases = datatable( + label:string, + billingProfileId:string, + priceBillingProfileId:string, + meterId:string, + priceMeterId:string, + offerId:string, // cost-side x_SkuOfferId; blank for MCA (mapped from OfferId, which MCA exports never populate) + priceOfferId:string, // price-side x_SkuOfferId; blank for MCA (pass-through from the Price Sheet export) + expectedDivergent:bool +) +[ + // EA: offer ID populated on both sides -- gate passes and key matches before; must still backfill after. + 'EA offer ID present, matching meter', 'ea-profile-1', 'ea-profile-1', 'meter-1', 'meter-1', 'OFFER-EA-1', 'OFFER-EA-1', false, + // EA: same profile and meter, but genuinely different offer IDs (e.g. Production vs Dev/Test) -- the key must + // keep separating these both before and after the fix; confirms leaving x_SkuOfferId in the key still works. + 'EA same profile/meter, different offer (Prod vs Dev/Test)', 'ea-profile-1', 'ea-profile-1', 'meter-1', 'meter-1', 'OFFER-EA-PROD', 'OFFER-EA-DEVTEST', false, + // MCA: offer ID blank on both cost and price rows (confirmed in #2176). Old gate excludes the row outright; + // new gate passes and, since the blank offer ID is an identical no-op suffix on both sides, the key matches. + 'MCA blank offer ID on both sides', 'mca-profile-1', 'mca-profile-1', 'meter-2', 'meter-2', '', '', true, + // Genuinely different meter within the same MCA profile: must stay unmatched under both old and new (gate + // failure under old, key mismatch under new) -- proves the fix does not backfill unrelated rows. + 'MCA different meter, same profile', 'mca-profile-1', 'mca-profile-1', 'meter-2', 'meter-3', '', '', false, + // Genuinely different billing profile: must not match under either, even post-fix. + 'EA different profile entirely', 'ea-profile-1', 'ea-profile-2', 'meter-1', 'meter-1', 'OFFER-EA-1', 'OFFER-EA-1', false, +] +// old: gate requires a non-empty offer ID +| extend old_gate = isnotempty(meterId) and isnotempty(offerId) +| extend old_costKey = tolower(strcat(billingProfileId, meterId, offerId)) +| extend old_priceKey = tolower(strcat(priceBillingProfileId, priceMeterId, priceOfferId)) +| extend old_matches = old_gate and old_costKey == old_priceKey +// new: gate drops the offer ID requirement; key itself is unchanged +| extend new_gate = isnotempty(meterId) +| extend new_costKey = tolower(strcat(billingProfileId, meterId, offerId)) +| extend new_priceKey = tolower(strcat(priceBillingProfileId, priceMeterId, priceOfferId)) +| extend new_matches = new_gate and new_costKey == new_priceKey +| where (old_matches != new_matches) != expectedDivergent +| project label, old_matches, new_matches, expectedDivergent +| order by label asc diff --git a/src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_0.kql b/src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_0.kql index 50401ebb7..9358a2263 100644 --- a/src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_0.kql +++ b/src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_0.kql @@ -403,12 +403,12 @@ Costs_transform_v1_0() ConsumedQuantity ) // - // Populate missing prices -- mapping to on-demand prices requires meter ID and offer ID + // Populate missing prices -- mapping to on-demand prices requires meter ID; offer ID is EA-only and not required, so MCA rows (whose offer ID is always blank) can still be recovered | extend tmp_MissingPrices = ProviderName == 'Microsoft' and (ListUnitPrice == 0 or ContractedUnitPrice == 0) and x_EffectiveUnitPrice != 0 and not(CommitmentDiscountCategory == 'Spend' and CommitmentDiscountStatus == 'Unused') - and isnotempty(x_SkuMeterId) and isnotempty(x_SkuOfferId) + and isnotempty(x_SkuMeterId) | as allCosts | where tmp_MissingPrices | extend tmp_ReservationPriceLookupKey = tolower(strcat(x_BillingProfileId, substring(ChargePeriodStart, 0, 7), x_SkuMeterId, x_SkuOfferId)) diff --git a/src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_2.kql b/src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_2.kql index da41cafa4..b53635862 100644 --- a/src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_2.kql +++ b/src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_2.kql @@ -408,12 +408,12 @@ Costs_transform_v1_2() ConsumedQuantity ) // - // Populate missing prices -- mapping to on-demand prices requires meter ID and offer ID + // Populate missing prices -- mapping to on-demand prices requires meter ID; offer ID is EA-only and not required, so MCA rows (whose offer ID is always blank) can still be recovered | extend tmp_MissingPrices = ProviderName == 'Microsoft' and (isempty(ListUnitPrice) or isempty(ContractedUnitPrice) or ListUnitPrice == 0 or ContractedUnitPrice == 0) and x_EffectiveUnitPrice != 0 and not(CommitmentDiscountCategory == 'Spend' and CommitmentDiscountStatus == 'Unused') - and isnotempty(x_SkuMeterId) and isnotempty(x_SkuOfferId) + and isnotempty(x_SkuMeterId) | as allCosts | where tmp_MissingPrices | extend tmp_ReservationPriceLookupKey = tolower(strcat(x_BillingProfileId, substring(ChargePeriodStart, 0, 7), x_SkuMeterId, x_SkuOfferId))