Skip to content

feat(azure-cost): add App Service optimization guide#2559

Open
jongio wants to merge 5 commits into
microsoft:mainfrom
jongio:feature/cost-appservice-optimization
Open

feat(azure-cost): add App Service optimization guide#2559
jongio wants to merge 5 commits into
microsoft:mainfrom
jongio:feature/cost-appservice-optimization

Conversation

@jongio

@jongio jongio commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Adds an App Service cost optimization reference guide to the azure-cost skill.

Changes

  • New services/appservice/azure-app-service.md with optimization rules (stopped apps, empty plans, Premium in dev, idle slots, over-provisioned plans), tier decision matrix, Resource Graph queries, CLI commands, and pricing
  • Consolidated service routing in workflow.md into a compact table (Storage + App Service), reducing token usage
  • Added 2 vally eval stimuli for App Service cost routing
  • Minor trim to azure-storage.md intro and SKILL.md to maintain token budgets

Checklist

  • Tests pass locally (cd tests && npm test): vally lint 3/3, build passes, token limits pass
  • If modifying skill descriptions: verified routing correctness. App Service cost prompts use explicit cost/savings language to avoid routing to azure-deploy or azure-prepare
  • If modifying skill USE FOR / DO NOT USE FOR / PREFER OVER clauses: confirmed no routing regressions

Related Issues

Partial progress on #2524

- Add services/appservice/azure-app-service.md with plan rightsizing,
  idle slot detection, dev/test pricing, and Resource Graph queries
- Consolidate service routing in workflow.md into compact table format
- Add 2 vally eval stimuli for App Service cost routing
- Trim storage intro and SKILL.md to stay within token budgets

Partial progress on microsoft#2524
Copilot AI review requested due to automatic review settings June 3, 2026 22:55

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.

Pull request overview

Adds Azure App Service cost optimization coverage to the azure-cost skill by introducing a service-specific reference guide and wiring it into the cost-optimization workflow, with eval stimuli to validate routing.

Changes:

  • Added a new App Service optimization reference (azure-app-service.md) and linked it from the skill + workflow.
  • Consolidated service-specific routing in the cost-optimization workflow into a compact triggers table (Storage + App Service).
  • Added App Service routing stimuli to evals/azure-cost/eval.yaml and trimmed some markdown to maintain token budgets.
Show a summary per file
File Description
plugin/skills/azure-cost/SKILL.md Adds App Service to the Service Optimization Guides links and trims some scope/reference entries.
plugin/skills/azure-cost/cost-optimization/workflow.md Replaces the Storage-only conditional section with a compact service-specific triggers → reference table including App Service.
plugin/skills/azure-cost/cost-optimization/services/storage/azure-storage.md Minor intro text trim.
plugin/skills/azure-cost/cost-optimization/services/appservice/azure-app-service.md New App Service cost optimization reference (rules, tier matrix, ARG queries, commands, pricing notes).
evals/azure-cost/eval.yaml Adds 2 integration routing stimuli for App Service cost prompts.

Copilot's findings

  • Files reviewed: 5/5 changed files
  • Comments generated: 4

saikoumudi
saikoumudi previously approved these changes Jun 3, 2026
- Slots share plan workers (no per-slot charge), can drive scale-out
- Fix ARG query heading to match actual query scope
- Replace azure__appservice MCP reference with ARG/CLI for discovery

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's findings

  • Files reviewed: 5/5 changed files
  • Comments generated: 7

Comment thread plugin/skills/azure-cost/SKILL.md
Comment thread plugin/skills/azure-cost/SKILL.md
Comment thread plugin/skills/azure-cost/SKILL.md
Comment thread plugin/skills/azure-cost/cost-optimization/workflow.md Outdated
- Restore Billing Account scope pattern in SKILL.md
- Convert pipe-separated links to bullet list
- Fix workflow heading (Step 1.7 not Steps 1.7-1.75)
- Add Subscription Input Options to App Service guide
- Fix tier detection logic (Free and Shared are separate values)
- Clarify Dev/Test pricing is subscription-level

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's findings

  • Files reviewed: 5/5 changed files
  • Comments generated: 1

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

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's findings

  • Files reviewed: 5/5 changed files
  • Comments generated: 2

|----------|------|----------------|----------------|-------------|
| 🔴 Critical | Stopped App on Paid Plan | Site `properties.state == 'Stopped'` and associated plan tier is not Free/Shared | Delete the app or move it to a Free/Shared plan; if it’s the only app on the plan, delete/scale down the plan | $50-500/mo |
| 🔴 Critical | Empty App Service Plan | Plan has zero apps deployed | Delete the plan | $50-400/mo |
| 🟠 High | Premium in Non-Production | `sku.tier in ['PremiumV2','PremiumV3']` AND `tags.environment in ['dev','test','staging']` | Downgrade to Basic or Standard | $100-600/mo |
Comment on lines +38 to +42
Resources
| where type =~ 'microsoft.web/sites'
| where properties.state =~ 'Stopped'
| where isnotempty(properties.serverFarmId)
| project name, resourceGroup, kind, state=properties.state

