Add Start-FinOpsMultitool — cross-platform terminal UI for FinOps scanning - #2155
Add Start-FinOpsMultitool — cross-platform terminal UI for FinOps scanning#2155Zac larsen (z-larsen) wants to merge 119 commits into
Conversation
… GUI Adds the Azure FinOps Multitool as a new PowerShell cmdlet in the FinOps toolkit. The Multitool is a WPF-based GUI that scans an Azure tenant for cost optimization, governance, and FinOps insights including cost trends, orphaned resources, idle VMs, tag hygiene, reservation/savings plan utilization, AHB opportunities, budgets, anomaly alerts, and policy compliance. - Public/Start-FinOpsMultitool.ps1: thin launcher cmdlet with comment-based help - Private/FinOpsMultitool/: full implementation (24 scanner modules, WPF GUI, Power BI template) - Tests/Unit/Start-FinOpsMultitool.Tests.ps1: Pester unit tests Windows-only (requires WPF support).
|
@microsoft-github-policy-service agree company="Microsoft" |
|
Zac larsen (@z-larsen) This is exciting! I don't know much about the tool, but would love to learn more. Can you join us at the contributor sync next Wednesday to share? |
|
Thanks, Michael! Would love to join. |
…info - Add contract-aware cost access warning banner (EA/MCA/CSP) on Overview tab - Add contract-specific billing tab messages when billing access unavailable - Add MG hierarchy unavailable info node in tree view with role guidance - Fix tag cost queries: use TagKey grouping type (not Tag/Dimension) - Add batched TagKey+TagValue query attempt with per-tag fallback - Clear skipSubs between batched and per-tag strategies - Add throttle pacing (2s every 2 queries) to avoid 429s - Add EA/MCA cost access detection in Get-CostData - Add runspace pool for API call parallelization
|
Thanks Brett - all three are fixed in 1. TUI data-source selection fails before scans start 2. MACC results are not bounded by the selected scope 3. Billing Structure drops a readable EA enrollment Unit suite: 2433 passed / 0 failed / 7 skipped. |
Michael Flanakin (flanakin)
left a comment
There was a problem hiding this comment.
🤖 [AI][Claude] PR Review
Summary: Solid architecture and consistently read-only (no destructive Az cmdlets or write REST calls found anywhere across ~15,900 new lines of PowerShell). The main gaps are: agent-skill docs routing to a deleted skill, a cluster of "silent failure looks identical to clean scan" bugs across several scanner modules (a pattern this repo has been burned by before), a few standalone data-correctness bugs, and near-zero test coverage for the 30 scanner modules that are the actual feature being shipped.
🚫 Blockers (1)
- Eight agent-skill files route an AI agent to an
azure-cost-managementskill that a dedicated Pester test (AgentPlugins.Tests.ps1) confirms no longer ships — a dead end for any agent following the routing tables.
⚠️ Should fix (20)
Get-SavingsRealized.ps1— the per-subscription fallback path computes RI/SP savings with a different, incorrect formula than the rest of the file.Get-ContractInfo.ps1— theCurrencyfield is populated with a country code, not a currency code.Get-LegacyResources.ps1— sort order puts low-impact findings before high-impact ones.Get-PolicyRecommendations.ps1— two differently-named catalog entries share the same policy definition ID, double-counting one policy.Get-CarbonMetrics.ps1— the window-probe can lock onto an empty-data month and never try older windows.Get-IdleVMs.ps1— per-VM metrics failures are silently swallowed.Get-StorageTierAdvice.ps1— capacity/transaction metric failures are silently swallowed or defaulted to zero.Get-BudgetStatus.ps1— large-tenant sampling checks only the first 10 subscriptions (not random) and silently skips the rest if that sample is empty.Get-BudgetStatus.ps1— reusing a partialCostTrendresult can zero-fill months it doesn't actually cover.Get-CommitmentUtilization.ps1— per-reservation utilization fetch failures are silently swallowed, understating counts with no caveat.Get-AIWorkloadMetrics.ps1— AI-detection query failures are reported identically to "no AI workloads found."Get-PolicyInventory.ps1— an exception partway through the subscription loop aborts the rest of the tenant silently, and the per-subscription fallback strategy never triggers to fill the gap.Get-CostTrend.ps1— multi-subscription cost aggregation sums cost across subscriptions without checking or converting currency.Get-AIWorkloadMetrics.ps1— AI cost aggregation has the same cross-currency summation issue.Get-UnitEconomics.ps1— compute/storage cost aggregation has the same cross-currency summation issue.Get-SharedCostAllocation.ps1— the cost-by-resource query never followsnextLink, understating allocations for subscriptions with many billed resources.Get-CostByTag.ps1— the same missing-pagination gap in its core cost-by-tag query.Get-OrphanedResources.ps1— the same missing-pagination gap in its cost-enrichment query.Get-FOHubProvider.ps1— a KQL comparison wraps a column intolower(), which the project's own coding guidelines explicitly forbid (the operator used is already case-insensitive).Start-FinOpsMultitool.ps1— comment-based help claims export options for Excel, JSON, and Power BI; the code only ever produces CSV, HTML, and a text summary.
Test coverage: the 4 new Pester test files (559 lines) cover only shared plumbing (hub provider resolution, hub size classification, policy-effect string parsing, cmdlet parameter shape). None of the 30 scanner modules' actual domain logic (Get-IdleVMs, Get-OrphanedResources, Get-CostByTag, etc. — roughly 8,000+ lines) has a single test exercising it. Given "Include Pester tests for all functions" is a stated convention, this is worth closing before or shortly after merge — even a handful of mocked tests for the highest-risk scanners (idle VMs, orphaned resources, cost by tag) would materially reduce risk.
💡 Suggestions (6)
Get-TagRecommendations.ps1— every catalog entry isPriority = 'Required', so theMissingRecommendedfilter can never match anything.Get-VmCostBreakdown.ps1— the VM-lookup query has noorder by, so which VM is picked when a name is ambiguous isn't deterministic across runs.Get-SharedCostAllocation.ps1—$Spokesisn't deduplicated before allocation, so a caller-supplied duplicate subscription ID dilutes and double-counts.src/powershell/Private/FinOpsMultitool/README.md— listsAz.Resourcesas a required module; nothing in the multitool actually calls anAz.Resourcescmdlet (all reads go through raw REST or Resource Graph).- Naming: the "AHB" acronym is cased inconsistently —
Get-AhbVmSavingsRatio.ps1/Get-AhbVmRatesuseAhb,Get-AHBOpportunities.ps1usesAHB. Invoke-AzRestMethodWithRetry.ps1— exponential backoff has no jitter; low risk for the interactive CLI, but worth adding if-NonInteractiveis ever run as a fleet of concurrent scheduled jobs against the same tenant.
Not all 90+ files in this PR were reviewed at the same depth — review focused on the PowerShell scanner modules, helpers, entry points, tests, and docs/agent-skills, which make up the vast majority of the new code.
| | Should we buy reservations or savings plans? | Purchase recommendations | [references/commitments.md](references/commitments.md) | | ||
| | Are we using what we already bought? | Commitment utilization | [references/commitments.md](references/commitments.md) | | ||
| | What have commitments actually saved us? | Realized savings | [references/commitments.md](references/commitments.md) | | ||
| | How is our MACC tracking? | Consumption commitment burn-down | `azure-cost-management` → `references/azure-macc.md` | |
There was a problem hiding this comment.
🤖 [AI][Claude] 🚫 Blocker
This routing table (and lines 62, 66, 110 in this same file) points to an azure-cost-management skill that no longer ships — src/powershell/Tests/Unit/AgentPlugins.Tests.ps1 (lines 142-151) explicitly asserts skills/azure-cost-management doesn't exist. That test only scans plugin manifests, not skill body text, so these references slipped through. An agent following "How is our MACC tracking?" or "Advisor cost recommendations" hits a dead end. The same dangling reference also appears in finops-multitool/references/allocation.md, waste-detection.md, cost-analysis.md, commitments.md, anomaly-investigation/SKILL.md, azure-workbooks-finops/SKILL.md, and rate-optimization-portfolio/SKILL.md — recommend either restoring the referenced content inline/into an existing skill, or removing these pointers tenant-wide.
| $cost = [math]::Round([double]$row[0], 2) | ||
|
|
||
| if ($pricingModel -match 'Reservation') { | ||
| # Amortized RI cost — the actual RI spend | ||
| $riSavings += $cost * 0.4 # Approximate: RIs typically save ~40% vs PAYG | ||
| $committedAmort += $cost | ||
| [void]$details.Add([PSCustomObject]@{ | ||
| Subscription = $sub.Name | ||
| Category = 'Reservation Benefit' | ||
| Amount = $cost | ||
| Type = 'Commitment' | ||
| }) | ||
| } | ||
| elseif ($pricingModel -match 'SavingsPlan') { | ||
| $spSavings += $cost * 0.25 # Approximate: SPs save ~25% on average | ||
| $committedAmort += $cost |
There was a problem hiding this comment.
🤖 [AI][Claude]
This per-subscription fallback path computes $riSavings += $cost * 0.4 and $spSavings += $cost * 0.25 directly. That contradicts the formula this same file documents and uses everywhere else: lines 39-43 explain that for a discount rate d, savings = paid * d / (1-d), not paid * d — "At a 40% discount, $100 paid implies $66.67 saved, not $40." The primary MG-scope path (lines 141, 146) correctly applies $script:FinOpsRiSavingsFactor/FinOpsSpSavingsFactor (d/(1-d)), but this fallback — which runs whenever MG-scope Cost Management access is unavailable — reverts to the naive cost * d. For the same stated 40%/25% discount assumption (see the EstimateBasis message at line 392), this understates RI savings by ~40% and SP savings by ~25% relative to what the primary path would report for identical costs. Suggest reusing $script:FinOpsRiSavingsFactor/FinOpsSpSavingsFactor here too.
| AccountId = $matchedAccount.name | ||
| AgreementType = $props.agreementType | ||
| FriendlyType = $friendlyType | ||
| AccountStatus = $props.accountStatus |
There was a problem hiding this comment.
🤖 [AI][Claude]
Currency = if ($props.soldTo) { $props.soldTo.country } else { 'Unknown' } — soldTo is a billing mailing address, not a currency source, so this puts a country code (e.g. "US") into a field named Currency. Any downstream consumer reading this field gets a country code mislabeled as a currency code.
| HasData = ($allLegacy.Count -gt 0) | ||
| TotalCount = $allLegacy.Count | ||
| LegacyResources = @($allLegacy | Sort-Object @{ Expression = 'Impact'; Descending = $true }, Category) | ||
| ByCategory = $byCategory |
There was a problem hiding this comment.
🤖 [AI][Claude]
Sort-Object @{ Expression = 'Impact'; Descending = $true }, Category sorts on the string value of Impact, and the only values used elsewhere in this file are 'High' and 'Low'. Alphabetically 'H' < 'L', so a descending sort puts 'Low' before 'High' — the opposite of the intended remediation priority. A caller reading LegacyResources top-to-bottom sees the least-important finding first.
| ) | ||
| } | ||
| [PSCustomObject]@{ | ||
| PolicyDefId = '/providers/Microsoft.Authorization/policyDefinitions/7433c107-6db4-4ad1-b57a-a76dce0154a1' |
There was a problem hiding this comment.
🤖 [AI][Claude]
This entry ("Storage accounts should be limited by allowed SKUs") uses the same PolicyDefId (.../policyDefinitions/7433c107-6db4-4ad1-b57a-a76dce0154a1) as the "Allowed storage account SKUs" entry above at line 125. Looks like a copy-paste duplicate — the same built-in policy is presented twice under different display names, double-counting it across Analysis/Missing/Assigned.
| $res = Search-AzGraphSafe -Query $vmQuery -Subscription $SubscriptionIds -First 50 | ||
| $rows = if ($res) { @($res.Data) } else { @() } | ||
| if ($rows.Count -eq 0) { return $null } | ||
|
|
There was a problem hiding this comment.
🤖 [AI][Claude] 💡 Suggestion
Resolve-VmAssociation's ARG query has no order by, so when multiple VMs match an ambiguous name, which one becomes "the" VM ($rows[0]) isn't deterministic across calls. The Ambiguous/Note caveat already surfaced elsewhere is a nice touch, but consider adding e.g. order by id asc so repeat runs against the same tenant pick the same VM.
| foreach ($w in $weights.Values) { $totalWeight += [double]$w } | ||
|
|
||
| # -- Allocate ---------------------------------------------------------- | ||
| $nSpokes = $Spokes.Count |
There was a problem hiding this comment.
🤖 [AI][Claude] 💡 Suggestion
$Spokes is never deduplicated before allocation (only $costSubs gets Select-Object -Unique, for cost querying). A caller-supplied duplicate subscription ID inflates $nSpokes — diluting every other spoke's fixed-cost share — and emits two allocation rows for that spoke. Low likelihood since it requires bad caller input, but a Select-Object -Unique on $Spokes up front would close it.
| | Requirement | Details | | ||
| | --------------------- | --------------------------------------------------------------- | | ||
| | PowerShell | 7.0 or later (Windows, macOS, Linux) | | ||
| | Az modules | `Az.Accounts`, `Az.Resources`, `Az.ResourceGraph`, `Az.Storage` | |
There was a problem hiding this comment.
🤖 [AI][Claude] 💡 Suggestion
Az.Resources is listed as a required module, but nothing in the multitool actually calls an Az.Resources cmdlet (grepped for Get-AzResource, Get-AzPolicyAssignment, Get-AzRoleAssignment, etc. across all modules — zero hits; everything goes through raw REST calls or Search-AzGraphSafe). Worth double-checking whether it's still needed, or dropping it from the install instructions here and at line 31.
| @@ -0,0 +1,113 @@ | |||
| # Copyright (c) Microsoft Corporation. | |||
There was a problem hiding this comment.
🤖 [AI][Claude] 💡 Suggestion
Minor naming inconsistency: this file/function use all-caps AHB (Get-AHBOpportunities), while Get-AhbVmSavingsRatio.ps1/Get-AhbVmRates use Pascal-case Ahb. PowerShell/.NET naming guidance favors Pascal-case for multi-letter acronyms (Ahb) — worth picking one convention.
There was a problem hiding this comment.
This is a nit and valid to skip. We need to rename all of these when we move them to public funcs. They'll all become {verb}-FinOps{noun} anyway, so we can deliberate naming conventions later. Tho I would suggest we align with whatever Microsoft defines as standard conventions.
| elseif ($isServerErr) { | ||
| $retryAfter = [math]::Min(2 * [math]::Pow(2, $attempt), 30) | ||
| } | ||
| else { | ||
| $retryAfter = [math]::Min(10 * [math]::Pow(2, $attempt), 60) | ||
| } |
There was a problem hiding this comment.
🤖 [AI][Claude] 💡 Suggestion
Backoff here (and in Search-AzGraphSafe.ps1) is pure exponential with no jitter. Low risk for a single-process interactive CLI, but if -NonInteractive is run as a scheduled job across multiple tenants/machines that happen to hit the same throttle window, all instances retry on the identical schedule. Adding a small random jitter (e.g. Get-Random -Minimum 0.8 -Maximum 1.2) would spread that out.
# Conflicts: # docs-mslearn/toolkit/changelog.md
- Get-FOHubProvider: drop tolower() from comparison position; has_any is already case-insensitive (docs-wiki/Coding-guidelines.md) - Get-LegacyResources: rank Impact explicitly so High sorts before Low; a descending string sort placed 'Low' first - Get-ContractInfo: rename Currency to SoldToCountry; the field held a country code from soldTo, not a currency. Console and HTML updated. - Start-FinOpsMultitool: comment-based help no longer claims Excel, JSON, or Power BI exports (CSV/HTML/text only) or PowerShell 5.1 support
Cost Management query pagination: three scanners issued a query and read only the first page, so a subscription with a large resource footprint under-reported cost silently rather than erroring. - Add Get-CostQueryResponsePage helper: follows nextLink, warns (rather than staying silent) when a continuation page fails, caps the chain - Get-SharedCostAllocation: follow nextLink in the by-ResourceId query - Get-CostByTag: follow nextLink in the core cost-by-tag query - Get-OrphanedResources: follow nextLink in the cost-enrichment query - Add CostQueryPagination.Tests.ps1 covering the response-shape branches
Silent failure: six scanners treated a failed call as an empty result, so a throttled or unauthorized response was indistinguishable from a clean scan. Each now counts what it could not read and reports it. - Get-IdleVMs: empty catch on per-VM metrics now records the failure; adds EvaluatedVMs/MetricFailures, including on the no-running-VMs path - Get-StorageTierAdvice: same for per-account metrics; adds EvaluatedAccounts/MetricFailures - Get-CommitmentUtilization: per-reservation utilization failures no longer drop a reservation silently; adds UtilizationFailures. Also fixes an empty catch on the billing profile lookup. - Get-AIWorkloadMetrics: a failed detection query no longer reports "No AI workloads detected" as if verified; adds DetectionFailed and per-account MetricFailures - Get-BudgetStatus: large-tenant sampling now uses a random sample rather than the first 10, and no longer concludes "no budgets" when a probe failed - it queries the full tenant instead - Get-PolicyInventory: per-subscription try/catch so one failure cannot abandon the rest of the tenant scan; reports the partial count
Cross-currency summation: three scanners summed cost across rows while labelling the total with whichever row set Currency last, so a tenant billing in more than one currency produced a meaningless number that still looked authoritative. - Add Resolve-CurrencyLabel helper: records every currency seen and reports 'Mixed' rather than picking one; casing and padding normalized - Get-CostTrend: track currencies per month in Set-TrendFromGrouped - Get-UnitEconomics: Add-MeterCosts collects currencies instead of overwriting a single ref. Also fixes its empty catch. - Get-AIWorkloadMetrics: same for the AI cost aggregation - Add CurrencyLabel.Tests.ps1 (10 cases) covering the label, casing normalization, blank handling, and the mixed-tenant path
Final should-fix batch, plus a security hardening found during the pass. - Get-SavingsRealized: the per-subscription fallback multiplied cost by the raw discount rate (0.4 / 0.25) instead of the savings factor the same file documents and the main path uses (d / (1 - d)), understating RI/SP savings - Get-PolicyRecommendations: removed a duplicate catalog entry; two entries shared definition ID 7433c107-6db4-4ad1-b57a-a76dce0154a1 and double-counted it. Kept the authoritative display name confirmed against the Azure API. - Get-CarbonMetrics: the window probe accepted any HTTP 200, so a month that published no data locked the window and older windows were never tried. It now requires a non-empty value array. - Get-CostQueryResponsePage: validate nextLink before following it. A relative or malformed value silently produced an empty path, and an absolute URL on another host would have been rewritten onto the ARM endpoint. Now accepts a rooted relative path or an https URL on the signed-in cloud's ARM host. - Extend CostQueryPagination.Tests.ps1 with 8 nextLink validation cases
🛠️ Description
Adds the FinOps multitool to the FinOps toolkit. Discussed with Brett Wilson (@MSBrett), who suggested contributing the tool into the official toolkit.
The multitool scans an Azure environment for cost optimization, governance, and FinOps insights — cost trends, orphaned resources, idle VMs, tag hygiene, reservation and savings plan utilization, Azure Hybrid Benefit opportunities, budgets, anomaly alerts, and policy compliance — and grounds its findings in live resource state.
Everything in this PR is read-only. It needs Reader or Cost Management Reader on the target scope and never creates, changes, or deletes a resource. Remediation and the MCP server were split out to a separate branch and will follow as their own PR.
One scanner engine, two consumers:
Start-FinOpsMultitoolsrc/templates/agent-skills/azor an Azure MCP serverRunning it
The terminal UI uses arrow-key menus when the console supports them. Consoles that can't render those menus — PowerShell remoting sessions, some editor terminals — fall back to numbered prompts, which is also what a screen reader can follow. Both paths run the same scans and produce the same results.
For automation,
-NonInteractivewith-Scans,-DataSource,-SubscriptionId, and-OutputPathsupplies every choice, so the tool runs from a pipeline or a scheduled job:FinOps hub data paths
This addresses Brett Wilson (@MSBrett)'s scaling review. When a FinOps hub is present, cost scans prefer the hub's Kusto database — an Azure Data Explorer or Fabric cluster, auto-discovered through Resource Graph, or a local ftklocal emulator via
FINOPS_HUB_KUSTO_URI— and push aggregation into the engine, returning only summarized result sets. Raw cost rows are never materialized in PowerShell on that path.The storage-export reader remains as a small-dataset fallback rather than the scalable path, and the terminal UI warns before using it on a hub with no reachable cluster, offering the live Cost Management API instead.
Scans
30 scan modules across optimization, governance, cost analysis, commitments, monitoring, Advisor, account, AI and ML, and sustainability. The terminal UI surfaces 26 of them. Results render in the terminal and export to one CSV per scan, a
FinOpsReport.htmlsummary, and aScanSummary.txtfile.📦 Files added / changed
Public/Start-FinOpsMultitool.ps1Invoke-FinOpsMultitool.ps1+FinOpsMultitool.psm1modules/helpers/Get-FOHubProvider.ps1+Invoke-FOHubKustoQuery.ps1agent-skills/finops-multitool/+references/agent-skills/cost-data-source/agent-skills/{power-bi-finops, cost-allocation, …}/Tests/Unit/Start-FinOpsMultitool.Tests.ps1+FOHubProvider.Tests.ps1docs-mslearn/.../powershell/multitool/+docs/multitool.md📸 Screenshots
Screenshots are in the public repo README.
📋 Checklist
🧪 How did you test this change?
🐳 Deploy to test?
N/A — standalone PowerShell tooling, not a template deployment.
🏷️ Do any of the following that apply?
📄 Did you update
docs/changelog.md?📖 Did you update documentation?
docs-mslearn/.../powershell/multitool/, a Jekyll landing page, overview/TOC/changelog entries, and the module README plus thefinops-multitoolandcost-data-sourceskills.