Fix Data Explorer dashboard savings formulas and FX fallback - #2248
Fix Data Explorer dashboard savings formulas and FX fallback#2248Michael Flanakin (flanakin) wants to merge 3 commits into
Conversation
Replaces inline sum(ListCost) - sum(EffectiveCost) style savings calculations in dashboard.json with sum() of the canonical, row-level clamped columns (x_TotalSavings, x_CommitmentDiscountSavings, x_NegotiatedDiscountSavings) across 21 tiles. Raw subtraction of aggregated sums can diverge from the canonical clamped values. Also removes the silent 1:1 exchange-rate fallback (coalesce(x_BillingExchangeRate, real(1))) used when deriving CommitmentDiscountQuantity for spend-based commitments in HubSetup_v1_2.kql and IngestionSetup_v1_2.kql. When the rate is missing, the quantity (and any utilization built on it) is now left unset rather than silently distorted by the true FX factor. Addresses the remaining Phase 1.3 and savings-plan FX distortion items from #2093; BillingCurrency visibility in detail tables and the Phase 2 USD-normalization toggle remain out of scope.
Roland Krummenacher (RolandKrummenacher)
left a comment
There was a problem hiding this comment.
Reviewed the refactor and validated the assumptions by running the before/after formulas against three production hubs I have read access to (read-only queries, no identifying details below). The mechanical KQL refactor is clean, but the behavior change is materially larger than the description suggests, and it breaks the tiles that render savings as a visible equation.
Summary
The core issue is that x_TotalSavings, x_CommitmentDiscountSavings, and x_NegotiatedDiscountSavings are clamped per row. Summing them is therefore not equivalent to subtracting the aggregates — it is a different metric, one that structurally cannot net out credits, refunds, and corrections. On one hub the last-3-months total savings comes out ~40% higher after this change; on another the divergence is ~0.04%. So the blast radius is entirely tenant-dependent, which makes shipping it silently riskier, not safer.
That may well be the metric we want. But it is a product decision that belongs in #2093 with a release note, not a side effect of a "use the canonical column" refactor.
Details in the inline comments. Ordered by severity:
- Savings-breakdown tiles now display arithmetic that does not add up — operands stayed raw, results became clamped.
- Parts no longer sum to the whole — negotiated + commitment overstates total by ~61% on one hub.
- Effective savings rate silently inflated — clamped numerator over raw denominator.
- The FX guard likely regresses non-Microsoft FOCUS ingestion — and is a no-op on every Azure hub I checked.
- The auditability claim in the description only holds for one of the two code paths.
What is right
- The refactor itself is correct KQL. The union/pivot tiles properly re-sum the already-renamed columns in the second
summarize. - Dropping the redundant local
extendre-derivations in3b3f0a58and0341b3e4is a genuine simplification — the latter's were entirely dead code. - Removing now-unused
ListCost/ContractedCostfromsummarizelists (e.g.2d7b6447) is a real efficiency win on wide scans. - Leaving the Summary of list, contracted, and effective cost alignment diagnostic on raw unclamped deltas is the right call, and the reasoning in the description is sound.
Two things not introduced here, but now load-bearing
The two clamp formulas disagree. HubSetup_v1_2.kql:169-171 uses iff(ContractedCost < EffectiveCost, real(0), ...); IngestionSetup_v1_2.kql:706-708 uses iff(isempty(ListCost) or ListCost == 0 or ListCost - EffectiveCost < 0.0001, real(0), ...). Pre-existing, but this PR makes 21 tiles depend on those columns, so a hub with mixed v1.0/v1.2 data will now get two different clamping semantics inside a single displayed number. Worth aligning while we are here.
Dashboard/hub version coupling. I verified via getschema that a hub running a current released version does not yet expose the x_*Savings columns on Costs_v1_2() — they come from this branch. Fine for a normal template deploy, but importing the new dashboard.json against a not-yet-upgraded hub now produces hard query failures on all 21 tiles rather than degraded output. Probably worth a release-note line.
Recommendation
Blocking on 1-3: either keep raw subtraction in the tiles that render savings as a visible equation, or derive the displayed operands from the clamped relationship so the math closes. 4 should be narrowed to a currency-equality guard before merge. 5 is a follow-up.
Happy to take the tile fixes if that is useful.
| { | ||
| "dataSource": { "kind": "inline", "dataSourceId": "23540be2-ffc9-4b61-8c4c-05e493e682a6" }, | ||
| "text": "let data = materialize(\n CostsByMonth\n //\n // Don't double-count commitment discount purchases\n | where x_AmortizationClass != 'Principal'\n //\n | summarize \n ListCost = sum(ListCost),\n ContractedCost = sum(ContractedCost),\n EffectiveCost = sum(EffectiveCost)\n | extend CommitmentDiscountSavings = ContractedCost - EffectiveCost\n | extend NegotiatedDiscountSavings = ListCost - ContractedCost\n | extend TotalSavings = ListCost - EffectiveCost\n | project json = todynamic(strcat('[',\n '{ \"order\":11, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":12, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":13, \"type\":\"Contracted\", \"label\":\"After negotiated discounts\", \"value\":\"', numberstring(round(ContractedCost, 2)), '\" },',\n '{ \"order\":14, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":15, \"type\":\"PartialSavings\", \"label\":\"Negotiated savings\", \"value\":\"', numberstring(round(NegotiatedDiscountSavings, 2)), '\" },',\n //\n '{ \"order\":21, \"type\":\"Contracted\", \"label\":\"After negotiated discounts\", \"value\":\"', numberstring(round(ContractedCost, 2)), '\" },',\n '{ \"order\":22, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":23, \"type\":\"Effective\", \"label\":\"After commitment discounts\", \"value\":\"', numberstring(round(EffectiveCost, 2)), '\" },',\n '{ \"order\":24, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":25, \"type\":\"PartialSavings\", \"label\":\"Commitment savings\", \"value\":\"', numberstring(round(CommitmentDiscountSavings, 2)), '\" },',\n //\n '{ \"order\":31, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":32, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":33, \"type\":\"Effective\", \"label\":\"After commitment discounts\", \"value\":\"', numberstring(round(EffectiveCost, 2)), '\" },',\n '{ \"order\":34, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":35, \"type\":\"TotalSavings\", \"label\":\"Total savings\", \"value\":\"', numberstring(round(TotalSavings, 2)), '\" }',\n ']'))\n | mv-expand json\n | order by toint(json.order) asc\n | project Label = tostring(json.label), Value = tostring(json.value), Type = tostring(json.type)\n);\ndata", | ||
| "text": "let data = materialize(\n CostsByMonth\n //\n // Don't double-count commitment discount purchases\n | where x_AmortizationClass != 'Principal'\n //\n | summarize \n ListCost = sum(ListCost),\n ContractedCost = sum(ContractedCost),\n EffectiveCost = sum(EffectiveCost),\n CommitmentDiscountSavings = sum(x_CommitmentDiscountSavings),\n NegotiatedDiscountSavings = sum(x_NegotiatedDiscountSavings),\n TotalSavings = sum(x_TotalSavings)\n | project json = todynamic(strcat('[',\n '{ \"order\":11, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":12, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":13, \"type\":\"Contracted\", \"label\":\"After negotiated discounts\", \"value\":\"', numberstring(round(ContractedCost, 2)), '\" },',\n '{ \"order\":14, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":15, \"type\":\"PartialSavings\", \"label\":\"Negotiated savings\", \"value\":\"', numberstring(round(NegotiatedDiscountSavings, 2)), '\" },',\n //\n '{ \"order\":21, \"type\":\"Contracted\", \"label\":\"After negotiated discounts\", \"value\":\"', numberstring(round(ContractedCost, 2)), '\" },',\n '{ \"order\":22, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":23, \"type\":\"Effective\", \"label\":\"After commitment discounts\", \"value\":\"', numberstring(round(EffectiveCost, 2)), '\" },',\n '{ \"order\":24, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":25, \"type\":\"PartialSavings\", \"label\":\"Commitment savings\", \"value\":\"', numberstring(round(CommitmentDiscountSavings, 2)), '\" },',\n //\n '{ \"order\":31, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":32, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":33, \"type\":\"Effective\", \"label\":\"After commitment discounts\", \"value\":\"', numberstring(round(EffectiveCost, 2)), '\" },',\n '{ \"order\":34, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":35, \"type\":\"TotalSavings\", \"label\":\"Total savings\", \"value\":\"', numberstring(round(TotalSavings, 2)), '\" }',\n ']'))\n | mv-expand json\n | order by toint(json.order) asc\n | project Label = tostring(json.label), Value = tostring(json.value), Type = tostring(json.type)\n);\ndata", |
There was a problem hiding this comment.
Blocking — the tile now renders arithmetic that does not add up.
This tile (e17346d1) and its CostsByDay twin (f1ef29df) literally display three equations:
Cost without discounts - After negotiated discounts = Negotiated savings
After negotiated disc. - After commitment discounts = Commitment savings
Cost without discounts - After commitment discounts = Total savings
The change leaves the operands as raw sum(ListCost) / sum(ContractedCost) / sum(EffectiveCost) but swaps the results to clamped sum(x_*Savings). Those disagree.
Measured on a production hub, last 3 months, applying this tile's own x_AmortizationClass != 'Principal' filter:
| Row | vs. the subtraction shown directly above it |
|---|---|
| Negotiated savings | +25.9% |
| Commitment savings | +53.8% |
| Total savings | +40.2% |
So the tile renders something like 2,283,331 - 2,056,289 = 285,782. That is not a rounding artifact — a user will read the two numbers above the result and see it is wrong.
Either keep the raw subtraction here (this tile's whole purpose is showing the derivation), or also derive the displayed ListCost/ContractedCost/EffectiveCost from the clamped relationship so the equation closes.
There was a problem hiding this comment.
Fixed by deriving the displayed operands from the same clamped, row-level basis as the results, rather than reverting to raw subtraction (which would have silently reintroduced the original clamping bug this PR set out to fix for this tile).
EffectiveCost (raw sum) is the only value that's never negative-clamped, so it anchors the chain:
ContractedCost' = EffectiveCost + sum(x_CommitmentDiscountSavings)ListCost' = ContractedCost' + sum(x_NegotiatedDiscountSavings)TotalSavings' = sum(x_NegotiatedDiscountSavings) + sum(x_CommitmentDiscountSavings)(sum of the two clamped components, not the independently-clampedx_TotalSavings)
All three displayed equations now hold exactly: List' - Contracted' = Negotiated, Contracted' - Effective = Commitment, List' - Effective = TotalSavings'. Verified against your counterexample rows (100,120,80) and (100,80,60): Negotiated=20, Commitment=60, Effective=140 → Contracted'=200 → List'=220; 220-200=20 ✓, 200-140=60 ✓, 220-140=80=TotalSavings' ✓.
The other tiles' clamped-sum fix is unaffected by this change.
| { | ||
| "dataSource": { "kind": "inline", "dataSourceId": "23540be2-ffc9-4b61-8c4c-05e493e682a6" }, | ||
| "text": "let data = materialize(\n CostsByDay\n //\n // Don't double-count commitment discount purchases\n | where ChargeCategory == 'Usage' or isempty(CommitmentDiscountId)\n //\n | summarize \n ListCost = sum(ListCost),\n ContractedCost = sum(ContractedCost),\n EffectiveCost = sum(EffectiveCost)\n | extend CommitmentDiscountSavings = ContractedCost - EffectiveCost\n | extend NegotiatedDiscountSavings = ListCost - ContractedCost\n | extend TotalSavings = ListCost - EffectiveCost\n | project json = todynamic(strcat('[',\n '{ \"order\":11, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":12, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":13, \"type\":\"Contracted\", \"label\":\"After negotiated discounts\", \"value\":\"', numberstring(round(ContractedCost, 2)), '\" },',\n '{ \"order\":14, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":15, \"type\":\"PartialSavings\", \"label\":\"Negotiated savings\", \"value\":\"', numberstring(round(NegotiatedDiscountSavings, 2)), '\" },',\n //\n '{ \"order\":21, \"type\":\"Contracted\", \"label\":\"After negotiated discounts\", \"value\":\"', numberstring(round(ContractedCost, 2)), '\" },',\n '{ \"order\":22, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":23, \"type\":\"Effective\", \"label\":\"After commitment discounts\", \"value\":\"', numberstring(round(EffectiveCost, 2)), '\" },',\n '{ \"order\":24, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":25, \"type\":\"PartialSavings\", \"label\":\"Commitment savings\", \"value\":\"', numberstring(round(CommitmentDiscountSavings, 2)), '\" },',\n //\n '{ \"order\":31, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":32, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":33, \"type\":\"Effective\", \"label\":\"After commitment discounts\", \"value\":\"', numberstring(round(EffectiveCost, 2)), '\" },',\n '{ \"order\":34, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":35, \"type\":\"TotalSavings\", \"label\":\"Total savings\", \"value\":\"', numberstring(round(TotalSavings, 2)), '\" }',\n ']'))\n | mv-expand json\n | order by toint(json.order) asc\n | project Label = tostring(json.label), Value = tostring(json.value), Type = tostring(json.type)\n);\ndata", | ||
| "text": "let data = materialize(\n CostsByDay\n //\n // Don't double-count commitment discount purchases\n | where ChargeCategory == 'Usage' or isempty(CommitmentDiscountId)\n //\n | summarize \n ListCost = sum(ListCost),\n ContractedCost = sum(ContractedCost),\n EffectiveCost = sum(EffectiveCost),\n CommitmentDiscountSavings = sum(x_CommitmentDiscountSavings),\n NegotiatedDiscountSavings = sum(x_NegotiatedDiscountSavings),\n TotalSavings = sum(x_TotalSavings)\n | project json = todynamic(strcat('[',\n '{ \"order\":11, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":12, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":13, \"type\":\"Contracted\", \"label\":\"After negotiated discounts\", \"value\":\"', numberstring(round(ContractedCost, 2)), '\" },',\n '{ \"order\":14, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":15, \"type\":\"PartialSavings\", \"label\":\"Negotiated savings\", \"value\":\"', numberstring(round(NegotiatedDiscountSavings, 2)), '\" },',\n //\n '{ \"order\":21, \"type\":\"Contracted\", \"label\":\"After negotiated discounts\", \"value\":\"', numberstring(round(ContractedCost, 2)), '\" },',\n '{ \"order\":22, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":23, \"type\":\"Effective\", \"label\":\"After commitment discounts\", \"value\":\"', numberstring(round(EffectiveCost, 2)), '\" },',\n '{ \"order\":24, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":25, \"type\":\"PartialSavings\", \"label\":\"Commitment savings\", \"value\":\"', numberstring(round(CommitmentDiscountSavings, 2)), '\" },',\n //\n '{ \"order\":31, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":32, \"type\":\"\", \"label\":\"\", \"value\":\"➖\" },',\n '{ \"order\":33, \"type\":\"Effective\", \"label\":\"After commitment discounts\", \"value\":\"', numberstring(round(EffectiveCost, 2)), '\" },',\n '{ \"order\":34, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":35, \"type\":\"TotalSavings\", \"label\":\"Total savings\", \"value\":\"', numberstring(round(TotalSavings, 2)), '\" }',\n ']'))\n | mv-expand json\n | order by toint(json.order) asc\n | project Label = tostring(json.label), Value = tostring(json.value), Type = tostring(json.type)\n);\ndata", |
There was a problem hiding this comment.
Same defect as e17346d1 above — this is the CostsByDay twin of the savings-breakdown tile, with the same three rendered equations whose operands stayed raw while the results became clamped. Whatever fix lands on the monthly tile needs to land here too.
There was a problem hiding this comment.
Fixed by deriving the displayed operands from the same clamped, row-level basis as the results, rather than reverting to raw subtraction (which would have silently reintroduced the original clamping bug this PR set out to fix for this tile).
EffectiveCost (raw sum) is the only value that's never negative-clamped, so it anchors the chain:
ContractedCost' = EffectiveCost + sum(x_CommitmentDiscountSavings)ListCost' = ContractedCost' + sum(x_NegotiatedDiscountSavings)TotalSavings' = sum(x_NegotiatedDiscountSavings) + sum(x_CommitmentDiscountSavings)(sum of the two clamped components, not the independently-clampedx_TotalSavings)
All three displayed equations now hold exactly: List' - Contracted' = Negotiated, Contracted' - Effective = Commitment, List' - Effective = TotalSavings'. Verified against your counterexample rows (100,120,80) and (100,80,60): Negotiated=20, Commitment=60, Effective=140 → Contracted'=200 → List'=220; 220-200=20 ✓, 200-140=60 ✓, 220-140=80=TotalSavings' ✓.
The other tiles' clamped-sum fix is unaffected by this change.
| { | ||
| "dataSource": { "kind": "inline", "dataSourceId": "23540be2-ffc9-4b61-8c4c-05e493e682a6" }, | ||
| "text": "let data = materialize(\n CostsByMonth | extend Period = 'Last n months'\n | union (CostsByDay | extend Period = 'Last n days')\n | summarize\n ListCost = round(sum(ListCost), 2),\n ContractedCost = round(sum(ContractedCost), 2),\n EffectiveCost = round(sum(EffectiveCost), 2)\n by\n Period\n | project Period, json = todynamic(strcat('[{ \"Label\":\"Total\", \"Value\":', ListCost - EffectiveCost, ' }, { \"Label\":\"Negotiated\", \"Value\":', ListCost - ContractedCost, ' }, { \"Label\":\"Commitment\", \"Value\":', ContractedCost - EffectiveCost, ' }]'))\n | mv-expand json\n | project Label = tostring(json.Label), Value = tolong(json.Value), Period\n);\ndata\n", | ||
| "text": "let data = materialize(\n CostsByMonth | extend Period = 'Last n months'\n | union (CostsByDay | extend Period = 'Last n days')\n | summarize\n TotalSavings = round(sum(x_TotalSavings), 2),\n NegotiatedDiscountSavings = round(sum(x_NegotiatedDiscountSavings), 2),\n CommitmentDiscountSavings = round(sum(x_CommitmentDiscountSavings), 2)\n by\n Period\n | project Period, json = todynamic(strcat('[{ \"Label\":\"Total\", \"Value\":', TotalSavings, ' }, { \"Label\":\"Negotiated\", \"Value\":', NegotiatedDiscountSavings, ' }, { \"Label\":\"Commitment\", \"Value\":', CommitmentDiscountSavings, ' }]'))\n | mv-expand json\n | project Label = tostring(json.Label), Value = tolong(json.Value), Period\n);\ndata\n", |
There was a problem hiding this comment.
Blocking — the parts no longer sum to the whole.
This tile shows Total / Negotiated / Commitment side by side. Because each x_*Savings column clamps independently at row level, x_NegotiatedDiscountSavings + x_CommitmentDiscountSavings != x_TotalSavings.
On a production hub across the full dataset, negotiated + commitment came out ~61% larger than total. Previously the three were consistent by construction, since all came from the same aggregates.
A user looking at this tile will see two components that visibly exceed the total they are supposed to decompose.
There was a problem hiding this comment.
Fixed. This tile doesn't display List/Contracted/Effective operands, only the three savings bars (Total, Negotiated, Commitment), so the fix here is narrower than the waterfall tiles: TotalSavings is now derived as sum(x_NegotiatedDiscountSavings) + sum(x_CommitmentDiscountSavings) instead of the independently-clamped sum(x_TotalSavings), so the Total bar always equals Negotiated + Commitment shown alongside it — the parts sum to the whole again, by construction.
Verified against your counterexample rows: Negotiated=20, Commitment=60 → Total'=80, which is internally consistent (no longer the independently-clamped 60 that didn't match 20+60=80).
The other tiles' clamped-sum fix is unaffected by this change.
| { | ||
| "dataSource": { "kind": "inline", "dataSourceId": "23540be2-ffc9-4b61-8c4c-05e493e682a6" }, | ||
| "text": "let monthname = dynamic(['', 'January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December']);\nCostsPlus\n| where startofmonth(ChargePeriodStart) >= startofmonth(now(), -1)\n| summarize \n EffectiveCost = sum(EffectiveCost),\n ContractedCost = sum(ContractedCost),\n ListCost = sum(ListCost)\n by\n ChargePeriodStart,\n Month = strcat(format_datetime(ChargePeriodStart, 'MM '), monthname[monthofyear(ChargePeriodStart)])\n| extend CommitmentDiscountSavings = ContractedCost - EffectiveCost\n| extend NegotiatedDiscountSavings = ListCost - ContractedCost\n| order by ChargePeriodStart asc\n| extend EffectiveCostRunningTotal = row_cumsum(EffectiveCost, prev(Month) != Month)\n| extend CommitmentDiscountSavingsRunningTotal = row_cumsum(CommitmentDiscountSavings, prev(Month) != Month)\n| extend NegotiatedDiscountSavingsRunningTotal = row_cumsum(NegotiatedDiscountSavings, prev(Month) != Month)\n| project ChargePeriodStart, CommitmentDiscountSavingsRunningTotal, NegotiatedDiscountSavingsRunningTotal, EffectiveCostRunningTotal, Month\n| render areachart ", | ||
| "text": "let monthname = dynamic(['', 'January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December']);\nCostsPlus\n| where startofmonth(ChargePeriodStart) >= startofmonth(now(), -1)\n| summarize \n EffectiveCost = sum(EffectiveCost),\n CommitmentDiscountSavings = sum(x_CommitmentDiscountSavings),\n NegotiatedDiscountSavings = sum(x_NegotiatedDiscountSavings)\n by\n ChargePeriodStart,\n Month = strcat(format_datetime(ChargePeriodStart, 'MM '), monthname[monthofyear(ChargePeriodStart)])\n| order by ChargePeriodStart asc\n| extend EffectiveCostRunningTotal = row_cumsum(EffectiveCost, prev(Month) != Month)\n| extend CommitmentDiscountSavingsRunningTotal = row_cumsum(CommitmentDiscountSavings, prev(Month) != Month)\n| extend NegotiatedDiscountSavingsRunningTotal = row_cumsum(NegotiatedDiscountSavings, prev(Month) != Month)\n| project ChargePeriodStart, CommitmentDiscountSavingsRunningTotal, NegotiatedDiscountSavingsRunningTotal, EffectiveCostRunningTotal, Month\n| render areachart ", |
There was a problem hiding this comment.
Related to the parts-vs-whole problem: this area chart stacks EffectiveCost + CommitmentDiscountSavings + NegotiatedDiscountSavings. Previously that stack summed to exactly ListCost by construction, so the chart height was meaningful. With independently clamped columns it no longer does, and the stacked total will overshoot list cost by a tenant-dependent amount.
There was a problem hiding this comment.
Looked at this one closely: the stack itself (EffectiveCost base layer + sum(x_CommitmentDiscountSavings) + sum(x_NegotiatedDiscountSavings), both already clamped sums per this PR) reconciles internally by construction — a stacked area chart's top is always the sum of its layers, so the stack top already equals EffectiveCost + Commitment + Negotiated, the same derived "cost without discounts" quantity used in the waterfall/ESR tiles after their fix. No formula change was needed here.
What I did change: added a comment in the query clarifying that the stack top is this derived figure, not the raw sum(ListCost) KPI shown elsewhere on the page — so nobody cross-checking the stack height against a separate raw List cost tile is confused by a legitimate difference between the two (your 220 vs 200 example). That raw/derived distinction is now called out explicitly at the top of the query.
The other tiles' clamped-sum fix is unaffected by this change.
| { | ||
| "dataSource": { "kind": "inline", "dataSourceId": "23540be2-ffc9-4b61-8c4c-05e493e682a6" }, | ||
| "text": "let data = materialize(\n CostsByMonth\n //\n // Don't double-count commitment discount purchases\n | where x_AmortizationClass != 'Principal'\n //\n | summarize \n ListCost = sum(ListCost),\n ContractedCost = sum(ContractedCost),\n EffectiveCost = sum(EffectiveCost)\n | extend TotalSavings = ListCost - EffectiveCost\n | extend EffectiveSavingsRate = TotalSavings / ListCost\n | project json = todynamic(strcat('[',\n '{ \"order\":11, \"type\":\"TotalSavings\", \"label\":\"Total savings\", \"value\":\"', numberstring(round(TotalSavings, 2)), '\" },',\n '{ \"order\":12, \"type\":\"\", \"label\":\"\", \"value\":\"➗\" },',\n '{ \"order\":13, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":14, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":15, \"type\":\"EffectiveSavingsRate\", \"label\":\"Effective savings rate\", \"value\":\"', percentstring(EffectiveSavingsRate), '\" }',\n ']'))\n | mv-expand json\n | order by toint(json.order) asc\n | project Label = tostring(json.label), Value = tostring(json.value), Type = tostring(json.type)\n);\ndata", | ||
| "text": "let data = materialize(\n CostsByMonth\n //\n // Don't double-count commitment discount purchases\n | where x_AmortizationClass != 'Principal'\n //\n | summarize \n ListCost = sum(ListCost),\n TotalSavings = sum(x_TotalSavings)\n | extend EffectiveSavingsRate = TotalSavings / ListCost\n | project json = todynamic(strcat('[',\n '{ \"order\":11, \"type\":\"TotalSavings\", \"label\":\"Total savings\", \"value\":\"', numberstring(round(TotalSavings, 2)), '\" },',\n '{ \"order\":12, \"type\":\"\", \"label\":\"\", \"value\":\"➗\" },',\n '{ \"order\":13, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":14, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":15, \"type\":\"EffectiveSavingsRate\", \"label\":\"Effective savings rate\", \"value\":\"', percentstring(EffectiveSavingsRate), '\" }',\n ']'))\n | mv-expand json\n | order by toint(json.order) asc\n | project Label = tostring(json.label), Value = tostring(json.value), Type = tostring(json.type)\n);\ndata", |
There was a problem hiding this comment.
Effective savings rate is silently inflated here.
EffectiveSavingsRate = sum(x_TotalSavings) / sum(ListCost) now mixes a clamped numerator with a raw denominator, so the ratio is no longer internally consistent.
On a production hub, last 3 months, the reported ESR rose by roughly 40% relative to the pre-change value. Broken out by month the instability is worse — one month moved +230% because a single large negative-savings correction got clamped to zero instead of netting out, while adjacent months moved 11-17%.
ESR is a headline FinOps KPI. A step change of that size with no release note will read as a regression to customers, and the month-to-month series becomes non-comparable across the upgrade boundary.
There was a problem hiding this comment.
Fixed by deriving TotalSavings and ListCost from the same clamped, row-level basis as the other tiles rather than mixing a clamped numerator with a raw denominator:
EffectiveCost= raw sum (never negative-clamped)TotalSavings' = sum(x_NegotiatedDiscountSavings) + sum(x_CommitmentDiscountSavings)ListCost' = EffectiveCost + TotalSavings'EffectiveSavingsRate = TotalSavings' / ListCost'
This is a genuine ESR definition change, as you and MSBrett both flagged — the numerator and denominator are now internally consistent with each other and with the other savings tiles on the dashboard, but the resulting rate will differ from the pre-PR raw-subtraction ESR whenever any row was clamped, so it isn't just a formula cleanup. Added a release note under FinOps hubs v15 in docs-mslearn/toolkit/changelog.md calling out the definition change explicitly, per your ask.
Verified against the counterexample: Effective=140, TotalSavings'=80 → ListCost'=220 → ESR = 80/220 ≈ 36.4%, self-consistent with the displayed operands.
| { | ||
| "dataSource": { "kind": "inline", "dataSourceId": "23540be2-ffc9-4b61-8c4c-05e493e682a6" }, | ||
| "text": "let data = materialize(\n CostsByDay\n //\n // Don't double-count commitment discount purchases\n | where x_AmortizationClass != 'Principal'\n //\n | summarize \n ListCost = sum(ListCost),\n ContractedCost = sum(ContractedCost),\n EffectiveCost = sum(EffectiveCost)\n | extend TotalSavings = ListCost - EffectiveCost\n | extend EffectiveSavingsRate = TotalSavings / ListCost\n | project json = todynamic(strcat('[',\n '{ \"order\":11, \"type\":\"TotalSavings\", \"label\":\"Total savings\", \"value\":\"', numberstring(round(TotalSavings, 2)), '\" },',\n '{ \"order\":12, \"type\":\"\", \"label\":\"\", \"value\":\"➗\" },',\n '{ \"order\":13, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":14, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":15, \"type\":\"EffectiveSavingsRate\", \"label\":\"Effective savings rate\", \"value\":\"', percentstring(EffectiveSavingsRate), '\" }',\n ']'))\n | mv-expand json\n | order by toint(json.order) asc\n | project Label = tostring(json.label), Value = tostring(json.value), Type = tostring(json.type)\n);\ndata", | ||
| "text": "let data = materialize(\n CostsByDay\n //\n // Don't double-count commitment discount purchases\n | where x_AmortizationClass != 'Principal'\n //\n | summarize \n ListCost = sum(ListCost),\n TotalSavings = sum(x_TotalSavings)\n | extend EffectiveSavingsRate = TotalSavings / ListCost\n | project json = todynamic(strcat('[',\n '{ \"order\":11, \"type\":\"TotalSavings\", \"label\":\"Total savings\", \"value\":\"', numberstring(round(TotalSavings, 2)), '\" },',\n '{ \"order\":12, \"type\":\"\", \"label\":\"\", \"value\":\"➗\" },',\n '{ \"order\":13, \"type\":\"List\", \"label\":\"Cost without discounts\", \"value\":\"', numberstring(round(ListCost, 2)), '\" },',\n '{ \"order\":14, \"type\":\"\", \"label\":\"\", \"value\":\"🟰\" },',\n '{ \"order\":15, \"type\":\"EffectiveSavingsRate\", \"label\":\"Effective savings rate\", \"value\":\"', percentstring(EffectiveSavingsRate), '\" }',\n ']'))\n | mv-expand json\n | order by toint(json.order) asc\n | project Label = tostring(json.label), Value = tostring(json.value), Type = tostring(json.type)\n);\ndata", |
There was a problem hiding this comment.
Same ESR issue as e1bc2d51 — clamped numerator over raw denominator, on the CostsByDay variant.
There was a problem hiding this comment.
Fixed by deriving TotalSavings and ListCost from the same clamped, row-level basis as the other tiles rather than mixing a clamped numerator with a raw denominator:
EffectiveCost= raw sum (never negative-clamped)TotalSavings' = sum(x_NegotiatedDiscountSavings) + sum(x_CommitmentDiscountSavings)ListCost' = EffectiveCost + TotalSavings'EffectiveSavingsRate = TotalSavings' / ListCost'
This is a genuine ESR definition change, as you and MSBrett both flagged — the numerator and denominator are now internally consistent with each other and with the other savings tiles on the dashboard, but the resulting rate will differ from the pre-PR raw-subtraction ESR whenever any row was clamped, so it isn't just a formula cleanup. Added a release note under FinOps hubs v15 in docs-mslearn/toolkit/changelog.md calling out the definition change explicitly, per your ask.
Verified against the counterexample: Effective=140, TotalSavings'=80 → ListCost'=220 → ESR = 80/220 ≈ 36.4%, self-consistent with the displayed operands.
The edited branch is only reached by FOCUS-sourced rows where a null x_BillingExchangeRate means single-currency billing (PricingCurrency == BillingCurrency, so Cost Management doesn't populate a rate) -- not the EA/legacy null-source scenario the change was justified against. Those EA/legacy paths also null CommitmentDiscountQuantity upstream, so they short-circuit at the first case() branch and never reach this one. For single-currency accounts, a rate of 1.0 is correct, not a guess. Requiring isnotempty(x_BillingExchangeRate) instead silently drops these rows (and their CommitmentDiscountUnit) from utilization aggregates -- trading a bounded, visible number for a silent hole in sum()-based denominators. Restoring coalesce(x_BillingExchangeRate, real(1)). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Roland Krummenacher (RolandKrummenacher)
left a comment
There was a problem hiding this comment.
Re-reviewed at 0e05f98. The KQL revert resolves the two threads it touches — replied there, no objection. The PR is now a pure dashboard.json change, +21/-21.
The six dashboard threads are untouched by this commit and all still apply, so keeping changes requested. Consolidating them here, since they're one root cause rather than six independent problems.
Root cause: the change swaps raw subtractions for sum(x_*Savings) on the result side of each tile, while leaving sum(ListCost) / sum(ContractedCost) / sum(EffectiveCost) raw on the operand side. The x_*Savings columns clamp negatives at row level, so the two sides no longer reconcile.
| Tile | Symptom, measured on a production hub |
|---|---|
Savings breakdown, monthly (e17346d1) + daily (f1ef29df) |
the three equations the tile literally displays don't hold: +25.9%, +53.8%, +40.2% |
| Total / Negotiated / Commitment (line 3074) | components exceed the total they decompose by ~61% |
| Stacked savings area chart (line 2876) | stack no longer sums to ListCost, overshoots by a tenant-dependent amount |
Effective Savings Rate (e1bc2d51 + daily twin) |
clamped numerator over raw denominator: ESR +40% relative, one month +230% |
One decision settles all four: either keep the raw subtractions on these tiles, or derive the displayed cost operands from the clamped columns so the arithmetic closes. I don't have a strong preference as long as each tile is internally consistent — a user reading two numbers and their stated difference shouldn't see a third number that isn't the difference.
ESR is the one I'd treat as separate from the arithmetic question: it's a headline KPI, and a step change of that size across an upgrade boundary makes the month-over-month series non-comparable. If the clamped definition is the one we want, that needs a release note rather than just a formula change.
Happy to re-measure whichever direction you pick.
Brett Wilson (MSBrett)
left a comment
There was a problem hiding this comment.
Requesting changes. I re-reviewed the current head (0e05f98b) from the base diff, traced the savings fields to their source definitions, checked all 21 changed queries, and independently reproduced the arithmetic failures below.
The replacement of aggregate subtraction with sums of row-level clamped savings is not semantics-preserving for visuals that still rely on exact accounting relationships:
-
The monthly and daily savings-summary tiles display false equations (
dashboard.json:3086and:3092). They still show raw aggregate values for List, Contracted, and Effective cost, followed by subtraction and equals signs, but the displayed results now sum independently clamped row values. Whenever a row is clamped, the displayed operands no longer produce the displayed result. -
The Total / Negotiated / Commitment summary is no longer a valid decomposition (
dashboard.json:3074). Independently clamped negotiated and commitment savings are not additive and can sum to more than total savings. -
The stacked Cost + savings chart no longer reconciles to list cost (
dashboard.json:2876). It stacks raw effective cost with both independently clamped savings categories, so the stack can overstate the implied cost without discounts.
A minimal counterexample confirms all three failures: for rows (List, Contracted, Effective) = (100,120,80) and (100,80,60), the raw negotiated difference is 0 while summed clamped negotiated savings is 20; negotiated plus commitment savings is 80 while total savings is 60; and the stacked value is 220 against list cost of 200.
The clamped Effective Savings Rate formula is not inherently invalid—it matches the repository's savings-summary-report.kql precedent—but it is a material KPI-definition change rather than a mechanical refactor. If retained for ESR and standalone savings visuals, the changed semantics and historical discontinuity should be explicitly approved and documented in release notes.
Please either keep aggregate subtraction for visuals that present reconciled equations/components, or redesign those visuals and labels so the independently clamped metrics are not presented as arithmetic identities. Also update the PR title/body now that the FX change has been reverted and remove the claim that the savings portion was verified clean while these blocking threads remain unresolved.
…hboard tiles Roland Krummenacher and MSBrett both flagged that summing clamped, row-level savings columns (x_TotalSavings, x_CommitmentDiscountSavings, x_NegotiatedDiscountSavings) for the result side of 6 tiles, while leaving ListCost/ContractedCost/EffectiveCost as raw aggregates on the operand side, broke the on-screen equations those tiles visually promise (e.g. "List - Contracted = Negotiated savings"). Clamping doesn't distribute over sums, so the two sides no longer reconciled. Rather than reverting to raw subtraction (which would silently reintroduce the original per-row clamping bug for just these tiles), derive the displayed List/Contracted cost operands from the same clamped basis as the results, anchored on EffectiveCost (the only value that's never negative-clamped): ContractedCost' = EffectiveCost + sum(x_CommitmentDiscountSavings) ListCost' = ContractedCost' + sum(x_NegotiatedDiscountSavings) TotalSavings' = sum(x_NegotiatedDiscountSavings) + sum(x_CommitmentDiscountSavings) Applied to: - Savings breakdown waterfall, monthly (e17346d1) and daily (f1ef29df) - Total/Negotiated/Commitment savings bar (f5f240a8) - Cost + savings stacked area chart (dcc1f533) - already reconciled by construction; added a clarifying comment only - Effective Savings Rate, monthly (e1bc2d51) and daily (cb34f22b) This changes the ESR definition on the two ESR tiles, since TotalSavings and ListCost are now derived consistently rather than mixing a clamped numerator with a raw denominator. Added a release note under FinOps hubs v15 in docs-mslearn/toolkit/changelog.md per review feedback. Verified against the counterexample from review: rows (List, Contracted, Effective) = (100,120,80) and (100,80,60) give NegotiatedDiscountSavings=20, CommitmentDiscountSavings=60, Effective=140 -> ContractedCost'=200 -> ListCost'=220; all displayed equations hold exactly. The other ~15 tiles' clamped-sum fix from the original PR is unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Addresses the remaining Phase 1.3 items from #2093 that were left open after PR #2103 shipped the currency-filtering fix:
sum(ListCost) - sum(EffectiveCost)style calculations withsum()of the canonical, row-level clamped columns (x_TotalSavings,x_CommitmentDiscountSavings,x_NegotiatedDiscountSavings) across 21 tiles indashboard.json.Savings-plan utilization FX distortion — removed the silent 1:1 exchange-rate fallback in the data layer.Reverted — see note below.Why
Savings formulas (dashboard.json)
Costs/CostsPlus/CostsByMonth/CostsByDayalready carryx_TotalSavings,x_CommitmentDiscountSavings, andx_NegotiatedDiscountSavingsper row, computed with built-in clamping (e.g.x_NegotiatedDiscountSavings = iff(ListCost < ContractedCost, decimal(0), ListCost - ContractedCost)). Several dashboard tiles instead summed the raw cost columns first and then subtracted (sum(ListCost) - sum(EffectiveCost)), which skips that clamping and can diverge from the canonical values (e.g. when a row's contracted cost temporarily exceeds list cost due to data timing/rounding).Fixed by summing the canonical
x_*Savingscolumns directly wherever a query aggregates overCosts*and then derives savings. Two tiles (Savings breakdown by month,Effective cost breakdown by month) were already locally re-deriving the sameiff()clamp formula redundantly — simplified those to just sum the canonical columns that already exist on the source table, removing the redundantextend.Left unchanged: the Summary of list, contracted, and effective cost alignment data-quality diagnostic table (Data ingestion page). That tile intentionally computes raw, unclamped deltas (
ListCost - ContractedCost, etc.) to detect data-quality anomalies likeListCost too loworContractedCost should be 0— using clamped values there would defeat its purpose.Status: 15 of the 21 tiles were code-reviewed and verified clean as originally shipped — no further changes. The other 6 tiles required a follow-up fix (below) after review feedback.
Follow-up: reconciling on-screen equations on 6 tiles
@rkrummenacher and Brett Wilson (@MSBrett) independently flagged that 6 tiles display an equation or decomposition the user can visually verify on screen (for example, "List − Contracted = Negotiated savings", or Total savings ÷ List cost = Effective Savings Rate). On those tiles, summing the clamped per-row savings columns for the result side while leaving the operand side (
ListCost/ContractedCost/EffectiveCost) as raw aggregates broke that on-screen reconciliation — the two sides of the displayed equation no longer matched, because clamping doesn't distribute over sums (sum(clamp(a-b)) + sum(clamp(b-c))isn't alwayssum(clamp(a-c))).Rather than reverting these 6 tiles to the original raw-subtraction formulas (which would silently reintroduce the original clamping bug for just these tiles), the fix derives the displayed
ListCost/ContractedCostoperands from the same clamped, row-level basis as the results:EffectiveCost(raw sum) is the only unclamped anchor — it's never negative-clamped.ContractedCost' = EffectiveCost + sum(x_CommitmentDiscountSavings)ListCost' = ContractedCost' + sum(x_NegotiatedDiscountSavings)TotalSavings' = sum(x_NegotiatedDiscountSavings) + sum(x_CommitmentDiscountSavings)(the sum of the two already-clamped components, not the independently clampedx_TotalSavings)This makes both sides of each displayed equation reconcile exactly, using clamped math throughout — it does not go back to the raw-subtraction bug the original PR fixed.
Affected tiles:
This changes the Effective Savings Rate (ESR) definition on the two ESR tiles, since ESR's
TotalSavingsandListCostinputs are now derived the same way. A release note has been added under FinOps hubs v15 indocs-mslearn/toolkit/changelog.md.Savings-plan utilization FX distortion (data layer) — reverted
This PR originally also changed
CommitmentDiscountQuantityderivation (HubSetup_v1_2.kql,IngestionSetup_v1_2.kql) to requireisnotempty(x_BillingExchangeRate)before dividing by it, falling through toreal(null)otherwise, to avoid silently assuming a 1:1 rate when the exchange rate was missing.That change has been reverted after a closer look at the actual execution path found the original justification didn't hold:
CommitmentDiscountCategory == 'Spend', EffectiveCost / x_BillingExchangeRate) sits inside acase()that short-circuits earlier — atisnotempty(CommitmentDiscountQuantity)— for rows that already have a quantity, and the EA/legacy raw-source ingestion paths cited as the motivating scenario also null outCommitmentDiscountQuantityupstream, so they short-circuit at that first branch and never reach the edited line at all.x_BillingExchangeRateis an "Azure 1.0-preview(v1)+" FOCUS column that Cost Management commonly leaves null for single-currency billing accounts (no exchange rate needed whenPricingCurrency == BillingCurrency) — which is the common case, not the exception. For those accounts, assuming a rate of 1.0 is correct, not a guess.isnotempty(x_BillingExchangeRate)therefore made single-currency accounts (the majority) silently drop rows fromCommitmentDiscountQuantity-based aggregates —sum()ignores nulls, so utilization percentages lose rows from their denominator without any visible signal. That trades a bounded, visible inaccuracy (a wrong-but-present number under the old code) for a silent aggregate hole (a confidently wrong percentage) under the new code — a regression for the common case, not an improvement.CommitmentDiscountUnitderives fromisempty(CommitmentDiscountQuantity), so nulling the quantity also blanks the unit column for the same rows.Restored the original
coalesce(x_BillingExchangeRate, real(1))fallback in both files. Only these two lines (plus the comment that justified the now-reverted change) were touched — the savings-formula fix indashboard.jsonis untouched and remains in scope.Out of scope
Per the issue, this PR does not address:
BillingCurrencyvisibility in detail tables (only the Purchases table currently shows it).x_*InUsdcolumn coverage validation across ingestion paths).Verification
dashboard.jsonvalidated as well-formed JSON before and after edits.src/powershell/Tests/Unit/HubsKqlOperators.Tests.ps1andsrc/powershell/Tests/Lint/KqlJoinKinds.Tests.ps1pass (578/578).(List, Contracted, Effective) = (100,120,80)and(100,80,60): clamped sums giveNegotiatedDiscountSavings=20,CommitmentDiscountSavings=60; derivedEffectiveCost=140→ContractedCost'=200→ListCost'=220;List' - Contracted' = 20✓ matches Negotiated;Contracted' - Effective = 60✓ matches Commitment;TotalSavings' = 80andESR = 80/220 ≈ 36.4%, both self-consistent with the displayed operands.🤖 Generated with Claude Code