Fix KQL join pitfalls and adopt lookup for dimension enrichment - #2225
Fix KQL join pitfalls and adopt lookup for dimension enrichment#2225RolandKrummenacher wants to merge 15 commits into
Conversation
- SavingsPlan summary/details: the final resourcecontainers join had no kind and defaulted to innerunique, deduplicating the left side by subscription and silently dropping all but one savings plan recommendation per subscription. Now kind=inner. - AHB "VM Latest Change Last 7 days": joined resourcechanges record id against resources id, which never match, so the tile was always empty. Now joins on properties.targetResourceId (lowercased both sides) and uses kind=inner so mv-expanded license change rows are not collapsed. - Get-SQL-AHB-Disabled/Enabled: bare join on VMName dropped SQL VMs with duplicate names across resource groups/subscriptions (innerunique) and never matched VMs with uppercase names (left was original-case name, right tolower(name)). Now joins on the SQL VM properties.virtualMachineResourceId against the VM resource id with kind=inner, and the tag-filter semi-join states kind=inner explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Azure Resource Graph joins without an explicit kind default to innerunique, which deduplicates the left side on the join key and can silently drop rows. Make every bare join an explicit kind=inner in the hub recommendation queries and the finops-alerts logic app. Recommendations-Microsoft-SQLVMsWithoutAHB additionally joined SQL VMs to compute VMs on VMName with mismatched casing (left original case, right tolower), so VMs with uppercase names never matched, and duplicate VM names across resource groups collapsed. It now joins the SQL VM properties.virtualMachineResourceId against the VM resource id. docs/deploy/finops-alerts-*.json are generated from logicApp.bicep and intentionally not hand-edited; the next release build picks this up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The legacy EA transform functions in IngestionSetup_v1_0/v1_2 enriched cost rows with the open-data dimension tables (PricingUnits, Regions, ResourceTypes, Services) via join kind=leftouter. That shape is exactly what the lookup operator is built for: the large fact table stays on the left, the small dimension table is broadcast, and the duplicated join key columns (x_PricingUnitDescription1, ResourceLocation1, ...) are not emitted. No downstream code referenced the suffixed columns, so output is unchanged aside from dropping them before the final project. Also guard the Services enrichment against row fan-out: Services is not unique per x_ResourceType (a resource type can map to multiple consumed services), so joining its raw projection could duplicate cost rows. Dedupe with summarize take_any(...) by x_ResourceType, matching the pattern already used for the x_ConsumedService fallback, and apply the same dedup to the existing distinct-based lookups in the FOCUS transforms and HubSetup_v1_2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…talog tagging-policy-compliance, storage-tier-distribution, and macc-consumption-vs-commitment all join a large fact stream to a small, key-unique aggregate. Switch those joins to lookup so the small side is broadcast and the duplicated join key columns are not emitted. The biggest win is tagging-policy-compliance, where the full Costs() row set was previously the left side of a hash join against the distinct-tags dimension. Output schemas are unchanged; the suffixed key columns were never referenced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- compute.md AHB queries: bare ARG joins defaulted to innerunique; the SQL VM example additionally joined on VMName with mismatched casing (left original case vs right tolower), so VMs with uppercase names never matched and duplicate names across resource groups were silently dropped. Joins are now explicit kind=inner and the SQL VM example joins properties.virtualMachineResourceId to the VM id. - compute.md commitment coverage queries: switch the Prices dimension join to lookup kind=leftouter, the recommended pattern for enriching the large Costs table from a small key-unique aggregate. - finops-hub-database-guide.md / ftk-database-query.md: "on 1 == 1" is not a valid KQL join predicate; rewrite the percent-of-total examples with toscalar(), which is also cheaper (no second full-table join). - cost-spike/service-cost skill references: coalesce the join keys after kind=fullouter so baseline-only rows keep their dimension values instead of rendering with empty names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ Validation results (FTK test tenant + ftk-dev ADX cluster)ARG semantics probe — confirmed ARG bare Services open-data fan-out (commit 3) — confirmed against Hub ingestion lookup chain (commit 3) — the exact edited fragment (PricingUnits → Regions → ResourceTypes → Services ×2) executed against the Catalog queries (commit 4) — old vs new run back-to-back on
Recommendation queries + AHB/SavingsPlan workbook queries (commits 1-2) — all executed via the ARG REST API with parameters substituted: parse and run cleanly, incl. the 4-join SavingsPlan queries (ARG accepted 4 joins). Old vs new SavingsPlan counts match in this tenant (1 recommendation per subscription, so Not validated: row-level semantics of the SQL VM / public IP / app gateway queries (test tenant has no IaaS resources — all returned 0 rows, syntax-only), and the logic app queries (validated indirectly via identical shapes + 🤖 Generated with Claude Code |
Adds Tests/Lint/KqlJoinKinds.Tests.ps1, which scans every KQL-carrying surface (hub scripts, query catalog, ARG recommendation queries, ADX dashboard, finops-alerts logic app, workbooks, optimization engine runbooks and views, docs-mslearn best-practices examples) and fails on any bare "| join" without an explicit kind, since the innerunique default deduplicates the left side and silently drops rows. Remaining pre-existing bare joins (48 across 4 workbook files, all with unique left keys today) are baselined per file as a ratchet: counts can only go down, and lowering is enforced when a file is cleaned up. Also brings two surfaces to zero so they need no baseline: the SQL DB optimization runbook (2 bare joins, left side unique per ResourceId, so kind=inner preserves behavior) and the networking.md doc examples (2 bare joins in the backendless app gateway and idle public IP queries, published as copy-paste guidance). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live validation of the networking.md idle public IP query failed with DisallowedLogicalTableName: the joined subquery referenced "resource" instead of "resources", so the published example never ran. Found while verifying the explicit join kinds added in this PR; the corrected query now executes against Azure Resource Graph. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ Validation round 2 — full coverage of everything testableCompleting the earlier validation, every remaining testable change has now been run against live services (ARG REST API on the FTK test tenant; ARG (now all executed):
Hub ADX cluster (old vs new,
Transform functions (compile-checked against real schemas):
Mirrors: the three edited queries in Remaining untestable in this environment: row-level semantics of the SQL VM / public IP / app gateway ARG queries (test tenant has no IaaS — all validated as executing with 0 rows). 🤖 Generated with Claude Code |
✅ Validation round 3 — production-scale hub deploymentsAdditional read-only validation against two production FinOps hub deployments (36.6M and 28.3M cost rows; identities withheld). Key findings: The Services fan-out fix (commit 3) is directly relevant to deployed hubs:
No existing data corruption found in either hub: both currently ingest via the FOCUS path, and their deployed Equivalence re-confirmed on production data: 🤖 Generated with Claude Code |
There was a problem hiding this comment.
🟡 Human review recommended
The PR changes KQL behavior across multiple production-impacting surfaces (ingestion, recommendations, workbooks) and also introduces linting coverage gaps that should be resolved before relying on the new guardrail.
Pull request overview
This PR addresses correctness and performance pitfalls in KQL usage across the FinOps Toolkit by making join semantics explicit (avoiding ARG/ADX defaults like innerunique) and adopting lookup for small-dimension enrichment where appropriate. It also introduces a lint rule intended to prevent reintroducing bare join operators.
Changes:
- Fixed workbook and ARG recommendation queries where implicit/bad joins caused dropped rows or empty results, and made join kinds explicit.
- Converted several ingestion/catalog “fact-to-dimension” enrichments from
jointolookup, including dedupingServicesmappings to prevent fan-out. - Added a Pester lint test to detect bare
| joinusages (missingkind=) with a per-file baseline/ratchet.
File summaries
| File | Description |
|---|---|
| src/workbooks/optimization/SavingsPlan/SavingsPlan.workbook | Makes the final subscription enrichment join explicit to prevent innerunique row loss. |
| src/workbooks/optimization/Compute/AHB.workbook | Fixes join keys/kinds for AHB tiles and SQL VM enrichment to avoid mismatches and implicit dedupe. |
| src/workbooks/optimization/AHB/AHB.workbook | Mirrors AHB workbook join-key/kind fixes for correctness and consistency. |
| src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Recommendations/queries/Recommendations-Microsoft-VMsWithoutAHB.json | Makes the subscription join explicit (kind=inner) to avoid implicit innerunique. |
| src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Recommendations/queries/Recommendations-Microsoft-UnattachedPublicIPs.json | Makes the PIP enrichment join explicit (kind=inner). |
| src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Recommendations/queries/Recommendations-Microsoft-SQLVMsWithoutAHB.json | Fixes SQLVM↔VM join key (resourceId) and makes join kinds explicit. |
| src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Recommendations/queries/Recommendations-Microsoft-BackendlessAppGateways.json | Makes join kind explicit for backend pool summarization/enrichment. |
| src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_2.kql | Uses lookup for dimension enrichment and dedupes Services mapping to prevent fan-out. |
| src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/IngestionSetup_v1_0.kql | Same lookup + Services dedupe improvements for v1_0 transforms. |
| src/templates/finops-hub/modules/Microsoft.FinOpsHubs/Analytics/scripts/HubSetup_v1_2.kql | Dedupes Services lookup to avoid x_ResourceType fan-out. |
| src/templates/finops-alerts/modules/logicApp.bicep | Makes embedded ARG query join kinds explicit in the logic app definition. |
| src/templates/claude-plugin/agents/ftk-database-query.md | Rewrites percent-of-total example to avoid invalid join on 1 == 1, using toscalar(). |
| src/templates/agent-skills/finops-toolkit/references/service-cost-deep-dive.md | Improves fullouter join examples by coalescing join keys and projecting away duplicates. |
| src/templates/agent-skills/finops-toolkit/references/cost-spike-investigation.md | Same fullouter coalesce pattern for baseline/spike comparison example. |
| src/queries/finops-hub-database-guide.md | Rewrites percent-of-total example using toscalar() instead of join on 1 == 1. |
| src/queries/catalog/tagging-policy-compliance.kql | Converts a leftouter enrichment join to lookup for a small dimension. |
| src/queries/catalog/storage-tier-distribution.kql | Converts an inner join to lookup for currency totals enrichment. |
| src/queries/catalog/macc-consumption-vs-commitment.kql | Converts joins to lookup for small, aggregated right sides. |
| src/powershell/Tests/Lint/KqlJoinKinds.Tests.ps1 | Adds a lint test to detect bare ` |
| src/optimization-engine/runbooks/recommendations/Recommend-SqlDbOptimizationsToBlobStorage.ps1 | Makes join kind=inner explicit in embedded KQL. |
| docs-mslearn/toolkit/changelog.md | Updates changelog and ms.date to reflect join/lookup fixes. |
| docs-mslearn/best-practices/networking.md | Updates examples to use explicit join kinds and corrects a table name typo. |
| docs-mslearn/best-practices/compute.md | Updates examples to use lookup and fixes SQL VM join guidance to use resource IDs. |
Review details
Suppressed comments (1)
src/powershell/Tests/Lint/KqlJoinKinds.Tests.ps1:70
- If
docs-mslearn/toolkitis added to the scan targets, the current baseline needs an entry for the existing bare joins indocs-mslearn/toolkit/workbooks/customize-workbooks.md(currently 2 occurrences of| join () so the ratchet works as intended.
'src/workbooks/optimization/AHB/AHB.workbook' = 24
'src/workbooks/optimization/Compute/AHB.workbook' = 20
'src/workbooks/optimization/Networking/Networking.workbook' = 3
'src/workbooks/governance/workbook.json' = 1
}
- Files reviewed: 23/23 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Review feedback on PR #2225: the lint scan only covered docs-mslearn/best-practices, leaving other published docs unguarded. Scan all of docs-mslearn recursively (122 markdown files) and fix the two bare joins that surfaced in customize-workbooks.md, which teach the ResourceContainers-to-resources pattern with the implicit innerunique default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extends the KQL section with a "Joins and lookups" subsection in the same intent-first format as the string-matching rules: the innerunique default trap, lookup as the canonical dimension-enrichment form, mandatory dimension dedup (take_any by key, not distinct), leftanti for exclusions, fullouter key coalescing, ARG constraints (no lookup, no hints, 3-join limit), and the KqlJoinKinds.Tests.ps1 lint that enforces explicit join kinds with a per-file ratchet baseline. Grounded in the findings and live validation of PR #2225. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… example Live-probing Azure Resource Graph (all join flavors + lookup) showed the docs understate what ARG accepts and the repo overstates it: - supported: inner, innerunique, leftouter, rightouter, fullouter - rejected with InvalidQuery: lookup, leftsemi, leftanti, rightsemi, rightanti, and in/!in with a subquery The lint now enforces this: workbooks, recommendation queries, and the alerts logic app fail the build if they use lookup or a semi/anti join flavor, with an allowlist escape hatch for legitimate Log Analytics queries inside workbooks. The probe surfaced a broken published example: the orphaned-snapshots query in the azure-cost-management skill used join kind=leftanti, which ARG rejects, and order by on an uncast dynamic column, which ARG also rejects. Rewritten as leftouter + isempty (the only exclusion form ARG supports) with tolower on both join keys and toint on the sort column; verified executing against ARG. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review feedback (Copilot): the avoid/prefer table used backslash-escaped pipes inside inline code, which renders fine but pastes as invalid KQL from the raw markdown. The snippets are reworded to not need pipes at all. The KqlJoinKinds.Tests.ps1 references now note the test is added in PR #2225, since it does not exist on this branch; merge order is called out in the PR description. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cy example note, markdownlint - Reword the case-insensitivity rule so it no longer claims every plain operator is case-insensitive (== and in are not); scope the claim to matching operators with the _cs/equality forms as the opt-in. - Add an explicit note that the legacy 'join ... on 1 == 1' example further down is replaced by toscalar() in #2225 and must not be copied. - Fix MD036 (bold-as-heading) and align the new change log row's table pipes with the header (MD060). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The rule itself already states the pattern is invalid; the note would go stale the moment #2225 replaces the example it points at. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 Changes
Follow-up to a repo-wide review of KQL
joinvslookupbest practices (~880 join/lookup usages examined across hub database scripts, workbooks, ARG recommendation queries, the query catalog, and docs). This PR fixes the high-severity correctness bugs, adoptslookupwhere it is the documented best practice, and adds a lint rule so bare joins cannot come back. One commit per surface:resourcecontainersjoin had nokindand defaulted toinnerunique, deduplicating results by subscription and silently dropping all but one savings plan recommendation per subscription. Nowkind=inner.resourcechangesrecord id againstresources.id, which never match, so the tile was always empty. Now joinsproperties.targetResourceId(lowercased both sides) withkind=innerso mv-expanded license changes are preserved.VMNamedropped SQL VMs with duplicate names (innerunique) and never matched uppercase names (left original case vs righttolower). Now joinsproperties.virtualMachineResourceIdagainst the VM resource id.| join (made an explicitkind=inner;Recommendations-Microsoft-SQLVMsWithoutAHBgets the samevirtualMachineResourceIdjoin-key fix as above.docs/deploy/finops-alerts-*.jsonare generated and intentionally untouched; the next release build picks up the bicep change.PricingUnits,Regions,ResourceTypes,Services) withjoin kind=leftouter; converted tolookup kind=leftouter(broadcasts the small dimension, no duplicated key columns; no downstream references to the suffixed columns existed). Also guards theServicesenrichment against row fan-out withsummarize take_any(...) by x_ResourceType— this was an active bug: Services.csv has 30 duplicate resource-type keys (up to ×31 formicrosoft.sql/locations), so cost rows for those types were being multiplied.tagging-policy-compliance,storage-tier-distribution,macc-consumption-vs-commitment: fact-to-small-dimension joins converted tolookup.lookup, invalidjoin ... on 1 == 1percent-of-total examples rewritten withtoscalar(),fullouterexamples now coalesce their join keys so baseline-only rows keep their dimension values, and networking.md examples fixed (bare joins + an invalidresourcetable name that made the idle public IP example fail outright).Tests/Lint/KqlJoinKinds.Tests.ps1scans every KQL-carrying surface and fails on any bare| joinwithout an explicitkind=. The 48 remaining pre-existing bare joins (4 workbook files, all with unique left keys today) are baselined per file as a ratchet: counts can only go down (tracked in Workbooks: make remaining implicit and innerunique ARG join kinds explicit #2228). The SQL DB optimization runbook and networking.md were brought to zero in this commit.✅ Validation
All changes that can run against live services were validated — three rounds documented in the comments:
on 1 == 1versions fail live with General_BadRequest; newtoscalar()versions return correct totals),fullouterfixes eliminate the empty-key rows, and all 8 edited transform/HubSetup functions compile-checked against real schemas.kind=inneruniquetag-filter semi-joins across workbooks (style only; left keys unique today).lookup(maintenance-only query, intentionally left as is; not issue-worthy).🤖 Generated with Claude Code