Skip to content

Fix Data Explorer dashboard savings formulas and FX fallback - #2248

Open
Michael Flanakin (flanakin) wants to merge 1 commit into
flanakin/v15-prepfrom
flanakin/2093-dashboard-savings-formulas
Open

Fix Data Explorer dashboard savings formulas and FX fallback#2248
Michael Flanakin (flanakin) wants to merge 1 commit into
flanakin/v15-prepfrom
flanakin/2093-dashboard-savings-formulas

Conversation

@flanakin

Copy link
Copy Markdown
Collaborator

Summary

Addresses the remaining Phase 1.3 items from #2093 that were left open after PR #2103 shipped the currency-filtering fix:

  1. Savings formulas — replaced inline sum(ListCost) - sum(EffectiveCost) style calculations with sum() of the canonical, row-level clamped columns (x_TotalSavings, x_CommitmentDiscountSavings, x_NegotiatedDiscountSavings) across 21 tiles in dashboard.json.
  2. Savings-plan utilization FX distortion — removed the silent 1:1 exchange-rate fallback in the data layer.

Why

Savings formulas (dashboard.json)

Costs/CostsPlus/CostsByMonth/CostsByDay already carry x_TotalSavings, x_CommitmentDiscountSavings, and x_NegotiatedDiscountSavings per 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_*Savings columns directly wherever a query aggregates over Costs* and then derives savings. Two tiles (Savings breakdown by month, Effective cost breakdown by month) were already locally re-deriving the same iff() clamp formula redundantly — simplified those to just sum the canonical columns that already exist on the source table, removing the redundant extend.

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 like ListCost too low or ContractedCost should be 0 — using clamped values there would defeat its purpose.

Savings-plan utilization FX distortion (data layer)

The dashboard doesn't reference x_BillingExchangeRate directly — the reported distortion actually lives one layer down, in CommitmentDiscountQuantity derivation (HubSetup_v1_2.kql, IngestionSetup_v1_2.kql):

CommitmentDiscountCategory == 'Spend', EffectiveCost / coalesce(x_BillingExchangeRate, real(1)),

x_BillingExchangeRate is null by design for non-Azure ingestion paths (and can be missing elsewhere), so this silently assumed a 1:1 rate whenever it was absent, distorting the derived commitment quantity — and any utilization metric built from it — by the true FX factor.

Fixed by following the existing pattern already used in the same case() expression for genuinely not-derivable values: fall through to real(null) instead of fabricating a number from an assumed rate.

CommitmentDiscountCategory == 'Spend' and isnotempty(x_BillingExchangeRate), EffectiveCost / x_BillingExchangeRate,
...
real(null)

This affects rows where a spend-based commitment discount applies but no exchange rate was captured — CommitmentDiscountQuantity is now left unset for those rows rather than silently wrong. The pipeline already has visibility into this: x_SourceValues's checkReal('CommitmentDiscountQuantity', ...) records whenever the derived value differs from the raw source value, so the change remains auditable without adding a new flag.

Note: the dashboard's own x_CommitmentDiscountUtilizationAmount/x_CommitmentDiscountUtilizationPotential (used in the "Commitment discount usage" tile) are cost/quantity-based and never touch x_BillingExchangeRate — they were not affected by this bug.

Out of scope

Per the issue, this PR does not address:

  • BillingCurrency visibility in detail tables (only the Purchases table currently shows it).
  • Phase 2 USD-normalization toggle (blocked on x_*InUsd column coverage validation across ingestion paths).

Verification

  • dashboard.json validated as well-formed JSON before and after edits; confirmed exactly 21 query text fields changed and no other structural change (same query/tile/page counts, same ids).
  • src/powershell/Tests/Unit/HubsKqlOperators.Tests.ps1 and src/powershell/Tests/Lint/KqlJoinKinds.Tests.ps1 pass (578/578).
  • Full Invoke-Pester unit suite passes (2181/2181, 119 skipped — pre-existing integration/deploy tests).
  • ./src/scripts/Build-Toolkit finops-hub completes successfully, confirming the edited KQL scripts are syntactically valid and build cleanly.

