From 589e343d2294f45ca579ac53fdc6a431e241434f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 17:15:16 +0200 Subject: [PATCH 1/2] chore(lint): reorder fieldalignment structs in providers/azure Reorder struct fields to the fieldalignment-optimal layout. serviceResult and the AssertConsumptionUnitVariants table get keyed or reordered literals so the reorder cannot silently reassign values. Client in services/savingsplans/client.go keeps its order with a nolint because open PR #491 modifies that file. Part of #100 Co-Authored-By: Claude Opus 5.5 --- .../internal/recommendations/consumption.go | 8 +-- .../internal/recommendations/converter.go | 36 +++++------ providers/azure/mocks/consumption_fixtures.go | 6 +- providers/azure/mocks/pricing_http.go | 9 +-- providers/azure/provider.go | 21 +++---- providers/azure/recommendations.go | 18 +++--- .../recommendations_multi_subscription.go | 8 +-- providers/azure/recommendations_test.go | 60 +++++++++---------- providers/azure/services/cache/client.go | 9 +-- providers/azure/services/compute/client.go | 52 ++++++++-------- providers/azure/services/compute/exchange.go | 23 +++---- .../services/compute/exchange_operations.go | 23 +++---- providers/azure/services/cosmosdb/client.go | 9 +-- providers/azure/services/database/client.go | 22 +++---- .../internal/reservations/displayname.go | 19 +++--- .../internal/reservations/purchase.go | 4 +- .../reservations/purchasecommitment.go | 4 +- .../reservations/reservationstest/capture.go | 16 ++--- .../azure/services/managedredis/client.go | 6 +- .../savingsplans/benefit_recommendations.go | 10 ++-- .../azure/services/savingsplans/client.go | 2 +- providers/azure/services/search/client.go | 8 +-- providers/azure/services/synapse/client.go | 10 ++-- 23 files changed, 197 insertions(+), 186 deletions(-) diff --git a/providers/azure/internal/recommendations/consumption.go b/providers/azure/internal/recommendations/consumption.go index 12f53a5c..354510a3 100644 --- a/providers/azure/internal/recommendations/consumption.go +++ b/providers/azure/internal/recommendations/consumption.go @@ -185,8 +185,8 @@ func TermYears(term string) (int, error) { // ReservationPrice is the retail price of one reservation unit for a whole // term (the Retail Prices API's priceType 'Reservation' row), in Currency. type ReservationPrice struct { - Total float64 Currency string + Total float64 } // PriceFunc resolves the term-total retail price of one reservation unit. @@ -198,8 +198,8 @@ type priceKey struct { } type priceResult struct { - price float64 err error + price float64 } // Pricer resolves reservation unit prices through lookup, once per distinct @@ -208,9 +208,9 @@ type priceResult struct { // catalog (see IsCollectionFailure) is never cached. It is meant // to live for one GetRecommendations call and is not safe for concurrent use. type Pricer struct { - service string lookup PriceFunc cache map[priceKey]priceResult + service string } // NewPricer returns a Pricer for service backed by lookup. @@ -300,8 +300,8 @@ func ExpandConsumptionVariants(ctx context.Context, base common.Recommendation, // SkipLog collects the recommendations a collection dropped. It is attached // to the collection context by WithSkipLog and is safe for concurrent use. type SkipLog struct { - mu sync.Mutex entries []string + mu sync.Mutex } type skipLogKey struct{} diff --git a/providers/azure/internal/recommendations/converter.go b/providers/azure/internal/recommendations/converter.go index 8cef5f7c..c13a46a1 100644 --- a/providers/azure/internal/recommendations/converter.go +++ b/providers/azure/internal/recommendations/converter.go @@ -33,24 +33,6 @@ import ( // reservation recommendation services, normalised across both Legacy // and Modern API response shapes. type ExtractedFields struct { - Region string - ResourceType string - Count int - OnDemandCost float64 - CommitmentCost float64 - EstimatedSavings float64 - Term string - // LookBackDays is the lookback window the cost fields were computed over; - // set only by ExtractConsumption (0 from plain Extract). - LookBackDays int - // Scope is populated from the API response ("Shared" or a subscription ID) - // but is not yet threaded into the purchase body. All service clients - // currently send armreservations.AppliedScopeTypeShared, which is correct because - // the recommendation filter in recommendations.go already asserts - // "properties/scope eq 'Shared'" — only Shared-scope recommendations are - // ever requested. This field is retained for future wiring if single- - // subscription scoped recommendations are ever requested. - Scope string // RecurringMonthlyCost is the covered/effective cost over Azure's lookback // window, set by plain Extract only (from TotalCostWithReservedInstances, or // OnDemandCost - NetSavings when that is absent; nil, never a fabricated 0, @@ -58,6 +40,24 @@ type ExtractedFields struct { // ExpandConsumptionVariants sets it per payment variant (0 for upfront, // Count x reservation price / term months for monthly). RecurringMonthlyCost *float64 + Region string + ResourceType string + Term string + // Scope is populated from the API response ("Shared" or a subscription ID) + // but is not yet threaded into the purchase body. All service clients + // currently send armreservations.AppliedScopeTypeShared, which is correct because + // the recommendation filter in recommendations.go already asserts + // "properties/scope eq 'Shared'" — only Shared-scope recommendations are + // ever requested. This field is retained for future wiring if single- + // subscription scoped recommendations are ever requested. + Scope string + Count int + OnDemandCost float64 + CommitmentCost float64 + EstimatedSavings float64 + // LookBackDays is the lookback window the cost fields were computed over; + // set only by ExtractConsumption (0 from plain Extract). + LookBackDays int } // float64Ptr returns a pointer to the given float64 value. Used to diff --git a/providers/azure/mocks/consumption_fixtures.go b/providers/azure/mocks/consumption_fixtures.go index 7b45f3b4..4275ebdc 100644 --- a/providers/azure/mocks/consumption_fixtures.go +++ b/providers/azure/mocks/consumption_fixtures.go @@ -46,13 +46,13 @@ func AssertConsumptionUnitVariants(t *testing.T, recs []common.Recommendation, p t.Helper() require.Len(t, recs, 4, "two recommendations expand to four variants") cases := []struct { + term string upfront, monthly common.Recommendation onDemand, total, days float64 unit, months float64 - term string }{ - {recs[0], recs[1], fixture1yOnDemand, fixture1yTotal, fixtureLegacyWinDy, price1y, 12, "1yr"}, - {recs[2], recs[3], fixture3yOnDemand, fixture3yTotal, fixtureModernWinDy, price3y, 36, "3yr"}, + {"1yr", recs[0], recs[1], fixture1yOnDemand, fixture1yTotal, fixtureLegacyWinDy, price1y, 12}, + {"3yr", recs[2], recs[3], fixture3yOnDemand, fixture3yTotal, fixtureModernWinDy, price3y, 36}, } for i := range cases { c := &cases[i] diff --git a/providers/azure/mocks/pricing_http.go b/providers/azure/mocks/pricing_http.go index 7972c5e5..ce275670 100644 --- a/providers/azure/mocks/pricing_http.go +++ b/providers/azure/mocks/pricing_http.go @@ -14,13 +14,14 @@ import ( // the lookup for one SKU fail while the others succeed. The rows are // hand-built (synthetic prices, not a live Azure quote). type PricingHTTP struct { - Items []map[string]any - FailMatch string // Err, when set, is returned from every request (a transport failure). Err error - mu sync.Mutex - calls int + FailMatch string + Items []map[string]any + calls int + + mu sync.Mutex } // Do implements the HTTPClient interface the service clients accept. diff --git a/providers/azure/provider.go b/providers/azure/provider.go index c413f46e..b3c2a62f 100644 --- a/providers/azure/provider.go +++ b/providers/azure/provider.go @@ -96,13 +96,19 @@ func (r *realCredentialProvider) NewDefaultAzureCredential() (azcore.TokenCreden // Provider implements the provider.Provider interface for Azure. type Provider struct { cred azcore.TokenCredential - credOnce sync.Once credErr error - subscriptionID string - region string // Default region for operations subscriptionsClient SubscriptionsClient credProvider CredentialProvider + accountsSF singleflight.Group + + // cache subsystem implementation (getOrFetchAccounts, fetchAccounts, + // cloneAccounts, InvalidateAccountsCache) lives in accounts_cache.go. + subscriptionID string + region string // Default region for operations + cachedAccounts []common.Account + accountsGen uint64 + // accountsMu guards cachedAccounts. GetAccounts, GetServiceClient, and // GetRecommendationsClient all resolve the subscription list on the hot // path; without caching, a single logical operation (e.g. a @@ -121,13 +127,8 @@ type Provider struct { // singleflight call and gates the cache write, so an ARM fetch that // started before an invalidation can neither publish its stale snapshot // nor be joined by a caller that arrived after it. - accountsMu sync.RWMutex - cachedAccounts []common.Account - accountsGen uint64 - accountsSF singleflight.Group - - // cache subsystem implementation (getOrFetchAccounts, fetchAccounts, - // cloneAccounts, InvalidateAccountsCache) lives in accounts_cache.go. + accountsMu sync.RWMutex + credOnce sync.Once } // NewAzureProvider creates a new Azure provider instance. diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go index 2ffcc7f3..519b6b71 100644 --- a/providers/azure/recommendations.go +++ b/providers/azure/recommendations.go @@ -79,8 +79,8 @@ var ( // overridden per-instance in tests to avoid real ARM network calls. type RecommendationsClientAdapter struct { cred azcore.TokenCredential - subscriptionID string getAdvisorRecsFn func(ctx context.Context, params common.RecommendationParams) ([]common.Recommendation, error) + subscriptionID string } // NewRecommendationsClientAdapter builds a RecommendationsClientAdapter with @@ -222,11 +222,11 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p return nil, err } - merged, err := mergeServiceResults(serviceResult{"compute", computeRecs, computeErr, includeCompute}, - serviceResult{"database", dbRecs, dbErr, includeDB}, - serviceResult{"cache", cacheRecs, cacheErr, includeCache}, - serviceResult{"cosmosdb", cosmosRecs, cosmosErr, includeCosmos}, - serviceResult{"synapse", synapseRecs, synapseErr, includeSynapse}, + merged, err := mergeServiceResults(serviceResult{err: computeErr, name: "compute", recs: computeRecs, attempted: includeCompute}, + serviceResult{err: dbErr, name: "database", recs: dbRecs, attempted: includeDB}, + serviceResult{err: cacheErr, name: "cache", recs: cacheRecs, attempted: includeCache}, + serviceResult{err: cosmosErr, name: "cosmosdb", recs: cosmosRecs, attempted: includeCosmos}, + serviceResult{err: synapseErr, name: "synapse", recs: synapseRecs, attempted: includeSynapse}, // The savingsplans client is a stub that unconditionally returns // ([], nil) until the Benefits Recommendations API stabilizes (see // services/savingsplans Client.GetRecommendations). Counting its @@ -234,7 +234,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // all-attempted-failed guard from ever firing on a total provider // failure (expired credential, subscription-wide throttle), so it is // excluded from the guard until it makes real API calls. - serviceResult{"savingsplans", spRecs, spErr, false}, + serviceResult{err: spErr, name: "savingsplans", recs: spRecs, attempted: false}, // The Advisor client is excluded from the all-attempted-failed guard // for the same reason: getAdvisorRecommendations swallows pagination // errors (the auth failure from an expired credential surfaces as a @@ -244,7 +244,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // credential failure -- the same hazard the savingsplans exclusion // prevents. When getAdvisorRecommendations is changed to propagate // hard errors, flip this flag back to true. - serviceResult{"advisor", dropCoveredAdvisorSavingsPlans(advisorRecs, spRecs), advisorErr, false}) + serviceResult{err: advisorErr, name: "advisor", recs: dropCoveredAdvisorSavingsPlans(advisorRecs, spRecs), attempted: false}) return withSkips(merged, err, skips) } @@ -302,9 +302,9 @@ func (r *RecommendationsClientAdapter) advisorFn() func(context.Context, common. // stays under the cyclomatic-complexity gate after the post-Wait ctx.Err() // propagation was added. type serviceResult struct { + err error name string recs []common.Recommendation - err error // attempted records whether the service call was actually launched // (i.e. not skipped by the params service filter). Skipped services // carry nil recs and nil err, which is indistinguishable from a diff --git a/providers/azure/recommendations_multi_subscription.go b/providers/azure/recommendations_multi_subscription.go index 31d4f215..847a7aec 100644 --- a/providers/azure/recommendations_multi_subscription.go +++ b/providers/azure/recommendations_multi_subscription.go @@ -32,15 +32,15 @@ var newSubscriptionRecommendationsClientFn = func(cred azcore.TokenCredential, s // client so fan-out logs and error messages can identify which subscription // a failure came from. type subscriptionClient struct { - subscriptionID string client provider.RecommendationsClient + subscriptionID string } // SubscriptionFailure records one subscription that could not be queried // during an org-wide fan-out. type SubscriptionFailure struct { - SubscriptionID string Err error + SubscriptionID string } // ErrSubscriptionNotAccessible is the cause recorded for a subscription named @@ -93,6 +93,8 @@ func AsPartialResult(err error) *PartialResultError { // Failing the whole sweep on one transient subscription error would be worse // than a partial result, which is why the successful data is still returned. type PartialSubscriptionFailureError struct { + // Failed carries every subscription that errored, with its cause. + Failed []SubscriptionFailure // Attempted is how many subscriptions the sweep was supposed to cover: // the ones actually queried plus any named by AccountFilter that the // principal cannot see (those are never queried, but they were asked @@ -102,8 +104,6 @@ type PartialSubscriptionFailureError struct { // Succeeded is how many returned a result. Always < Attempted and > 0: // an all-failed sweep is a plain error, not a partial one. Succeeded int - // Failed carries every subscription that errored, with its cause. - Failed []SubscriptionFailure } func (e *PartialSubscriptionFailureError) Error() string { diff --git a/providers/azure/recommendations_test.go b/providers/azure/recommendations_test.go index 44f82153..fbf9157e 100644 --- a/providers/azure/recommendations_test.go +++ b/providers/azure/recommendations_test.go @@ -426,12 +426,12 @@ func TestMergeServiceResults_OrderIsStable(t *testing.T) { // Replicate the exact call order from GetRecommendations. result, err := mergeServiceResults( - serviceResult{"compute", []common.Recommendation{computeRec}, nil, true}, - serviceResult{"database", []common.Recommendation{dbRec}, nil, true}, - serviceResult{"cache", []common.Recommendation{cacheRec}, nil, true}, - serviceResult{"cosmosdb", []common.Recommendation{cosmosRec}, nil, true}, - serviceResult{"savingsplans", []common.Recommendation{spRec}, nil, true}, - serviceResult{"advisor", []common.Recommendation{advisorRec}, nil, true}, + serviceResult{err: nil, name: "compute", recs: []common.Recommendation{computeRec}, attempted: true}, + serviceResult{err: nil, name: "database", recs: []common.Recommendation{dbRec}, attempted: true}, + serviceResult{err: nil, name: "cache", recs: []common.Recommendation{cacheRec}, attempted: true}, + serviceResult{err: nil, name: "cosmosdb", recs: []common.Recommendation{cosmosRec}, attempted: true}, + serviceResult{err: nil, name: "savingsplans", recs: []common.Recommendation{spRec}, attempted: true}, + serviceResult{err: nil, name: "advisor", recs: []common.Recommendation{advisorRec}, attempted: true}, ) require.NoError(t, err, "all-success merge must not error") @@ -464,12 +464,12 @@ func TestMergeServiceResults_AllAttemptedFailed(t *testing.T) { authErr := errors.New("DefaultAzureCredential: federated token expired") recs, err := mergeServiceResults( - serviceResult{"compute", nil, authErr, true}, - serviceResult{"database", nil, authErr, true}, - serviceResult{"cache", nil, authErr, true}, - serviceResult{"cosmosdb", nil, authErr, true}, - serviceResult{"savingsplans", nil, nil, false}, - serviceResult{"advisor", nil, nil, false}, + serviceResult{err: authErr, name: "compute", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "database", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "cache", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "cosmosdb", recs: nil, attempted: true}, + serviceResult{err: nil, name: "savingsplans", recs: nil, attempted: false}, + serviceResult{err: nil, name: "advisor", recs: nil, attempted: false}, ) require.Error(t, err, "all-attempted-failed merge must fail loud, not return (empty, nil)") @@ -490,12 +490,12 @@ func TestMergeServiceResults_SkippedServicesDoNotMaskTotalFailure(t *testing.T) throttleErr := errors.New("429 too many requests") _, err := mergeServiceResults( - serviceResult{"compute", nil, throttleErr, true}, - serviceResult{"database", nil, nil, false}, - serviceResult{"cache", nil, nil, false}, - serviceResult{"cosmosdb", nil, nil, false}, - serviceResult{"savingsplans", nil, nil, false}, - serviceResult{"advisor", nil, nil, false}, + serviceResult{err: throttleErr, name: "compute", recs: nil, attempted: true}, + serviceResult{err: nil, name: "database", recs: nil, attempted: false}, + serviceResult{err: nil, name: "cache", recs: nil, attempted: false}, + serviceResult{err: nil, name: "cosmosdb", recs: nil, attempted: false}, + serviceResult{err: nil, name: "savingsplans", recs: nil, attempted: false}, + serviceResult{err: nil, name: "advisor", recs: nil, attempted: false}, ) require.Error(t, err, "skipped services must not count as successes in the all-failed guard") @@ -511,12 +511,12 @@ func TestMergeServiceResults_PartialFailureStillSucceeds(t *testing.T) { computeRec := common.Recommendation{Service: common.ServiceCompute, Provider: common.ProviderAzure} recs, err := mergeServiceResults( - serviceResult{"compute", []common.Recommendation{computeRec}, nil, true}, - serviceResult{"database", nil, svcErr, true}, - serviceResult{"cache", nil, svcErr, true}, - serviceResult{"cosmosdb", nil, svcErr, true}, - serviceResult{"savingsplans", nil, nil, false}, - serviceResult{"advisor", nil, nil, false}, + serviceResult{err: nil, name: "compute", recs: []common.Recommendation{computeRec}, attempted: true}, + serviceResult{err: svcErr, name: "database", recs: nil, attempted: true}, + serviceResult{err: svcErr, name: "cache", recs: nil, attempted: true}, + serviceResult{err: svcErr, name: "cosmosdb", recs: nil, attempted: true}, + serviceResult{err: nil, name: "savingsplans", recs: nil, attempted: false}, + serviceResult{err: nil, name: "advisor", recs: nil, attempted: false}, ) require.NoError(t, err, "partial failure must still return the successful services' recs") @@ -537,12 +537,12 @@ func TestMergeServiceResults_StubsDoNotMaskTotalFailure(t *testing.T) { authErr := errors.New("DefaultAzureCredential: federated token expired") recs, err := mergeServiceResults( - serviceResult{"compute", nil, authErr, true}, - serviceResult{"database", nil, authErr, true}, - serviceResult{"cache", nil, authErr, true}, - serviceResult{"cosmosdb", nil, authErr, true}, - serviceResult{"savingsplans", []common.Recommendation{}, nil, false}, - serviceResult{"advisor", []common.Recommendation{}, nil, false}, + serviceResult{err: authErr, name: "compute", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "database", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "cache", recs: nil, attempted: true}, + serviceResult{err: authErr, name: "cosmosdb", recs: nil, attempted: true}, + serviceResult{err: nil, name: "savingsplans", recs: []common.Recommendation{}, attempted: false}, + serviceResult{err: nil, name: "advisor", recs: []common.Recommendation{}, attempted: false}, ) require.Error(t, err, "stubs that swallow errors must not mask a total provider failure") diff --git a/providers/azure/services/cache/client.go b/providers/azure/services/cache/client.go index 7cbc96f5..259cc346 100644 --- a/providers/azure/services/cache/client.go +++ b/providers/azure/services/cache/client.go @@ -70,13 +70,15 @@ type RedisCachesPager interface { // Client handles Azure Cache for Redis Reserved Capacity. type Client struct { cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager redisCachesPager RedisCachesPager + skuCacheMap map[string]redisSKUEntry + subscriptionID string + region string + // Lazy SKU catalog cache. armredis exposes no per-region "list all // possible SKUs" surface, so we derive shard counts from the existing // caches in the subscription (NewListBySubscriptionPager). Fetched @@ -85,7 +87,6 @@ type Client struct { // skuCacheMap nil and converters fall back to Shards=0 with a WARN // log — the conversion itself does NOT fail. skuCacheOnce sync.Once - skuCacheMap map[string]redisSKUEntry } // NewClient creates a new Azure Cache client. @@ -541,9 +542,9 @@ func (c *Client) getCommonSKUs() []string { // RedisPricing contains pricing information for Redis Cache. type RedisPricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } func (c *Client) getRedisPricing(ctx context.Context, sku, region string, termYears int) (*RedisPricing, error) { diff --git a/providers/azure/services/compute/client.go b/providers/azure/services/compute/client.go index 2f25efa8..98234656 100644 --- a/providers/azure/services/compute/client.go +++ b/providers/azure/services/compute/client.go @@ -87,15 +87,21 @@ type vmSKUEntry struct { // Client handles Azure VM Reserved Instances. type Client struct { - cred azcore.TokenCredential - subscriptionID string - region string - httpClient HTTPClient + cred azcore.TokenCredential + httpClient HTTPClient // For testing - these can be set to mock implementations recommendationsPager RecommendationsPager resourceSKUsPager ResourceSKUsPager + capacityProviderErr error + + // Optional injected pager for ListExchangeableReservations. When nil + // (the production default) the method creates a real + // armreservations.ReservationClient. Tests inject a stub to run + // hermetically without Azure credentials. + exchangeablePager ExchangeableReservationPager + // Optional injected factories for the reservation-inventory path used by // GetExistingCommitments. When nil (the production default) the method // builds real armreservations SDK clients via @@ -103,26 +109,7 @@ type Client struct { // SetInventoryFactories to run hermetically without Azure credentials. inventoryFactories *InventoryFactories - // Microsoft.Capacity provider registration check (cached per client lifetime) - capacityProviderOnce sync.Once - capacityProviderErr error - - // Lazy SKU catalog cache. armcompute.ResourceSKUsClient.NewListPager - // returns every SKU available to the subscription with its - // Capabilities (vCPUs, MemoryGB). Fetched ONCE per client lifetime; - // subsequent converter calls in the same GetRecommendations run hit - // the in-memory map. A failed fetch leaves skuCacheMap nil and - // converters fall back to VCPU=0/MemoryGB=0 with a WARN log — the - // conversion itself does NOT fail (graceful-degradation contract, - // matches cache/cosmosdb/database from PR #81). - skuCacheOnce sync.Once - skuCacheMap map[string]vmSKUEntry - - // Optional injected pager for ListExchangeableReservations. When nil - // (the production default) the method creates a real - // armreservations.ReservationClient. Tests inject a stub to run - // hermetically without Azure credentials. - exchangeablePager ExchangeableReservationPager + skuCacheMap map[string]vmSKUEntry // Optional injected LRO callers for CalculateExchange and // ExecuteExchange. When nil (the production default) the methods @@ -137,6 +124,21 @@ type Client struct { // fake HTTP transport so the production LRO wrappers and the SDK's own // request serialization run without reaching Azure. exchangeClientOptions *arm.ClientOptions + subscriptionID string + region string + + // Microsoft.Capacity provider registration check (cached per client lifetime) + capacityProviderOnce sync.Once + + // Lazy SKU catalog cache. armcompute.ResourceSKUsClient.NewListPager + // returns every SKU available to the subscription with its + // Capabilities (vCPUs, MemoryGB). Fetched ONCE per client lifetime; + // subsequent converter calls in the same GetRecommendations run hit + // the in-memory map. A failed fetch leaves skuCacheMap nil and + // converters fall back to VCPU=0/MemoryGB=0 with a WARN log — the + // conversion itself does NOT fail (graceful-degradation contract, + // matches cache/cosmosdb/database from PR #81). + skuCacheOnce sync.Once } // NewClient creates a new Azure Compute client. @@ -674,9 +676,9 @@ func (c *Client) isAvailableInRegion(sku *armcompute.ResourceSKU, region string) // VMPricing contains VM pricing information. type VMPricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } // getVMPricing gets real VM pricing from Azure Retail Prices API. diff --git a/providers/azure/services/compute/exchange.go b/providers/azure/services/compute/exchange.go index 72206101..ac56a3ae 100644 --- a/providers/azure/services/compute/exchange.go +++ b/providers/azure/services/compute/exchange.go @@ -28,6 +28,10 @@ import ( // ExchangeableReservation represents an Azure VM reservation that is // eligible for exchange. type ExchangeableReservation struct { + + // ExpiryDate is when the reservation expires. Zero if not set by Azure. + ExpiryDate time.Time `json:"expiry_date,omitempty"` + // ReservationOrderID is the GUID of the parent reservation order, // parsed from the ARM resource ID. Required by CalculateExchange. ReservationOrderID string `json:"reservation_order_id"` @@ -59,16 +63,9 @@ type ExchangeableReservation struct { // Azure did not report one, which CalculateExchange refuses. AppliedScopeType armreservations.AppliedScopeType `json:"applied_scope_type,omitempty"` - // AppliedScopes are the ARM scopes (e.g. "/subscriptions/{id}") a Single - // scoped reservation applies to. Empty for Shared. - AppliedScopes []string `json:"applied_scopes,omitempty"` - // SKU is the VM size (e.g. "Standard_D2s_v3"). SKU string `json:"sku"` - // Quantity is the number of VM instances covered. - Quantity int32 `json:"quantity"` - // Region is the ARM region name (e.g. "eastus"). May be empty for // reservations with AppliedScopeType == Shared. Region string `json:"region,omitempty"` @@ -79,15 +76,19 @@ type ExchangeableReservation struct { // pair as unsupported. Term string `json:"term,omitempty"` - // ExpiryDate is when the reservation expires. Zero if not set by Azure. - ExpiryDate time.Time `json:"expiry_date,omitempty"` - // InstanceFlexibility reports the instance size flexibility setting. // Always "On" for reservations returned by this function. InstanceFlexibility string `json:"instance_flexibility"` // DisplayName is the human-readable reservation name set at purchase time. DisplayName string `json:"display_name,omitempty"` + + // AppliedScopes are the ARM scopes (e.g. "/subscriptions/{id}") a Single + // scoped reservation applies to. Empty for Shared. + AppliedScopes []string `json:"applied_scopes,omitempty"` + + // Quantity is the number of VM instances covered. + Quantity int32 `json:"quantity"` } // ExchangeableReservationPager defines the paging contract for listing @@ -194,13 +195,13 @@ func isExchangeEligible(item *armreservations.ReservationResponse) bool { // when the source pointer is nil. Grouped into a struct (rather than 7 // positional returns) per gocritic's tooManyResultsChecker. type reservationFields struct { + expiryDate time.Time id string sku string region string term string displayName string quantity int32 - expiryDate time.Time } // extractReservationFields reads the optional pointer fields from item and its diff --git a/providers/azure/services/compute/exchange_operations.go b/providers/azure/services/compute/exchange_operations.go index 298b8a36..1e3a3bf7 100644 --- a/providers/azure/services/compute/exchange_operations.go +++ b/providers/azure/services/compute/exchange_operations.go @@ -33,6 +33,12 @@ import ( // CompatibleOffering describes one candidate target SKU that Azure priced as // an exchange destination for the source reservations. type CompatibleOffering struct { + + // BillingCurrencyTotal is the net amount the customer would pay in their + // billing currency for this offering. Nil when Azure did not report an + // amount (never coerced to 0 -- absent is not the same as free). + BillingCurrencyTotal *float64 `json:"billing_currency_total"` + // SKU is the VM size (e.g. "Standard_D4s_v3"). SKU string `json:"sku"` @@ -46,16 +52,11 @@ type CompatibleOffering struct { // pair as unsupported. Term string `json:"term"` - // Quantity is the number of instances that would be purchased. - Quantity int32 `json:"quantity"` - - // BillingCurrencyTotal is the net amount the customer would pay in their - // billing currency for this offering. Nil when Azure did not report an - // amount (never coerced to 0 -- absent is not the same as free). - BillingCurrencyTotal *float64 `json:"billing_currency_total"` - // CurrencyCode is the ISO 4217 code for BillingCurrencyTotal (e.g. "USD"). CurrencyCode string `json:"currency_code,omitempty"` + + // Quantity is the number of instances that would be purchased. + Quantity int32 `json:"quantity"` } // ExchangePreview holds the priced result of a CalculateExchange call: what @@ -123,9 +124,6 @@ type ExchangeTarget struct { // CalculateExchange rather than second-guessed here. Term armreservations.ReservationTerm - // Quantity is the number of instances to reserve. Required: must be >= 1. - Quantity int32 - // BillingScopeID is the subscription or billing account that will be // charged. Required by the Azure exchange API. BillingScopeID string @@ -145,6 +143,9 @@ type ExchangeTarget struct { // CalculateExchange session; ExecuteExchange sends only the session ID, // so the executed exchange is billed with the plan that was priced. BillingPlan armreservations.ReservationBillingPlan + + // Quantity is the number of instances to reserve. Required: must be >= 1. + Quantity int32 } // CalculateExchangeCallerFunc is the narrow LRO-invoker interface that diff --git a/providers/azure/services/cosmosdb/client.go b/providers/azure/services/cosmosdb/client.go index 0d149f16..3b2d44c8 100644 --- a/providers/azure/services/cosmosdb/client.go +++ b/providers/azure/services/cosmosdb/client.go @@ -68,13 +68,15 @@ type CosmosAccountsPager interface { // Client handles Azure Cosmos DB Reserved Capacity. type Client struct { cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager cosmosAccountsPager CosmosAccountsPager + subscriptionID string + region string + apiType string + // Lazy account-API-type cache. Cosmos DB has no per-SKU APIType (the // API type lives on the account, not the reservation SKU which is // just a throughput tier like "100RU"). The cache fetches the @@ -83,7 +85,6 @@ type Client struct { // "Dominant" = the only API type observed if there's exactly one, // else empty (meaning: subscription is multi-API, can't infer). apiTypeOnce sync.Once - apiType string } // NewClient creates a new Azure Cosmos DB client. @@ -539,9 +540,9 @@ func (c *Client) getCommonSKUs() []string { // CosmosPricing contains pricing information for Cosmos DB. type CosmosPricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } func (c *Client) getCosmosPricing(ctx context.Context, sku, region string, termYears int) (*CosmosPricing, error) { diff --git a/providers/azure/services/database/client.go b/providers/azure/services/database/client.go index 1955d3d2..e29f078c 100644 --- a/providers/azure/services/database/client.go +++ b/providers/azure/services/database/client.go @@ -85,9 +85,8 @@ type SQLManagedInstancesPager interface { // Client handles Azure SQL Database Reserved Capacity. type Client struct { + serverInfoRetryAfter time.Time cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager @@ -95,13 +94,20 @@ type Client struct { serversPager SQLServersPager managedInstancesPager SQLManagedInstancesPager + skuCacheMap map[string]sqlSKUEntry + + subscriptionID string + region string + azConfig string + deployment string + serverInfoRetryDelay time.Duration + // Lazy SKU catalog cache. Populated once on the first // recommendation conversion in this client's GetRecommendations // call — single ListByLocation per region instead of N+1 per rec. // A failed fetch leaves skuCacheMap nil; converters then fall back // to empty EngineVersion with a one-time WARN log. skuCacheOnce sync.Once - skuCacheMap map[string]sqlSKUEntry // Lazy server-info cache. Populated by fetchServerInfo, which walks the // managed-instances and servers pagers. Both azConfig and deployment are @@ -109,12 +115,8 @@ type Client struct { // Only a complete walk is cached (serverInfoLoaded); a failed or // canceled walk is retried, but not before serverInfoRetryAfter so a // persistently failing subscription is not re-walked per recommendation. - serverInfoMu sync.Mutex - serverInfoLoaded bool - serverInfoRetryAfter time.Time - serverInfoRetryDelay time.Duration - azConfig string - deployment string + serverInfoMu sync.Mutex + serverInfoLoaded bool } // serverInfoRetryDelayDefault is the minimum wait before re-walking the @@ -570,9 +572,9 @@ func (c *Client) extractManagedInstanceSKUs(capabilities armsql.LocationCapabili // SQLPricing contains pricing information for SQL Database. type SQLPricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } // getSQLPricing gets real pricing from Azure Retail Prices API. diff --git a/providers/azure/services/internal/reservations/displayname.go b/providers/azure/services/internal/reservations/displayname.go index 97b55d88..4210b9ba 100644 --- a/providers/azure/services/internal/reservations/displayname.go +++ b/providers/azure/services/internal/reservations/displayname.go @@ -52,6 +52,13 @@ func SanitizeDisplayName(s string) string { // deterministic assertions. Production callers leave randSource nil (the // builder then uses crypto/rand) and pass time.Now(). type DisplayNameFields struct { + + // Now is the timestamp baseline. Tests inject a fixed value for + // determinism; production callers should pass time.Now(). A zero + // time.Time is replaced with time.Now() by the builder so the + // timestamp segment never emits the placeholder "00010101T000000". + Now time.Time + // Service is a short identifier for the Azure service, e.g. "vm", // "redis", "cosmos", "sql", "search". Set per-call by the service // client; the builder treats it as opaque and sanitizes it. @@ -63,9 +70,6 @@ type DisplayNameFields struct { // ResourceType is the Azure SKU name (e.g. "Standard_D2a_v4"). ResourceType string - // Count is the reservation quantity. Always rendered as "{N}x". - Count int - // Term is the commitment term, normalized to "1yr" / "3yr" by upstream // recommendation parsers. Pass through as-is; the builder collapses // it to "1yr"/"3yr" when possible, and sanitizes otherwise. @@ -77,16 +81,13 @@ type DisplayNameFields struct { // "monthly") so the segment stays under 8 chars. Payment string - // Now is the timestamp baseline. Tests inject a fixed value for - // determinism; production callers should pass time.Now(). A zero - // time.Time is replaced with time.Now() by the builder so the - // timestamp segment never emits the placeholder "00010101T000000". - Now time.Time - // randSource is an optional 4-byte source for the random suffix. // When nil (production), the builder reads from crypto/rand. Tests set // it via WithRandSource to make output deterministic. randSource []byte + + // Count is the reservation quantity. Always rendered as "{N}x". + Count int } // WithRandSource returns a copy of f with the given bytes used as the diff --git a/providers/azure/services/internal/reservations/purchase.go b/providers/azure/services/internal/reservations/purchase.go index 9ff4912f..3bc3e115 100644 --- a/providers/azure/services/internal/reservations/purchase.go +++ b/providers/azure/services/internal/reservations/purchase.go @@ -445,7 +445,8 @@ func doCalculatePrice(ctx context.Context, httpClient HTTPClient, calcURL string // the list-reservation-orders endpoint. Only the fields needed for the // idempotency-token lookup are decoded. type reservationOrdersListResponse struct { - Value []struct { + NextLink string `json:"nextLink"` + Value []struct { // Name is the reservation order GUID; this is what Azure mints as // reservationOrderId during calculatePrice and what DoPurchaseTwoStep // returns on success. @@ -468,7 +469,6 @@ type reservationOrdersListResponse struct { DisplayName string `json:"displayName"` } `json:"properties"` } `json:"value"` - NextLink string `json:"nextLink"` } // orderDisposition says what an existing reservation order that carries the diff --git a/providers/azure/services/internal/reservations/purchasecommitment.go b/providers/azure/services/internal/reservations/purchasecommitment.go index b1def490..7d81c32d 100644 --- a/providers/azure/services/internal/reservations/purchasecommitment.go +++ b/providers/azure/services/internal/reservations/purchasecommitment.go @@ -15,10 +15,10 @@ import ( // PurchaseParams holds what differs between the Azure service clients when // they build a reservation purchase body. type PurchaseParams struct { - // ReservedResourceType is the properties.reservedResourceType value. - ReservedResourceType string // DisplayName returns properties.displayName for the recommendation. DisplayName func(common.Recommendation) string + // ReservedResourceType is the properties.reservedResourceType value. + ReservedResourceType string } // BuildPurchaseBody parses rec.Term, builds the calculatePrice/purchase body diff --git a/providers/azure/services/internal/reservations/reservationstest/capture.go b/providers/azure/services/internal/reservations/reservationstest/capture.go index 685cc756..c697359e 100644 --- a/providers/azure/services/internal/reservations/reservationstest/capture.go +++ b/providers/azure/services/internal/reservations/reservationstest/capture.go @@ -18,14 +18,14 @@ import ( // Capture is an HTTP client that records the calculatePrice request body and // answers every call with a minimal success response. It performs no network IO. type Capture struct { - mu sync.Mutex - Calls int + // FailWhen, when set, is consulted after the request is recorded. A + // non-nil return is returned from Do as the transport error. + FailWhen func(*Request) error BodyBytes []byte // Requests lists every request received, in order. Requests []Request - // FailWhen, when set, is consulted after the request is recorded. A - // non-nil return is returned from Do as the transport error. - FailWhen func(*Request) error + Calls int + mu sync.Mutex } // Request is one recorded HTTP request. @@ -98,10 +98,10 @@ func (c *Capture) BodyWithoutDisplayName() ([]byte, error) { // CountingCred is an azcore.TokenCredential that counts GetToken calls and can // fail on demand. type CountingCred struct { - mu sync.Mutex - Calls int // Err, when non-nil, is returned from every GetToken call. - Err error + Err error + Calls int + mu sync.Mutex } var _ azcore.TokenCredential = (*CountingCred)(nil) diff --git a/providers/azure/services/managedredis/client.go b/providers/azure/services/managedredis/client.go index e258fcea..95551369 100644 --- a/providers/azure/services/managedredis/client.go +++ b/providers/azure/services/managedredis/client.go @@ -63,12 +63,12 @@ type RedisCachesPager interface { // provider-dispatch level. type Client struct { cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager redisCachesPager RedisCachesPager + subscriptionID string + region string } // recommendationsListArgs builds the (scope, options) for the Consumption @@ -495,9 +495,9 @@ func (c *Client) commonSKUs() []string { // RedisPricing holds pricing details for a given Redis Cache SKU. type RedisPricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } func (c *Client) getRedisPricing(ctx context.Context, sku, region string, termYears int) (*RedisPricing, error) { diff --git a/providers/azure/services/savingsplans/benefit_recommendations.go b/providers/azure/services/savingsplans/benefit_recommendations.go index d779160b..de9e364c 100644 --- a/providers/azure/services/savingsplans/benefit_recommendations.go +++ b/providers/azure/services/savingsplans/benefit_recommendations.go @@ -50,25 +50,25 @@ const ( ) type benefitRecsPage struct { - Value []benefitRecommendation `json:"value"` NextLink string `json:"nextLink"` + Value []benefitRecommendation `json:"value"` } type benefitRecommendation struct { + Properties *benefitRecommendationProps `json:"properties"` ID string `json:"id"` Kind string `json:"kind"` - Properties *benefitRecommendationProps `json:"properties"` } type benefitRecommendationProps struct { + TotalHours *int32 `json:"totalHours"` + CostWithoutBenefit *float64 `json:"costWithoutBenefit"` + RecommendationDetails *benefitRecommendDetls `json:"recommendationDetails"` ArmSkuName string `json:"armSkuName"` Term string `json:"term"` Scope string `json:"scope"` CommitmentGranularity string `json:"commitmentGranularity"` CurrencyCode string `json:"currencyCode"` - TotalHours *int32 `json:"totalHours"` - CostWithoutBenefit *float64 `json:"costWithoutBenefit"` - RecommendationDetails *benefitRecommendDetls `json:"recommendationDetails"` } type benefitRecommendDetls struct { diff --git a/providers/azure/services/savingsplans/client.go b/providers/azure/services/savingsplans/client.go index beb53986..7c85790f 100644 --- a/providers/azure/services/savingsplans/client.go +++ b/providers/azure/services/savingsplans/client.go @@ -62,7 +62,7 @@ type HTTPClient interface { } // Client handles Azure Savings Plans. -type Client struct { +type Client struct { //nolint:govet // fieldalignment: file owned by open PR #491 cred azcore.TokenCredential subscriptionID string region string diff --git a/providers/azure/services/search/client.go b/providers/azure/services/search/client.go index 78b38cdb..f00ecde0 100644 --- a/providers/azure/services/search/client.go +++ b/providers/azure/services/search/client.go @@ -54,12 +54,12 @@ type ServicesPager interface { // Client handles Azure Cognitive Search Reserved Capacity. type Client struct { cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager searchServicesPager ServicesPager + subscriptionID string + region string } // NewClient creates a new Azure Search client. @@ -116,8 +116,8 @@ func (c *Client) GetRegion() string { // AzureRetailPrice represents pricing information from Azure Retail Prices API. type AzureRetailPrice struct { - Items []pricing.RetailPriceItem `json:"Items"` NextPageLink string `json:"NextPageLink"` + Items []pricing.RetailPriceItem `json:"Items"` Count int `json:"Count"` } @@ -428,9 +428,9 @@ func (c *Client) getCommonSKUs() []string { // Pricing contains pricing information for Azure Search. type Pricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } var searchDisplaySKUs = map[string]string{ diff --git a/providers/azure/services/synapse/client.go b/providers/azure/services/synapse/client.go index b4a03b3c..a56f317a 100644 --- a/providers/azure/services/synapse/client.go +++ b/providers/azure/services/synapse/client.go @@ -58,11 +58,11 @@ type ReservationsDetailsPager interface { // Client handles Azure Synapse Analytics Reserved Capacity. type Client struct { cred azcore.TokenCredential - subscriptionID string - region string httpClient HTTPClient recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager + subscriptionID string + region string } // NewClient creates a new Azure Synapse Analytics client. @@ -116,8 +116,6 @@ func (c *Client) GetRegion() string { // Synapse Analytics. Private pricing selection uses the shared item shape. type RetailPriceItem struct { CurrencyCode string `json:"currencyCode"` - RetailPrice float64 `json:"retailPrice"` - UnitPrice float64 `json:"unitPrice"` ArmRegionName string `json:"armRegionName"` ProductName string `json:"productName"` ServiceName string `json:"serviceName"` @@ -126,6 +124,8 @@ type RetailPriceItem struct { SKUName string `json:"skuName"` ReservationTerm string `json:"reservationTerm"` Type string `json:"type"` + RetailPrice float64 `json:"retailPrice"` + UnitPrice float64 `json:"unitPrice"` } // GetRecommendations retrieves Synapse reservation recommendations from the @@ -440,9 +440,9 @@ func (c *Client) GetValidResourceTypes(_ context.Context) ([]string, error) { // Pricing holds pricing information for Synapse Analytics. type Pricing struct { + Currency string HourlyRate float64 ReservationPrice float64 - Currency string } // getSynapsePricing fetches pricing from the Azure Retail Prices API. From fdbe489d0c6ec0834f466b10afef9d9e6733672e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 17:57:35 +0200 Subject: [PATCH 2/2] chore(lint): keep field comments attached after fieldalignment reorder in providers/azure Rebuild each reordered struct body so every field keeps its own doc comment directly above it and group headers head only the fields they describe (repeated per contiguous run where the layout splits a group). Documented fields are blank-line separated so no undocumented field sits under another field comment. Part of #100 Co-Authored-By: Claude Opus 5.5 --- .../internal/recommendations/converter.go | 12 ++++++--- providers/azure/mocks/pricing_http.go | 3 +-- providers/azure/provider.go | 22 ++++++++-------- providers/azure/recommendations.go | 1 + .../recommendations_multi_subscription.go | 2 ++ providers/azure/services/cache/client.go | 10 +++++++- providers/azure/services/compute/client.go | 14 +++++++++-- providers/azure/services/compute/exchange.go | 1 - .../services/compute/exchange_operations.go | 1 - providers/azure/services/cosmosdb/client.go | 7 +++--- providers/azure/services/database/client.go | 25 ++++++++++++++++--- .../internal/reservations/displayname.go | 1 - .../reservations/purchasecommitment.go | 1 + .../reservations/reservationstest/capture.go | 12 ++++++--- 14 files changed, 78 insertions(+), 34 deletions(-) diff --git a/providers/azure/internal/recommendations/converter.go b/providers/azure/internal/recommendations/converter.go index c13a46a1..838e1f40 100644 --- a/providers/azure/internal/recommendations/converter.go +++ b/providers/azure/internal/recommendations/converter.go @@ -40,9 +40,11 @@ type ExtractedFields struct { // ExpandConsumptionVariants sets it per payment variant (0 for upfront, // Count x reservation price / term months for monthly). RecurringMonthlyCost *float64 - Region string - ResourceType string - Term string + + Region string + ResourceType string + Term string + // Scope is populated from the API response ("Shared" or a subscription ID) // but is not yet threaded into the purchase body. All service clients // currently send armreservations.AppliedScopeTypeShared, which is correct because @@ -50,11 +52,13 @@ type ExtractedFields struct { // "properties/scope eq 'Shared'" — only Shared-scope recommendations are // ever requested. This field is retained for future wiring if single- // subscription scoped recommendations are ever requested. - Scope string + Scope string + Count int OnDemandCost float64 CommitmentCost float64 EstimatedSavings float64 + // LookBackDays is the lookback window the cost fields were computed over; // set only by ExtractConsumption (0 from plain Extract). LookBackDays int diff --git a/providers/azure/mocks/pricing_http.go b/providers/azure/mocks/pricing_http.go index ce275670..4acf7bab 100644 --- a/providers/azure/mocks/pricing_http.go +++ b/providers/azure/mocks/pricing_http.go @@ -20,8 +20,7 @@ type PricingHTTP struct { FailMatch string Items []map[string]any calls int - - mu sync.Mutex + mu sync.Mutex } // Do implements the HTTPClient interface the service clients accept. diff --git a/providers/azure/provider.go b/providers/azure/provider.go index b3c2a62f..eccd2f4c 100644 --- a/providers/azure/provider.go +++ b/providers/azure/provider.go @@ -99,15 +99,8 @@ type Provider struct { credErr error subscriptionsClient SubscriptionsClient credProvider CredentialProvider - - accountsSF singleflight.Group - - // cache subsystem implementation (getOrFetchAccounts, fetchAccounts, - // cloneAccounts, InvalidateAccountsCache) lives in accounts_cache.go. - subscriptionID string - region string // Default region for operations - cachedAccounts []common.Account - accountsGen uint64 + subscriptionID string + region string // Default region for operations // accountsMu guards cachedAccounts. GetAccounts, GetServiceClient, and // GetRecommendationsClient all resolve the subscription list on the hot @@ -127,8 +120,15 @@ type Provider struct { // singleflight call and gates the cache write, so an ARM fetch that // started before an invalidation can neither publish its stale snapshot // nor be joined by a caller that arrived after it. - accountsMu sync.RWMutex - credOnce sync.Once + accountsSF singleflight.Group + cachedAccounts []common.Account + accountsGen uint64 + accountsMu sync.RWMutex + + credOnce sync.Once + + // cache subsystem implementation (getOrFetchAccounts, fetchAccounts, + // cloneAccounts, InvalidateAccountsCache) lives in accounts_cache.go. } // NewAzureProvider creates a new Azure provider instance. diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go index 519b6b71..36a1daf8 100644 --- a/providers/azure/recommendations.go +++ b/providers/azure/recommendations.go @@ -305,6 +305,7 @@ type serviceResult struct { err error name string recs []common.Recommendation + // attempted records whether the service call was actually launched // (i.e. not skipped by the params service filter). Skipped services // carry nil recs and nil err, which is indistinguishable from a diff --git a/providers/azure/recommendations_multi_subscription.go b/providers/azure/recommendations_multi_subscription.go index 847a7aec..c8f7a2d2 100644 --- a/providers/azure/recommendations_multi_subscription.go +++ b/providers/azure/recommendations_multi_subscription.go @@ -95,12 +95,14 @@ func AsPartialResult(err error) *PartialResultError { type PartialSubscriptionFailureError struct { // Failed carries every subscription that errored, with its cause. Failed []SubscriptionFailure + // Attempted is how many subscriptions the sweep was supposed to cover: // the ones actually queried plus any named by AccountFilter that the // principal cannot see (those are never queried, but they were asked // for, so leaving them out of the denominator would under-report the // gap this error exists to surface). Attempted int + // Succeeded is how many returned a result. Always < Attempted and > 0: // an all-failed sweep is a plain error, not a partial one. Succeeded int diff --git a/providers/azure/services/cache/client.go b/providers/azure/services/cache/client.go index 259cc346..198a26c7 100644 --- a/providers/azure/services/cache/client.go +++ b/providers/azure/services/cache/client.go @@ -75,7 +75,15 @@ type Client struct { reservationsPager ReservationsDetailsPager redisCachesPager RedisCachesPager - skuCacheMap map[string]redisSKUEntry + // Lazy SKU catalog cache. armredis exposes no per-region "list all + // possible SKUs" surface, so we derive shard counts from the existing + // caches in the subscription (NewListBySubscriptionPager). Fetched + // ONCE per client lifetime; subsequent converter calls in the same + // GetRecommendations run hit the in-memory map. A failed fetch leaves + // skuCacheMap nil and converters fall back to Shards=0 with a WARN + // log — the conversion itself does NOT fail. + skuCacheMap map[string]redisSKUEntry + subscriptionID string region string diff --git a/providers/azure/services/compute/client.go b/providers/azure/services/compute/client.go index 98234656..1b30578f 100644 --- a/providers/azure/services/compute/client.go +++ b/providers/azure/services/compute/client.go @@ -94,6 +94,7 @@ type Client struct { recommendationsPager RecommendationsPager resourceSKUsPager ResourceSKUsPager + // Microsoft.Capacity provider registration check (cached per client lifetime) capacityProviderErr error // Optional injected pager for ListExchangeableReservations. When nil @@ -109,6 +110,14 @@ type Client struct { // SetInventoryFactories to run hermetically without Azure credentials. inventoryFactories *InventoryFactories + // Lazy SKU catalog cache. armcompute.ResourceSKUsClient.NewListPager + // returns every SKU available to the subscription with its + // Capabilities (vCPUs, MemoryGB). Fetched ONCE per client lifetime; + // subsequent converter calls in the same GetRecommendations run hit + // the in-memory map. A failed fetch leaves skuCacheMap nil and + // converters fall back to VCPU=0/MemoryGB=0 with a WARN log — the + // conversion itself does NOT fail (graceful-degradation contract, + // matches cache/cosmosdb/database from PR #81). skuCacheMap map[string]vmSKUEntry // Optional injected LRO callers for CalculateExchange and @@ -124,8 +133,9 @@ type Client struct { // fake HTTP transport so the production LRO wrappers and the SDK's own // request serialization run without reaching Azure. exchangeClientOptions *arm.ClientOptions - subscriptionID string - region string + + subscriptionID string + region string // Microsoft.Capacity provider registration check (cached per client lifetime) capacityProviderOnce sync.Once diff --git a/providers/azure/services/compute/exchange.go b/providers/azure/services/compute/exchange.go index ac56a3ae..0c0a32c5 100644 --- a/providers/azure/services/compute/exchange.go +++ b/providers/azure/services/compute/exchange.go @@ -28,7 +28,6 @@ import ( // ExchangeableReservation represents an Azure VM reservation that is // eligible for exchange. type ExchangeableReservation struct { - // ExpiryDate is when the reservation expires. Zero if not set by Azure. ExpiryDate time.Time `json:"expiry_date,omitempty"` diff --git a/providers/azure/services/compute/exchange_operations.go b/providers/azure/services/compute/exchange_operations.go index 1e3a3bf7..b519da5c 100644 --- a/providers/azure/services/compute/exchange_operations.go +++ b/providers/azure/services/compute/exchange_operations.go @@ -33,7 +33,6 @@ import ( // CompatibleOffering describes one candidate target SKU that Azure priced as // an exchange destination for the source reservations. type CompatibleOffering struct { - // BillingCurrencyTotal is the net amount the customer would pay in their // billing currency for this offering. Nil when Azure did not report an // amount (never coerced to 0 -- absent is not the same as free). diff --git a/providers/azure/services/cosmosdb/client.go b/providers/azure/services/cosmosdb/client.go index 3b2d44c8..29a0e32a 100644 --- a/providers/azure/services/cosmosdb/client.go +++ b/providers/azure/services/cosmosdb/client.go @@ -72,10 +72,8 @@ type Client struct { recommendationsPager RecommendationsPager reservationsPager ReservationsDetailsPager cosmosAccountsPager CosmosAccountsPager - - subscriptionID string - region string - apiType string + subscriptionID string + region string // Lazy account-API-type cache. Cosmos DB has no per-SKU APIType (the // API type lives on the account, not the reservation SKU which is @@ -84,6 +82,7 @@ type Client struct { // dominant APIType the converter can apply to recommendations. // "Dominant" = the only API type observed if there's exactly one, // else empty (meaning: subscription is multi-API, can't infer). + apiType string apiTypeOnce sync.Once } diff --git a/providers/azure/services/database/client.go b/providers/azure/services/database/client.go index e29f078c..27b44ff0 100644 --- a/providers/azure/services/database/client.go +++ b/providers/azure/services/database/client.go @@ -85,7 +85,14 @@ type SQLManagedInstancesPager interface { // Client handles Azure SQL Database Reserved Capacity. type Client struct { - serverInfoRetryAfter time.Time + // Lazy server-info cache. Populated by fetchServerInfo, which walks the + // managed-instances and servers pagers. Both azConfig and deployment are + // derived in a single pass so the injected test pager is consumed once. + // Only a complete walk is cached (serverInfoLoaded); a failed or + // canceled walk is retried, but not before serverInfoRetryAfter so a + // persistently failing subscription is not re-walked per recommendation. + serverInfoRetryAfter time.Time + cred azcore.TokenCredential httpClient HTTPClient recommendationsPager RecommendationsPager @@ -94,10 +101,22 @@ type Client struct { serversPager SQLServersPager managedInstancesPager SQLManagedInstancesPager + // Lazy SKU catalog cache. Populated once on the first + // recommendation conversion in this client's GetRecommendations + // call — single ListByLocation per region instead of N+1 per rec. + // A failed fetch leaves skuCacheMap nil; converters then fall back + // to empty EngineVersion with a one-time WARN log. skuCacheMap map[string]sqlSKUEntry - subscriptionID string - region string + subscriptionID string + region string + + // Lazy server-info cache. Populated by fetchServerInfo, which walks the + // managed-instances and servers pagers. Both azConfig and deployment are + // derived in a single pass so the injected test pager is consumed once. + // Only a complete walk is cached (serverInfoLoaded); a failed or + // canceled walk is retried, but not before serverInfoRetryAfter so a + // persistently failing subscription is not re-walked per recommendation. azConfig string deployment string serverInfoRetryDelay time.Duration diff --git a/providers/azure/services/internal/reservations/displayname.go b/providers/azure/services/internal/reservations/displayname.go index 4210b9ba..0767c173 100644 --- a/providers/azure/services/internal/reservations/displayname.go +++ b/providers/azure/services/internal/reservations/displayname.go @@ -52,7 +52,6 @@ func SanitizeDisplayName(s string) string { // deterministic assertions. Production callers leave randSource nil (the // builder then uses crypto/rand) and pass time.Now(). type DisplayNameFields struct { - // Now is the timestamp baseline. Tests inject a fixed value for // determinism; production callers should pass time.Now(). A zero // time.Time is replaced with time.Now() by the builder so the diff --git a/providers/azure/services/internal/reservations/purchasecommitment.go b/providers/azure/services/internal/reservations/purchasecommitment.go index 7d81c32d..54d8a708 100644 --- a/providers/azure/services/internal/reservations/purchasecommitment.go +++ b/providers/azure/services/internal/reservations/purchasecommitment.go @@ -17,6 +17,7 @@ import ( type PurchaseParams struct { // DisplayName returns properties.displayName for the recommendation. DisplayName func(common.Recommendation) string + // ReservedResourceType is the properties.reservedResourceType value. ReservedResourceType string } diff --git a/providers/azure/services/internal/reservations/reservationstest/capture.go b/providers/azure/services/internal/reservations/reservationstest/capture.go index c697359e..cbeba983 100644 --- a/providers/azure/services/internal/reservations/reservationstest/capture.go +++ b/providers/azure/services/internal/reservations/reservationstest/capture.go @@ -20,12 +20,15 @@ import ( type Capture struct { // FailWhen, when set, is consulted after the request is recorded. A // non-nil return is returned from Do as the transport error. - FailWhen func(*Request) error + FailWhen func(*Request) error + BodyBytes []byte + // Requests lists every request received, in order. Requests []Request - Calls int - mu sync.Mutex + + Calls int + mu sync.Mutex } // Request is one recorded HTTP request. @@ -99,7 +102,8 @@ func (c *Capture) BodyWithoutDisplayName() ([]byte, error) { // fail on demand. type CountingCred struct { // Err, when non-nil, is returned from every GetToken call. - Err error + Err error + Calls int mu sync.Mutex }