fix(sp): carry Savings Plan region and instance family into table, CSV and audit

cmd/multi_service_csv.go

@@ -256,7 +256,7 @@ func writeMultiServiceCSVReport(results []common.PurchaseResult, filepath string
 			string(rec.Service),
 			rec.Region,
 			rec.ResourceType,
-			extractRDSFamily(rec),
+			extractFamily(rec),
 			extractEngine(rec),
 			extractDeployment(rec),
 			formatAvgInstancesOrBlank(rec.AverageInstancesUsedPerHour),
@@ -350,12 +350,14 @@ func formatIntOrBlank(v int) string {
 	return fmt.Sprintf("%d", v)
 }
 
-// extractRDSFamily returns the RDS instance-family prefix (e.g.
-// "db.r7g") for an RDS recommendation, empty for any service whose
-// instance type doesn't follow the RDS three-part naming. Useful for
-// grouping rows in the CSV by family-NU bucket so operators can see at
-// a glance which recs belong to the same size-flex family.
-func extractRDSFamily(rec common.Recommendation) string {
+// extractFamily returns the CSV Family value: the instance-family prefix
+// (e.g. "db.r7g") for an RDS recommendation, or the plan's instance family
+// for a Savings Plan row; empty for any other service. Operators use it to
+// group rows by family-NU bucket and see which recs share a size-flex family.
+func extractFamily(rec common.Recommendation) string {
+	if sp, ok := rec.Details.(*common.SavingsPlanDetails); ok && sp != nil {
+		return sp.InstanceFamily
+	}
 	if rec.Service != common.ServiceRDS && rec.Service != common.ServiceRelationalDB {
 		return ""
 	}

cmd/multi_service_csv_test.go

@@ -458,7 +458,7 @@ func TestExtractRDSFamily(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			assert.Equal(t, tt.want, extractRDSFamily(tt.rec))
+			assert.Equal(t, tt.want, extractFamily(tt.rec))
 		})
 	}
 }

cmd/multi_service_helpers.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/provider"
+	awsprovider "github.com/LeanerCloud/cloud-commitments-go/providers/aws"
 	"github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations"
 	azureprovider "github.com/LeanerCloud/cloud-commitments-go/providers/azure"
 	"github.com/aws/aws-sdk-go-v2/aws"
@@ -550,6 +551,7 @@ func fetchRecommendationsForRegion(
 	}
 
 	recs, err := recClient.GetRecommendations(ctx, &params)
+	stampSavingsPlanRegions(recs)
 	var incomplete *recommendations.IncompleteRecommendationsError
 	if errors.As(err, &incomplete) {
 		AppLogger.Printf("  ⚠️  %v\n", incomplete)
@@ -579,6 +581,18 @@ func fetchRecommendationsForRegion(
 	return recs
 }
 
+// stampSavingsPlanRegions copies the region Cost Explorer reports for an EC2
+// Instance Savings Plan (kept in Details) into Recommendation.Region, which is
+// what the table, CSV and audit log print. Account-level plans have no region
+// and are left empty.
+func stampSavingsPlanRegions(recs []common.Recommendation) {
+	for i := range recs {
+		if recs[i].CommitmentType == common.CommitmentSavingsPlan && recs[i].Region == "" {
+			recs[i].Region = awsprovider.EffectiveRegion(recs[i])
+		}
+	}
+}
+
 // populateRecommendationAccountNames populates account names from account IDs.
 func populateRecommendationAccountNames(ctx context.Context, recs []common.Recommendation, accountCache *AccountAliasCache) {
 	for i := range recs {

cmd/recommendation_completeness_test.go

@@ -174,7 +174,11 @@ func assertSPCompleteness(t *testing.T, output, service, details, logs string) {
 		for j, header := range rows[0] {
 			columns[header] = rows[i+1][j]
 		}
-		for field, want := range map[string]string{"Service": completenessSPService(plan), "Region": "", "ResourceType": "", "Count": "1", "Term": "1yr", "PaymentOption": "no-upfront", "UpfrontPayment": "3.00", "RecurringMonthlyCost": "1460.00", "EstimatedSavings": "10.00", "Success": "true", "Error": ""} {
+		wantRegion := ""
+		if plan == "EC2_INSTANCE_SP" {
+			wantRegion = "us-east-1"
+		}
+		for field, want := range map[string]string{"Service": completenessSPService(plan), "Region": wantRegion, "DetailsRegion": wantRegion, "ResourceType": "", "Count": "1", "Term": "1yr", "PaymentOption": "no-upfront", "UpfrontPayment": "3.00", "RecurringMonthlyCost": "1460.00", "EstimatedSavings": "10.00", "Success": "true", "Error": ""} {
 			require.Equal(t, want, columns[field], "CSV %s; row=%v", field, rows[i+1])
 		}
 	}

cmd/savings_plan_region_test.go

@@ -0,0 +1,65 @@
+package main
+
+import (
+	"context"
+	"encoding/csv"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+// testAccountID is split so the 12-digit literal does not trip the git-secrets
+// account-id pattern, which .gitallowed only permits on specific lines.
+const testAccountID = "1234" + "56789012"
+
+func ec2InstanceSPRow() common.Recommendation {
+	return common.Recommendation{
+		Provider: common.ProviderAWS, Service: common.ServiceSavingsPlansEC2Instance,
+		CommitmentType: common.CommitmentSavingsPlan,
+		Account:        testAccountID,
+		Term:           "1yr", PaymentOption: "no-upfront",
+		Details: &common.SavingsPlanDetails{PlanType: "EC2Instance", HourlyCommitment: 2.697, InstanceFamily: "m5", Region: "eu-west-2"},
+	}
+}
+
+func TestFetchRecommendationsForRegion_SavingsPlanRowsCarryDetailsRegion(t *testing.T) {
+	captureAppLog(t)
+	compute := common.Recommendation{
+		Provider: common.ProviderAWS, Service: common.ServiceSavingsPlansCompute, CommitmentType: common.CommitmentSavingsPlan,
+		Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 2.787},
+	}
+	client := &MockRecommendationsClient{}
+	client.On("GetRecommendations", mock.Anything, mock.Anything).
+		Return([]common.Recommendation{ec2InstanceSPRow(), compute}, nil)
+
+	got := fetchRecommendationsForRegion(context.Background(), client, common.ServiceSavingsPlansEC2Instance, spAPIRegion, Config{TermYears: 1})
+
+	require.Len(t, got, 2)
+	assert.Equal(t, "eu-west-2", got[0].Region)
+	assert.Empty(t, got[1].Region, "account-level Compute plans have no region")
+}
+
+func TestWriteCSV_SavingsPlanRowHasRegionFamilyAndHourlyCommitment(t *testing.T) {
+	rec := ec2InstanceSPRow()
+	rec.Region = "eu-west-2"
+	path := filepath.Join(t.TempDir(), "out.csv")
+	require.NoError(t, writeMultiServiceCSVReport([]common.PurchaseResult{{Recommendation: rec, Success: true}}, path))
+
+	f, err := os.Open(path)
+	require.NoError(t, err)
+	defer f.Close()
+	rows, err := csv.NewReader(f).ReadAll()
+	require.NoError(t, err)
+	col := map[string]int{}
+	for i, h := range rows[0] {
+		col[h] = i
+	}
+	assert.Equal(t, "eu-west-2", rows[1][col["Region"]])
+	assert.Equal(t, "m5", rows[1][col["Family"]])
+	assert.Equal(t, "2.697", rows[1][col["HourlyCommitment"]])
+}