diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go index 62f0ded3..ee898e8a 100644 --- a/providers/azure/recommendations.go +++ b/providers/azure/recommendations.go @@ -22,6 +22,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/cosmosdb" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/database" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/savingsplans" + "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/synapse" ) // serviceRecsGetter is the narrow interface satisfied by each per-service @@ -59,6 +60,9 @@ var ( newSavingsPlansClientFn func(azcore.TokenCredential, string, string) serviceRecsGetter = func(cred azcore.TokenCredential, sub, region string) serviceRecsGetter { return savingsplans.NewClient(cred, sub, region) } + newSynapseClientFn func(azcore.TokenCredential, string, string) serviceRecsGetter = func(cred azcore.TokenCredential, sub, region string) serviceRecsGetter { + return synapse.NewClient(cred, sub, region) + } ) // RecommendationsClientAdapter aggregates Azure reservation recommendations across all services. @@ -106,17 +110,17 @@ func NewRecommendationsClientAdapter(cred azcore.TokenCredential, subscriptionID // (see known_issues/10_azure_provider.md CRITICAL "Recommendation converters // ignore the API response entirely" for the matching converter work). // -// All six service calls run concurrently under errgroup. Each goroutine captures +// All seven service calls run concurrently under errgroup. Each goroutine captures // its own error and returns nil to the group so that a single service failure // does not cancel sibling calls. Results are appended in a deterministic order -// (compute → database → cache → cosmosdb → savingsplans → advisor) after all goroutines finish. +// (compute → database → cache → cosmosdb → synapse → savingsplans → advisor) after all goroutines finish. func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, params *common.RecommendationParams) ([]common.Recommendation, error) { if params == nil { return nil, fmt.Errorf("params cannot be nil") } var ( - computeRecs, dbRecs, cacheRecs, cosmosRecs, advisorRecs, spRecs []common.Recommendation - computeErr, dbErr, cacheErr, cosmosErr, advisorErr, spErr error + computeRecs, dbRecs, cacheRecs, cosmosRecs, advisorRecs, spRecs, synapseRecs []common.Recommendation + computeErr, dbErr, cacheErr, cosmosErr, advisorErr, spErr, synapseErr error ) g, gctx := errgroup.WithContext(ctx) @@ -154,6 +158,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p includeCache := shouldIncludeService(*params, common.ServiceCache) includeCosmos := shouldIncludeService(*params, common.ServiceNoSQL) includeSP := shouldIncludeService(*params, common.ServiceSavingsPlansAll) + includeSynapse := shouldIncludeService(*params, common.ServiceDataWarehouse) // Compute (VM) recommendations — subscription-wide. if includeCompute { @@ -187,6 +192,14 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p }) } + // Synapse (SqlDataWarehouse) recommendations — subscription-wide. Search and + // Managed Redis are deliberately not collected here: see #47. + if includeSynapse { + goService(&synapseErr, func() { + synapseRecs, synapseErr = newSynapseClientFn(r.cred, r.subscriptionID, "").GetRecommendations(gctx, params) + }) + } + // Savings Plans — Azure has no stable public API for SP purchase // recommendations (Benefits Recommendations API is still in preview). // The call returns an empty slice so the service appears in the fan-out @@ -205,10 +218,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // // getAdvisorRecsFn defaults to r.getAdvisorRecommendations and may be // replaced per-adapter in tests to avoid real ARM network calls. - advisorFn := r.getAdvisorRecsFn - if advisorFn == nil { - advisorFn = r.getAdvisorRecommendations - } + advisorFn := r.advisorFn() goService(&advisorErr, func() { advisorRecs, advisorErr = collectAdvisorOrSkip(gctx, advisorFn, *params) }) @@ -229,6 +239,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p serviceResult{"database", dbRecs, dbErr, includeDB}, serviceResult{"cache", cacheRecs, cacheErr, includeCache}, serviceResult{"cosmosdb", cosmosRecs, cosmosErr, includeCosmos}, + serviceResult{"synapse", synapseRecs, synapseErr, includeSynapse}, // The savingsplans client is a stub that unconditionally returns // ([], nil) until the Benefits Recommendations API stabilizes (see // services/savingsplans Client.GetRecommendations). Counting its @@ -291,6 +302,14 @@ func dropCoveredAdvisorSavingsPlans(advisor, sp []common.Recommendation) []commo return out } +// advisorFn returns the injected Advisor collector, or the real one. +func (r *RecommendationsClientAdapter) advisorFn() func(context.Context, common.RecommendationParams) ([]common.Recommendation, error) { + if r.getAdvisorRecsFn != nil { + return r.getAdvisorRecsFn + } + return r.getAdvisorRecommendations +} + // serviceResult bundles a per-service collection outcome for the deterministic // merge in mergeServiceResults. Extracted into a helper so GetRecommendations // stays under the cyclomatic-complexity gate after the post-Wait ctx.Err() @@ -310,7 +329,7 @@ type serviceResult struct { // mergeServiceResults logs per-service errors (matches the previous sequential // behavior where each error was logged inline via logging.Warnf) and appends // successful results in the order the slice is passed — callers must preserve -// the canonical compute → database → cache → cosmosdb → savingsplans → advisor +// the canonical compute → database → cache → cosmosdb → synapse → savingsplans → advisor // order so that order-sensitive consumers remain stable. The advisor entry's // error is logged via logging.Errorf to match the pre-parallelisation severity. // diff --git a/providers/azure/recommendations_advisor_test.go b/providers/azure/recommendations_advisor_test.go index e0ffcd5f..f31c191c 100644 --- a/providers/azure/recommendations_advisor_test.go +++ b/providers/azure/recommendations_advisor_test.go @@ -155,6 +155,21 @@ func TestGetRecommendations_AdvisorSavingsPlanDedupedAgainstBenefitRecs(t *testi assert.Len(t, got, 1, "Advisor SP advice is kept when the benefit source returns nothing") } +func TestGetRecommendations_CollectsSynapseForDataWarehouse(t *testing.T) { + overrideServiceClientFns(t, &fakeServiceClient{}, &fakeServiceClient{}, &fakeServiceClient{}, &fakeServiceClient{}, &fakeServiceClient{}) + want := common.Recommendation{Provider: common.ProviderAzure, Service: common.ServiceDataWarehouse} + newSynapseClientFn = newFakeFn(&fakeServiceClient{recs: []common.Recommendation{want}}) + a := &RecommendationsClientAdapter{cred: &mockAzureTokenCredential{}, subscriptionID: "sub", getAdvisorRecsFn: noopAdvisorFn} + + recs, err := a.GetRecommendations(context.Background(), &common.RecommendationParams{}) + require.NoError(t, err) + assert.Equal(t, []common.Recommendation{want}, recs) + + recs, err = a.GetRecommendations(context.Background(), &common.RecommendationParams{Service: common.ServiceCompute}) + require.NoError(t, err) + assert.Empty(t, recs, "synapse is skipped when another service is requested") +} + func TestConvertAdvisor_UnmappedReservedResourceTypeIsIgnoredNotSkipped(t *testing.T) { a := &RecommendationsClientAdapter{subscriptionID: "sub"} ctx, skips := azrecs.WithSkipLog(context.Background()) diff --git a/providers/azure/recommendations_test.go b/providers/azure/recommendations_test.go index f2d7ad0c..11dadace 100644 --- a/providers/azure/recommendations_test.go +++ b/providers/azure/recommendations_test.go @@ -207,7 +207,9 @@ func overrideServiceClientFns(t *testing.T, compute, database, cache, cosmos, sp origCache := newCacheClientFn origCosmos := newCosmosDBClientFn origSP := newSavingsPlansClientFn + origSynapse := newSynapseClientFn t.Cleanup(func() { + newSynapseClientFn = origSynapse newComputeClientFn = origCompute newDatabaseClientFn = origDatabase newCacheClientFn = origCache @@ -219,6 +221,7 @@ func overrideServiceClientFns(t *testing.T, compute, database, cache, cosmos, sp newCacheClientFn = newFakeFn(cache) newCosmosDBClientFn = newFakeFn(cosmos) newSavingsPlansClientFn = newFakeFn(sp) + newSynapseClientFn = newFakeFn(&fakeServiceClient{}) } func TestRecommendationsClientAdapter_GetRecommendationsForService(t *testing.T) { @@ -580,6 +583,7 @@ func TestGetRecommendations_Parallelism(t *testing.T) { origDatabase := newDatabaseClientFn origCache := newCacheClientFn origCosmos := newCosmosDBClientFn + origSynapse := newSynapseClientFn origSP := newSavingsPlansClientFn t.Cleanup(func() { newComputeClientFn = origCompute @@ -587,6 +591,7 @@ func TestGetRecommendations_Parallelism(t *testing.T) { newCacheClientFn = origCache newCosmosDBClientFn = origCosmos newSavingsPlansClientFn = origSP + newSynapseClientFn = origSynapse }) rec := func(svc common.ServiceType) common.Recommendation { @@ -599,6 +604,7 @@ func TestGetRecommendations_Parallelism(t *testing.T) { newCacheClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, recs: []common.Recommendation{rec(common.ServiceCache)}}) newCosmosDBClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, recs: []common.Recommendation{rec(common.ServiceNoSQL)}}) newSavingsPlansClientFn = noopFake + newSynapseClientFn = newFakeFn(&fakeServiceClient{}) adapter := &RecommendationsClientAdapter{ cred: &mockAzureTokenCredential{}, @@ -631,6 +637,7 @@ func TestGetRecommendations_OrderPreservation(t *testing.T) { origDatabase := newDatabaseClientFn origCache := newCacheClientFn origCosmos := newCosmosDBClientFn + origSynapse := newSynapseClientFn origSP := newSavingsPlansClientFn t.Cleanup(func() { newComputeClientFn = origCompute @@ -638,6 +645,7 @@ func TestGetRecommendations_OrderPreservation(t *testing.T) { newCacheClientFn = origCache newCosmosDBClientFn = origCosmos newSavingsPlansClientFn = origSP + newSynapseClientFn = origSynapse }) makeRec := func(svc common.ServiceType) common.Recommendation { @@ -652,6 +660,7 @@ func TestGetRecommendations_OrderPreservation(t *testing.T) { newCacheClientFn = newFakeFn(&fakeServiceClient{sleepDur: 20 * time.Millisecond, recs: []common.Recommendation{makeRec(common.ServiceCache)}}) newCosmosDBClientFn = newFakeFn(&fakeServiceClient{sleepDur: 10 * time.Millisecond, recs: []common.Recommendation{makeRec(common.ServiceNoSQL)}}) newSavingsPlansClientFn = newFakeFn(&fakeServiceClient{}) + newSynapseClientFn = newFakeFn(&fakeServiceClient{}) adapter := &RecommendationsClientAdapter{ cred: &mockAzureTokenCredential{}, @@ -682,6 +691,7 @@ func TestGetRecommendations_ErrorIsolation(t *testing.T) { origDatabase := newDatabaseClientFn origCache := newCacheClientFn origCosmos := newCosmosDBClientFn + origSynapse := newSynapseClientFn origSP := newSavingsPlansClientFn t.Cleanup(func() { newComputeClientFn = origCompute @@ -689,6 +699,7 @@ func TestGetRecommendations_ErrorIsolation(t *testing.T) { newCacheClientFn = origCache newCosmosDBClientFn = origCosmos newSavingsPlansClientFn = origSP + newSynapseClientFn = origSynapse }) makeRec := func(svc common.ServiceType) common.Recommendation { @@ -701,6 +712,7 @@ func TestGetRecommendations_ErrorIsolation(t *testing.T) { newCacheClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, recs: []common.Recommendation{makeRec(common.ServiceCache)}}) newCosmosDBClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, recs: []common.Recommendation{makeRec(common.ServiceNoSQL)}}) newSavingsPlansClientFn = newFakeFn(&fakeServiceClient{}) + newSynapseClientFn = newFakeFn(&fakeServiceClient{}) adapter := &RecommendationsClientAdapter{ cred: &mockAzureTokenCredential{},