microsoft / microsoft/finops-toolkit
Fix ingestion-side ListCost repair gaps for ESR/savings accuracy (defects 2-4 from #2214)
- Dominant language
- PowerShell
- Stars
- 603
- Forks
- 248
- Avg merge
- 7d 11h
- Merged PRs (30d)
- 11
Description
🤖 [AI] Split out from #2214, which covers 4 distinct defects behind the ADX dashboard's negative/wrong Effective Savings Rate. Defect 1 (dashboard aggregation bypassing the row-level x_TotalSavings clamp) is being fixed in #2248, scoped to dashboard.json only. This issue tracks the remaining 3, which live in the ingestion/data-repair pipeline (IngestionSetup_v1_0.kql / IngestionSetup_v1_2.kql) rather than the dashboard — a different layer with a different risk profile (write-path changes to stored columns vs. read-only dashboard queries), so it's tracked and will ship as a separate PR from #2248.
Defect 2 — Cost fallback gated behind an unrelated price-sheet lookup key
IngestionSetup_v1_2.kql has a fallback chain (ListCost → ContractedCost → EffectiveCost, lines ~460-487) that would repair rows with ListCost == 0. It never runs for rows without a Microsoft meter/offer ID — e.g. third-party Marketplace/ISV purchases — because the whole block is gated behind tmp_MissingPrices, which requires x_SkuMeterId and x_SkuOfferId to both be set. On one production hub, only 14 of 884 unrepaired rows had both IDs set; 0 passed all five conditions in the gate.
Fix: separate the price-sheet lookup (needs meter/offer ID) from the cost fallback (doesn't need it). Apply the fallback to any row with ListCost == 0 and EffectiveCost != 0, independent of the lookup gate.
Caution: on some of these rows, ContractedCost is already below EffectiveCost in the source export, so list >= contracted >= effective doesn't hold going in. The fallback chain alone won't repair that — decide what invariant to enforce before implementing.
Defect 3 — v1.0 ingestion path writes no x_SourceValues
IngestionSetup_v1_0.kql still repairs ListCost (line ~425) but never records the original value — the x_old_ListCost capture is a commented-out TODO. The v1.2 path does this correctly (10 checkReal() calls, x_SourceValues assignment). On one hub, 2.39M v1.0 rows have a hub-synthesized ListCost with no way to distinguish it from an exported value.
Fix: finish the TODO in IngestionSetup_v1_0.kql to record pre-repair values via x_SourceValues, matching the v1.2 pattern — or explicitly document that x_SourceValues is v1.2-only if backporting isn't feasible.
Defect 4 — MissingListCost flag unusable post-repair
The flag is computed before the repair runs, so it reflects the source export, not the current state. Combined with defect 3, this means v1.0 data has no way to tell "still broken" from "already repaired" — 98.9% of flagged rows on one hub now have a non-zero ListCost after repair, but the flag doesn't reflect that.
Fix: either clear the flag when repair succeeds, or add a distinct "repaired" marker so the flag can answer "what's still wrong" rather than "what was wrong on ingestion."
Minor cleanup noted alongside (optional, same files)
- Unreachable branch in both cost
casestatements inIngestionSetup_v1_2.kql(lines ~473, ~486) — two preceding conditions are exact complements, so the final fallback can never be selected. - Comment/condition mismatch at
IngestionSetup_v1_2.kql:466— comment says "billed cost" but condition testsEffectiveCost.
Scope note
This is a write-path change to the ingestion pipeline (stored x_SourceValues/flags), not a read-only dashboard fix — different risk profile from #2248, needs its own verification against real export data before merging.
Related: #2214, #2235, #2248
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Compare the cost-repair sections around lines 425 and 460-487 in IngestionSetup_v1_0.kql and IngestionSetup_v1_2.kql, starting with the v1.2 x_SourceValues pattern and the conditions around tmp_MissingPrices. Verify behavior against real export data, including third-party rows and pre-existing cost relationships. Done means the three tracked defects have an explicit, verified resolution and MissingListCost reliably distinguishes unresolved data from repaired data.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- azure
- Domain
- cloud, data-engineering
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100