@wbreza wbreza left a comment

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.

Code Review — PR #2559

Two-model independent review (Claude Opus 4.7 + GPT-5.4). Findings tagged by consensus.

TL;DR

App Service guide is mostly sound (slot billing language and serverfarm/site distinction are correct), but there are a few technical-accuracy issues worth a second look — most notably a KQL query that doesn't enforce the rule it documents, and an "Always On" recommendation that doesn't actually affect App Service billing.


🔴 Critical (one model)

Workflow bypass — cost data not gathered before recommendations

File: plugin/skills/azure-cost/cost-optimization/workflow.md (Step 1.7 area)

After the workflow.md consolidation, an App Service request can load services/appservice/azure-app-service.md and produce savings recommendations without first executing the mandatory cost-query / bill-breakdown flow in Step 2+. The service guide doesn't restate that requirement either. Consider explicitly re-entering Step 2 after a service guide loads, or restating the bill-breakdown requirement inside the App Service guide.

"Disable Always On" as a cost-saving action

File: plugin/skills/azure-cost/cost-optimization/services/appservice/azure-app-service.md

App Service is billed at the plan level — turning Always On off does not reduce plan charges. Always On is a free per-app toggle that affects cold-start / warm-up behavior, not billing. Recommend removing this from the cost rules list, or reframing it strictly as a perf tradeoff (not an optimization a user will see on their bill).


🟡 Important

"Stopped App on Paid Plan" KQL doesn't check plan tier (consensus — both reviewers)

File: plugin/skills/azure-cost/cost-optimization/services/appservice/azure-app-service.md

The rule's detection logic says "Site state == Stopped AND plan tier is not Free/Shared", but the query only filters on properties.state =~ 'Stopped' and isnotempty(properties.serverFarmId). It never joins to microsoft.web/serverfarms to inspect sku.tier, so stopped apps on Free/Shared plans (which incur no plan cost) will be returned and incorrectly recommended for deletion/downgrade.

Suggested rewrite:

Resources
| where type =~ 'microsoft.web/sites'
| where properties.state =~ 'Stopped'
| extend planId = tolower(tostring(properties.serverFarmId))
| join kind=inner (
    Resources
    | where type =~ 'microsoft.web/serverfarms'
    | project planId = tolower(id), planSku = tostring(sku.name), planTier = tostring(sku.tier)
  ) on planId
| where planTier !in~ ('Free', 'Shared', 'Dynamic')
| project name, resourceGroup, planSku, planTier

Decision matrix conflates Free and Basic on SLA

File: plugin/skills/azure-cost/cost-optimization/services/appservice/azure-app-service.md (tier decision matrix)

Basic is a paid dedicated tier with a 99.95% SLA, but the matrix row groups "Free / Basic B1" under "No SLA". This materially misstates tier semantics in a rightsizing guide. Split into separate rows, or drop the "No SLA" claim from Basic.

SKILL.md description missing App Service trigger terms

File: plugin/skills/azure-cost/SKILL.md

The top-level skill description doesn't include any App Service / web app / deployment-slot trigger language even though this PR adds an App Service guide and two App Service routing evals. Without those terms in the description, prompts like "how can I save on my App Service plan?" may not reliably route to azure-cost. Consider adding: "App Service cost", "web app spending", "deployment slots", "App Service plan".

Orphan reference file after link removal (consensus — both reviewers)

File: plugin/skills/azure-cost/SKILL.md (removed link)

The PR removes the [SDK: Redis .NET](cost-optimization/sdk/azure-resource-manager-redis-dotnet.md) link from SKILL.md, but the target file still exists on disk and is referenced nowhere else in the skill tree. After merge it ships in the plugin as a dead file. Either delete the file in this PR, or keep the link. If the intent is cleanup, suggest splitting into its own change.


🟢 Nits

  • Premium tier list mismatchservices/appservice/azure-app-service.md: detection row lists ['PremiumV2','PremiumV3'] while the KQL query below includes 'Premium' (v1) too. Align both to the same set.
  • Step grouping inconsistencyworkflow.md: Storage + App Service are consolidated into a shared step while Redis (1.5) and AKS (1.8) remain separate. Not blocking, just inconsistent.

What looks good ✅

  • Slot-billing language correctly states slots share plan workers (addresses prior reviewer concern).
  • serverfarms vs sites distinction is correctly used elsewhere in the guide.
  • New eval stimuli follow the existing routing-test pattern.
  • Frontmatter and token budgets look compliant.

Verdict

Comment — content is close to mergeable; the Stopped-Apps KQL, Always On framing, and Basic-SLA row are the highest-value fixes before merge. Author can adjudicate the workflow bypass concern and the orphan SDK file as they see fit.

Generated via multi-model code review (Opus 4.7 + GPT-5.4) with cross-model consensus tagging.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants