Skip to content

fix(open-data,hubs): align commitment discount eligibility with FOCUS spend and usage categories - #2284

Open
Roland Krummenacher (RolandKrummenacher) wants to merge 1 commit into
devfrom
RolandKrummenacher/2279-eligibility-focus-mapping
Open

fix(open-data,hubs): align commitment discount eligibility with FOCUS spend and usage categories#2284
Roland Krummenacher (RolandKrummenacher) wants to merge 1 commit into
devfrom
RolandKrummenacher/2279-eligibility-focus-mapping

Conversation

@RolandKrummenacher

Copy link
Copy Markdown
Collaborator

🛠️ Description

Fixes #2279.

x_CommitmentDiscountSpendEligibility reported reservation pricing and x_CommitmentDiscountUsageEligibility reported savings plan pricing. That is the reverse of the FOCUS CommitmentDiscountCategory definition the toolkit uses everywhere else: a reservation is a Usage commitment (you commit to a quantity of usage) and a savings plan is a Spend commitment (you commit to an amount of money).

IngestionSetup_v1_2.kql contained both mappings ten lines apart — it assigned CommitmentDiscountCategory correctly, then assigned the eligibility columns the opposite way, under a // TODO: Check this to ensure it's correct comment. That TODO is now resolved.

What changed

Component Change
src/scripts/Update-CommitmentDiscountEligibility.ps1 Mapping extracted into a new New-EligibilityRow helper and corrected. The change-summary key is now built from the row's own properties, so it can't drift from the CSV column order.
src/open-data/CommitmentDiscountEligibility.csv The two columns are swapped for all 92,624 meters. 36,287 rows change value; the 56,337 meters eligible for both are unaffected.
IngestionSetup_v1_0.kql, IngestionSetup_v1_2.kql The spend column now reads spMeters, the usage column reads riMeters.
docs-mslearn/toolkit/open-data.md Corrects the two column descriptions and the sample rows, and states the FOCUS reasoning.
src/powershell/Tests/Unit/Update-CommitmentDiscountEligibility.Tests.ps1 Five new tests pin the direction and the published column order.

The exclusion of reservation price rows from their own eligibility (and x_SkuPriceType != 'ReservedInstance') travels with riMeters to the usage column: it describes what an on-demand charge could be covered by, not the commitment row itself.

The best practice query in docs-mslearn/best-practices/compute.md already used the correct mapping and is unchanged. data-dictionary.md and the Copilot Studio schema-reference.md also already described the columns per FOCUS, and now match the data.

🚨 Breaking change

The column names stay the same and the values change. Any report, query, or dashboard reading either column produces different results after this update. Both changelog entries (FinOps hubs and Open data) are marked Breaking change.

For hubs specifically: prices ingested after upgrading carry the corrected values; rows ingested earlier keep the old values until they are reingested.

🧪 Verification

Corrected CSV values checked against the live Azure Retail Prices API (api-version=2023-01-01-preview, meterRegion='primary') for a stratified sample of 12 meters covering all three value combinations — 12/12 match, including the 00020329-e657-5687-9d1a-0be9876e5116 meter from the issue and the two other sample rows in open-data.md:

MeterId                                API:RI    API:SP    | CSV Spend     CSV Usage     match
00020329-e657-5687-9d1a-0be9876e5116   True      False     | Not Eligible  Eligible      OK
00003b45-e996-5b04-b673-a2db710f9237   True      True      | Eligible      Eligible      OK
0003c425-b1df-551f-a855-9c95a08cb4ae   False     True      | Eligible      Not Eligible  OK

Test runs:

  • Invoke-Pester ./src/powershell/Tests/Unit/* — 2397 passed, 0 failed (4 pre-existing skips)
  • ./src/scripts/Test-PowerShell -Lint — 3587 passed, 0 failed
  • HubsKqlOperators.Tests.ps1 + KqlJoinKinds.Tests.ps1 — 587 passed, 0 failed

📝 Note for #2246

Draft PR #2246 replaces the derived eligibility computation in both ingestion transforms with a lookup against the open data table. It touches the same lines, so it will conflict here. Once the CSV is corrected, that lookup inherits the corrected values — but its x_SkuPriceType != 'ReservedInstance' guard should move from the spend column to the usage column, as it does in this PR.

✅ Checklist

  • Changelog entries added under both FinOps hubs and Open data, marked as a breaking change
  • Unit tests added for the corrected mapping
  • ms.date updated on both modified docs-mslearn files

🤖 Generated with Claude Code

… categories

x_CommitmentDiscountSpendEligibility reported reservation pricing and
x_CommitmentDiscountUsageEligibility reported savings plan pricing, which is
the reverse of the FOCUS CommitmentDiscountCategory definition the toolkit
uses everywhere else: a reservation is a Usage commitment (a quantity of
usage) and a savings plan is a Spend commitment (an amount of money).

Fixes the four components that carried the swap:

- Update-CommitmentDiscountEligibility.ps1: the mapping moves into a new
  New-EligibilityRow helper so a unit test can pin the direction.
- CommitmentDiscountEligibility.csv: the two columns are swapped for all
  92,624 meters (36,287 rows change value). Verified against the Retail
  Prices API for a stratified sample of 12 meters covering all three value
  combinations.
- IngestionSetup_v1_0.kql / IngestionSetup_v1_2.kql: the spend column now
  reads spMeters and the usage column reads riMeters. The
  "TODO: Check this to ensure it's correct" comment on the v1_2 block is
  resolved. The exclusion of reservation price rows from their own
  eligibility travels with riMeters.
- open-data.md: corrects the two column descriptions and the sample rows.

The best practice query in compute.md already used the correct mapping and
is unchanged.

This is a breaking change for consumers: the column names stay the same and
the values change, so existing reports and queries return different results.
Both changelog entries call that out.

Fixes #2279

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 26, 2026 15:36
@RolandKrummenacher Roland Krummenacher (RolandKrummenacher) added Type: Bug 🐛 Something isn't working Breaking change 🚨 Change that breaks previous deployments and requires a new major version Tool: Open data Datasets to normalize and enhance FinOps reports Tool: FinOps hubs Data pipeline solution Tool: FOCUS FOCUS feedback and documentation labels Aug 26, 2026
@microsoft-github-policy-service microsoft-github-policy-service Bot added the Needs: Review 👀 PR that is ready to be reviewed label Aug 26, 2026

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 encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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

Labels

Breaking change 🚨 Change that breaks previous deployments and requires a new major version Needs: Review 👀 PR that is ready to be reviewed Tool: FinOps hubs Data pipeline solution Tool: FOCUS FOCUS feedback and documentation Tool: Open data Datasets to normalize and enhance FinOps reports Type: Bug 🐛 Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Commitment discount eligibility columns swap the FOCUS Spend and Usage categories

4 participants