fix(azure): Legacy (EA) recommendations pair the normalized SKU with the un-normalized quantity #1540

Activity

1. cristim

   added

   triagedItem has been triagedItem has been triaged

   priority/p1Next up; this sprintNext up; this sprint

   severity/criticalMajor harm when it happensMajor harm when it happens

   urgency/nowDrop other thingsDrop other things

   impact/manyAffects most usersAffects most users

   effort/sHoursHours

   type/bugDefectDefect

   on Jul 28, 2026
2. cristim commented on Aug 8, 2026

   

   cristim

   on Aug 8, 2026

   MemberAuthor

   More actions

   Investigation before implementation — premise confirmed by execution, scope wider than filed

   Verified against origin/main @ 9bfd6822c in a throwaway worktree. Every claim below is from a run, not a read.

   1. Still true on main? Yes.

   Drove the unmodified converter with the issue's exact scenario (Azure recommends 4 × Standard_D8s_v3, NormalizedSize="Standard_D2s_v3", RecommendedQuantity=4, RecommendedQuantityNormalized=16):

   LEGACY                      ResourceType="Standard_D2s_v3"  Count=4     <-- mismatched
   MODERN                      ResourceType="Standard_D8s_v3"  Count=4     <-- self-consistent
   LEGACY (no NormalizedSize)  ResourceType="Standard_D8s_v3"  Count=4     <-- self-consistent
   

   Both halves of the issue's claim hold: Legacy mixes the normalized SKU with the un-normalized count, and Modern does not.

   RecommendedQuantityNormalized appears nowhere in the repo — not in providers/, not in the fixture builders. The field is never read, so no path currently produces the correct normalized pair.

   The issue's line references are still accurate: converter.go:146 (ResourceType), :151-152 (Count), :228-230 (resolveLegacyResourceType).

   2. Does it reach a purchase, or only a display? A purchase.

   Carried the same payload through the compute converter into buildReservationBody and printed the bytes that go to calculatePrice and purchase:

   {"location":"eastus",
    "properties":{"appliedScopeType":"Shared","billingPlan":"Upfront",
      "displayName":"vm-eastus-Standard_D2s_v3-4x-1yr-allup-...",
      "quantity":4,"renew":false,"reservedResourceType":"VirtualMachines","term":"P1Y"},
    "sku":{"name":"Standard_D2s_v3"}}

   sku.name is the normalized SKU and quantity is the un-normalized count, on the wire. Azure has no reason to reject it: Standard_D2s_v3 is a real SKU and 4 is a valid quantity. The purchase succeeds and the customer is committed for the term.

   The only quantity guard on the path is rec.Count <= 0 (compute/client.go:488). Nothing anywhere validates that the SKU and the count are in the same units.

   And the money figures ride along unchanged. Same run:

   common.Recommendation: ResourceType="Standard_D2s_v3" Count=4
                          OnDemandCost=1000 CommitmentCost=600 EstimatedSavings=400
   

   EstimatedSavings is Azure's saving for the full recommended quantity — the converter says so itself at :161-167 ("NetSavings is the savings from buying the full recommended quantity"). So the dashboard reports $400/mo against a purchase that delivers roughly a quarter of it. That is the substance of "reported as fully actioned", and it is verifiable in-repo rather than inferred.

   3. Which direction does it fail? Under-buy — fails safe on commitment risk, unsafe on reported savings.

   The code pairs the smaller SKU with the smaller count, so it buys less capacity than intended. It cannot over-commit the customer into unusable capacity.

   Strictly, the direction is decided by Azure's data, not by the code: under-buy iff NormalizedSize ≤ the actual SKU. The armconsumption@v1.1.0 doc comments are terse ("The recommended Quantity Normalized") and do not state units, so that half rests on Azure's documented normalize-to-base-size behaviour rather than on anything in this repo. In-repo corroboration: every one of the 34 Legacy fixture call sites treats NormalizedSize as the small/base size — Standard_D2s_v3, GeneralPurpose_Gen5_2, 100RU, DW500c, Premium_P1.

   So: the customer spends real money on a 1–3 year commitment sized at a fraction of intent, keeps paying on-demand for the remainder, and sees the full-size savings number. Bad, but not the worse direction.

   4. Does the same logic serve other paths? Yes — considerably wider than the issue states.

   The issue says "every service purchase body, e.g. compute/client.go:421-426". It is seven services, all through the one shared recommendations.Extract:

   compute, cache, cosmosdb, database, managedredis, search, synapse.

   The Modern/MCA path is not affected — resolveModernResourceType prefers the top-level SKUName, so it is self-consistent (confirmed in the run above). This is a Legacy/EA-only defect, as filed.

   Not mentioned in the issue, and worth adding: Count also feeds provider-agnostic CLI logic in cmd/helpers.go —

   • applySizing / ApplyCoverage scale by newCount / rec.Count (:173-176),
   • ApplyTargetCoverage anchors on rec.Count (:207, :397-398),
   • --max-instances clamps on it (:528),
   • the duplicate checker compares existingCount >= rec.Count and subtracts (:686-698).

   None of that is gated on provider, so an Azure EA recommendation carries the wrong count into sizing math and into duplicate suppression as well as into the purchase body.

   5. Fix shape and size

   Two options, both named in the issue. Recommending A:

   Option A — make resolveLegacyResourceType prefer SKUProperties[SKUName], matching the Modern ladder. RecommendedQuantity is then already in the right units.

   • Legacy and Modern end up with one semantics — today an EA and an MCA customer with identical usage get different ResourceType.
   • Uses RecommendedQuantity (*float64) rather than RecommendedQuantityNormalized (*float32), avoiding a precision question on a money path.
   • Introduces no new "field present, its partner absent" state.
   • Test blast radius measured, not guessed: of 35 Legacy fixture call sites, 34 set NormalizedSize only (so the ladder still returns it and they are unchanged) and exactly one sets both — converter_test.go:101, TestExtract_ResourceTypePrefersNormalizedSize, which inverts.

   Option B — keep the NormalizedSize preference and source Count from RecommendedQuantityNormalized. Preserves the SKU shown today, but needs resolveLegacyResourceType to report which rung it used, a new fixture helper, and a decision for "normalized size present, normalized quantity absent". Note there is no empty-ResourceType guard anywhere in providers/azure, so a fall-through in that state would put an empty sku.name on the wire. Larger, and it adds a failure mode.

   Files: providers/azure/internal/recommendations/converter.go (~4 lines), providers/azure/internal/recommendations/converter_test.go (invert one test, add pairing coverage both directions). Option B additionally needs providers/azure/mocks/recommendation_fixtures.go. effort/s is right for A.

   Flag for the PR, not to be slipped in silently: either option changes what gets purchased for EA customers. Under A the purchased SKU changes from the normalized size to the actual size for every Legacy recommendation that carries both fields — which is the common real-world payload shape.

   Triage

   Premise confirmed, severity/critical defensible, labels unchanged. Two corrections to the issue text: the consumer count is seven services rather than one example, and the blast radius includes provider-agnostic CLI sizing and duplicate suppression in cmd/helpers.go, which the issue does not mention.

   Module state

   providers/azure baseline on main before any change: go build ./..., go vet ./..., go test -race -count=1 ./... all exit 0 — 12 ok + 2 no-test-files = 14 packages, 0 failures. No pre-existing breakage to work around, now that #1755 has the module running in CI.
3. cristim

   added 2 commits that reference this issue on Aug 8, 2026

   fix(azure): pair the recommended SKU with the count in its own units

   ...

   cd80e33

   fix(azure): pair the recommended SKU with the count in its own units (#…

   ...

   726389b
4. cristim

   closed this as completedin #1771on Aug 8, 2026