fix(recommendations): add Savings Plans to GetAllRecommendations fan-out (closes #784)

providers/aws/recommendations/client.go

@@ -254,6 +254,8 @@ func (c *Client) GetAllRecommendations(ctx context.Context) ([]common.Recommenda
 	var (
 		ec2Recs, rdsRecs, cacheRecs, osRecs, redshiftRecs []common.Recommendation
 		ec2Err, rdsErr, cacheErr, osErr, redshiftErr      error
+		spRecs                                            []common.Recommendation
+		spErr                                             error
 	)
 
 	g, gctx := errgroup.WithContext(ctx)
@@ -266,7 +268,9 @@ func (c *Client) GetAllRecommendations(ctx context.Context) ([]common.Recommenda
 	// no request is actually in flight. The Acquire/Release boundary lives
 	// inside GetRecommendations (around the individual CE SDK call), which
 	// frees slots during backoffs and gives the cap its full effective
-	// throughput. See pkg/concurrency.
+	// throughput. The SP goroutine expands further to 24 calls (2 terms × 3
+	// payment options × 4 plan types) internally via planTypesForParams.
+	// See pkg/concurrency.
 	g.Go(func() error {
 		ec2Recs, ec2Err = c.GetRecommendationsForService(gctx, common.ServiceEC2)
 		return nil
@@ -287,10 +291,14 @@ func (c *Client) GetAllRecommendations(ctx context.Context) ([]common.Recommenda
 		redshiftRecs, redshiftErr = c.GetRecommendationsForService(gctx, common.ServiceRedshift)
 		return nil
 	})
+	g.Go(func() error {
+		spRecs, spErr = c.GetRecommendationsForService(gctx, common.ServiceSavingsPlans)
+		return nil
+	})
 
 	// Wait for all goroutines. g.Wait() always returns nil because every
 	// goroutine returns nil — errors are captured per-service above. After
-	// Wait, propagate ctx cancellation so callers can distinguish "all five
+	// Wait, propagate ctx cancellation so callers can distinguish "all six
 	// services completed (with possibly per-service errors)" from "the
 	// parent ctx was canceled mid-fan-out".
 	_ = g.Wait()
@@ -304,6 +312,7 @@ func (c *Client) GetAllRecommendations(ctx context.Context) ([]common.Recommenda
 		serviceResult{name: "ElastiCache", recs: cacheRecs, err: cacheErr},
 		serviceResult{name: "OpenSearch", recs: osRecs, err: osErr},
 		serviceResult{name: "Redshift", recs: redshiftRecs, err: redshiftErr},
+		serviceResult{name: "SavingsPlans", recs: spRecs, err: spErr},
 	), nil
 }
 
@@ -319,8 +328,8 @@ type serviceResult struct {
 
 // mergeServiceResults logs per-service errors at WARN and appends successful
 // results in the order the slice is passed — callers must preserve the
-// canonical EC2 → RDS → ElastiCache → OpenSearch → Redshift order so that
-// order-sensitive consumers stay stable.
+// canonical EC2 → RDS → ElastiCache → OpenSearch → Redshift → SavingsPlans
+// order so that order-sensitive consumers stay stable.
 func mergeServiceResults(results ...serviceResult) []common.Recommendation {
 	total := 0
 	for _, r := range results {

providers/aws/recommendations/client_test.go

@@ -565,6 +565,69 @@ func TestGetAllRecommendations_PropagatesContextCancellation(t *testing.T) {
 	assert.Nil(t, recs)
 }
 
+// TestGetAllRecommendations_IncludesSavingsPlans pins the fix for issue #784:
+// GetAllRecommendations must include a Savings Plans goroutine so that SP recs
+// reach the merged output.  Without the 6th goroutine the SP mock branch is
+// never called and all SP rows are silently absent from the result.
+func TestGetAllRecommendations_IncludesSavingsPlans(t *testing.T) {
+	mockAPI := &mockCostExplorerAPI{
+		// RI services return an EC2 row so we can assert at least one EC2 rec
+		// is present alongside the SP row (cross-service non-regression).
+		riRecommendations: &costexplorer.GetReservationPurchaseRecommendationOutput{
+			Recommendations: []types.ReservationPurchaseRecommendation{
+				{
+					RecommendationDetails: []types.ReservationPurchaseRecommendationDetail{
+						{
+							RecommendedNumberOfInstancesToPurchase: aws.String("1"),
+							EstimatedMonthlySavingsAmount:          aws.String("50.00"),
+							EstimatedMonthlySavingsPercentage:      aws.String("20.0"),
+							InstanceDetails: &types.InstanceDetails{
+								EC2InstanceDetails: &types.EC2InstanceDetails{
+									InstanceType: aws.String("t3.medium"),
+									Platform:     aws.String("Linux/UNIX"),
+									Region:       aws.String("us-east-1"),
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		// SP mock returns a Compute Savings Plan row.
+		spRecommendations: &costexplorer.GetSavingsPlansPurchaseRecommendationOutput{
+			SavingsPlansPurchaseRecommendation: &types.SavingsPlansPurchaseRecommendation{
+				SavingsPlansPurchaseRecommendationDetails: []types.SavingsPlansPurchaseRecommendationDetail{
+					{
+						HourlyCommitmentToPurchase:    aws.String("1.00"),
+						EstimatedMonthlySavingsAmount: aws.String("120.00"),
+						EstimatedSavingsPercentage:    aws.String("30.0"),
+						UpfrontCost:                   aws.String("0"),
+						AccountId:                     aws.String("123456789012"),
+					},
+				},
+			},
+		},
+	}
+
+	client := NewClientWithAPI(mockAPI, "us-east-1")
+
+	recs, err := client.GetAllRecommendations(context.Background())
+	require.NoError(t, err)
+
+	// At least one EC2 rec must be present (existing services not regressed).
+	var ec2Count, spCount int
+	for _, r := range recs {
+		if r.Service == common.ServiceEC2 {
+			ec2Count++
+		}
+		if common.IsSavingsPlan(r.Service) {
+			spCount++
+		}
+	}
+	assert.Positive(t, ec2Count, "EC2 recs must be present in merged output")
+	assert.Positive(t, spCount, "Savings Plans recs must be present in merged output (issue #784 regression)")
+}
+
 // multiPageRIMock returns distinct pages for GetReservationPurchaseRecommendation
 // based on the NextPageToken in the incoming request. Implements CostExplorerAPI.
 type multiPageRIMock struct {

providers/aws/recommendations/parser_sp.go

@@ -95,6 +95,9 @@ func (c *Client) fetchSPAllPages(
 		if err != nil {
 			return nil, err
 		}
+		if result == nil {
+			break
+		}
 
 		if result.SavingsPlansPurchaseRecommendation != nil {
 			recs := c.parseSavingsPlansRecommendations(result.SavingsPlansPurchaseRecommendation, params, planType)