🤖 Generated with Claude Code

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Savings-breakdown tiles now display arithmetic that does not add up — operands stayed raw, results became clamped.
  2. Parts no longer sum to the whole — negotiated + commitment overstates total by ~61% on one hub.
  3. Effective savings rate silently inflated — clamped numerator over raw denominator.
  4. The FX guard likely regresses non-Microsoft FOCUS ingestion — and is a no-op on every Azure hub I checked.
  5. 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 extend re-derivations in 3b3f0a58 and 0341b3e4 is a genuine simplification — the latter's were entirely dead code.
  • Removing now-unused ListCost/ContractedCost from summarize lists (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",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
"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",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
"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",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
"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 ",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
"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",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
"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",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same ESR issue as e1bc2d51 — clamped numerator over raw denominator, on the CostsByDay variant.

// Don't assume a 1:1 exchange rate when it's missing -- that silently distorts
// the derived quantity (and any utilization built on it) by the true FX factor.
// Leave it unset like the other not-derivable cases below instead.
CommitmentDiscountCategory == 'Spend' and isnotempty(x_BillingExchangeRate), EffectiveCost / x_BillingExchangeRate,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This likely regresses non-Microsoft FOCUS ingestion.

x_BillingExchangeRate is a Microsoft-specific extension column — it does not exist in the FOCUS 1.0 spec. For an AWS or GCP FOCUS export, CommitmentDiscountCategory == 'Spend' is exactly what AWS Savings Plans land on, and those rows will have no exchange rate while BillingCurrency == PricingCurrency. In that case the old 1:1 assumption was correct, and the new guard nulls out CommitmentDiscountQuantity for every one of them. CommitmentDiscountUnit then follows it to '' via the isempty(CommitmentDiscountQuantity) branch just below.

Suggested narrower guard that keeps the real fix while not punishing same-currency sources:

CommitmentDiscountCategory == 'Spend' and isnotempty(x_BillingExchangeRate), EffectiveCost / x_BillingExchangeRate,
CommitmentDiscountCategory == 'Spend' and (isempty(PricingCurrency) or PricingCurrency == BillingCurrency), EffectiveCost,

PricingCurrency is explicitly set to '' when it cannot be determined (IngestionSetup_v1_2.kql:1180 and :1401), so the guard is safe.

Worth noting: I could not find a single row the strict version actually fixes. Across three production Azure hubs, every spend-commitment row had the exchange rate populated (and equal to 1) — zero missing. So as written this half of the PR changes nothing observable on Azure while creating a real risk on the non-Azure path.

Separately, the description justifies not adding a flag by pointing at x_SourceValues's checkReal('CommitmentDiscountQuantity', ...). That exists only at IngestionSetup_v1_2.kql:886. This function (Costs_v1_2()) applies the same case() to unioned Costs_final_v1_0 rows with no old_ capture and no checkReal — it just passes x_SourceValues through. So legacy v1.0 rows lose the quantity with no audit trail.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this addition, but it leaves us open for any cases where PricingCurrency isn't being set and we don't have an exchange rate. Is it better to leave CommitmentDiscountQuantity empty in those cases or assume a 1:1 exchange rate and populate it with the EffectiveCost? 🤔 If it's empty and FinOps practitioners need it, they'll just use EffectiveCost anyway, so I'm thinking it's best to fall back to EffectiveCost. And if we do that, this is a complete noop. It's arguably better logic, but no actual change.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if we do that, this is a complete noop.

That's true only if the EffectiveCost fallback also applies when the currencies are known to differ — in that case the whole expression collapses back to coalesce(x_BillingExchangeRate, real(1)) and we've written the original logic in more words. But that case is the one the PR was actually fixing, so it doesn't have to be included in the fallback.

Splitting it three ways keeps your fallback and is not a no-op:

CommitmentDiscountCategory == 'Spend' and isnotempty(x_BillingExchangeRate), EffectiveCost / x_BillingExchangeRate,
CommitmentDiscountCategory == 'Spend' and (isempty(PricingCurrency) or PricingCurrency == BillingCurrency), EffectiveCost,
// Spend, no exchange rate, and a known-DIFFERENT pricing currency: we cannot convert,
// so leave it empty rather than publish a number we know to be wrong.

Which gives:

Case Rate Currencies Result
1 present any EffectiveCost / x_BillingExchangeRate
2 absent equal (AWS/GCP Savings Plans) EffectiveCost — 1:1 is genuinely correct here
3 absent different empty — the actual fix
4 absent PricingCurrency unset EffectiveCost — your case

Case 4 is where I'd land on your side: PricingCurrency is required in FOCUS 1.0, so an empty one means a malformed export, and falling back there matches the previous behaviour and costs nothing. Case 3 is the only behaviour change, and it's the one worth having — a fabricated 1:1 quantity is worse than an empty one, because an empty column reads as "unavailable" while a wrong number reads as fact.

If it's empty and FinOps practitioners need it, they'll just use EffectiveCost anyway

Agreed for case 4. In case 3 that's exactly what we don't want them to do — EffectiveCost is in billing currency there, and the commitment is denominated in pricing currency, so silently substituting one for the other is the bug rather than the workaround.

Happy to measure how many rows actually land in each bucket on a real hub if that would help settle it — my guess is case 3 is rare on Microsoft data and case 2 is the whole of any AWS/GCP export.

// Don't assume a 1:1 exchange rate when it's missing -- that silently distorts
// the derived quantity (and any utilization built on it) by the true FX factor.
// Leave it unset like the other not-derivable cases below instead.
CommitmentDiscountCategory == 'Spend' and isnotempty(x_BillingExchangeRate), EffectiveCost / x_BillingExchangeRate,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern as the HubSetup_v1_2.kql copy — see that comment for the detail and the suggested currency-equality guard. This is the path that does have the old_CommitmentDiscountQuantity / checkReal audit trail, so the change is at least observable here.

@microsoft-github-policy-service microsoft-github-policy-service Bot added Needs: Attention 👋 Issue or PR needs to be reviewed by the author or it will be closed due to no activity and removed Needs: Review 👀 PR that is ready to be reviewed labels Aug 13, 2026
@microsoft-github-policy-service

Copy link
Copy Markdown

@Michael Flanakin (@flanakin): you have some new feedback!

Please review and resolve all comments and I'll let reviewers know by removing the Needs: Attention label. If I miss anything, just reply with #needs-review and I'll update the status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Attention 👋 Issue or PR needs to be reviewed by the author or it will be closed due to no activity Tool: FinOps hubs Data pipeline solution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants