fix(gcp/computeengine): normalize recommender savings by the projection duration

providers/gcp/recommendations_sdk_test.go

@@ -22,6 +22,7 @@ import (
 	"google.golang.org/grpc/metadata"
 	"google.golang.org/grpc/status"
 	"google.golang.org/grpc/test/bufconn"
+	"google.golang.org/protobuf/types/known/durationpb"
 	"google.golang.org/protobuf/types/known/structpb"
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
@@ -237,7 +238,8 @@ func sdkCostRecommendation(savings int64, state recommenderpb.RecommendationStat
 		PrimaryImpact: &recommenderpb.Impact{
 			Category: recommenderpb.Impact_COST,
 			Projection: &recommenderpb.Impact_CostProjection{CostProjection: &recommenderpb.CostProjection{
-				Cost: &money.Money{CurrencyCode: "USD", Units: -savings},
+				Duration: durationpb.New(730 * time.Hour),
+				Cost:     &money.Money{CurrencyCode: "USD", Units: -savings},
 			}},
 		},
 	}

providers/gcp/services/computeengine/client.go

@@ -1122,7 +1122,9 @@ func (c *Client) convertGCPRecommendation(ctx context.Context, gcpRec *recommend
 			gcpRec.GetName(), err)
 	}
 
-	extractCostImpactFromRecommendation(gcpRec, rec)
+	if err := extractCostImpactFromRecommendation(gcpRec, rec); err != nil {
+		return nil, err
+	}
 	if err := populateRecommendationResources(gcpRec, rec); err != nil {
 		return nil, err
 	}
@@ -1225,27 +1227,40 @@ func machineTypeFromResourcePath(resource string) (string, bool) {
 	return machineType, true
 }
 
-// extractCostImpactFromRecommendation extracts the cost impact from a GCP recommendation.
-func extractCostImpactFromRecommendation(gcpRec *recommenderpb.Recommendation, rec *common.Recommendation) {
+// savingsWindow is the period EstimatedSavings is reported over: a 730-hour
+// month (8760 / 12), the month length used across the providers.
+const savingsWindow = 730 * time.Hour
+
+// extractCostImpactFromRecommendation extracts the cost impact from a GCP
+// recommendation and normalizes it to savingsWindow using the projection's
+// Duration. A non-zero cost with a missing or non-positive Duration is an error
+// wrapping errBadAmount: the window is unknown, so the amount cannot be trusted
+// as monthly.
+func extractCostImpactFromRecommendation(gcpRec *recommenderpb.Recommendation, rec *common.Recommendation) error {
 	if gcpRec.PrimaryImpact == nil {
-		return
+		return nil
 	}
 
 	costProj := gcpRec.PrimaryImpact.GetCostProjection()
 	if costProj == nil || costProj.Cost == nil {
-		return
+		return nil
 	}
 
 	cost := costProj.Cost
-	if cost.Units != 0 || cost.Nanos != 0 {
-		// Cost savings is the negative of the cost projection impact.
-		// GCP Recommender sizes CUD recommendations for 100% coverage of
-		// the project's historical on-demand usage; this is the 100%-coverage
-		// baseline the dashboard scaler in summarizeRecommendationsWithCoverage
-		// depends on (issue #215 audit).
-		savings := -(float64(cost.Units) + float64(cost.Nanos)/1e9)
-		rec.EstimatedSavings = savings
+	if cost.Units == 0 && cost.Nanos == 0 {
+		return nil
 	}
+	if costProj.GetDuration() == nil || costProj.GetDuration().AsDuration() <= 0 {
+		return fmt.Errorf("%w: cost projection has a non-zero cost but no positive duration", errBadAmount)
+	}
+	// Cost savings is the negative of the cost projection impact.
+	// GCP Recommender sizes CUD recommendations for 100% coverage of
+	// the project's historical on-demand usage; this is the 100%-coverage
+	// baseline the dashboard scaler in summarizeRecommendationsWithCoverage
+	// depends on (issue #215 audit).
+	projected := -(float64(cost.Units) + float64(cost.Nanos)/1e9)
+	rec.EstimatedSavings = projected * float64(savingsWindow) / float64(costProj.GetDuration().AsDuration())
+	return nil
 }
 
 // requireSingleAmountResource rejects a payload whose VCPU and MEMORY amounts

providers/gcp/services/computeengine/client_test.go

@@ -25,6 +25,7 @@ import (
 	"google.golang.org/genproto/googleapis/type/money"
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/durationpb"
 	"google.golang.org/protobuf/types/known/structpb"
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
@@ -1043,6 +1044,7 @@ func TestComputeEngineClient_GetRecommendations_WithMock(t *testing.T) {
 					Category: recommenderpb.Impact_COST,
 					Projection: &recommenderpb.Impact_CostProjection{
 						CostProjection: &recommenderpb.CostProjection{
+							Duration: durationpb.New(savingsWindow),
 							Cost: &money.Money{
 								Units:        -100,
 								Nanos:        0,
@@ -1149,6 +1151,7 @@ func TestComputeEngineClient_ConvertGCPRecommendation(t *testing.T) {
 			Category: recommenderpb.Impact_COST,
 			Projection: &recommenderpb.Impact_CostProjection{
 				CostProjection: &recommenderpb.CostProjection{
+					Duration: durationpb.New(savingsWindow),
 					Cost: &money.Money{
 						Units:        -50,
 						Nanos:        -500000000,
@@ -1213,6 +1216,7 @@ func realisticCUDRecommendation() *recommenderpb.Recommendation {
 			Category: recommenderpb.Impact_COST,
 			Projection: &recommenderpb.Impact_CostProjection{
 				CostProjection: &recommenderpb.CostProjection{
+					Duration: durationpb.New(savingsWindow),
 					Cost: &money.Money{
 						Units:        -200,
 						Nanos:        0,
@@ -1425,7 +1429,8 @@ func TestConvertGCPRecommendation_EmptyParamsDefaultsToMonthly(t *testing.T) {
 			Category: recommenderpb.Impact_COST,
 			Projection: &recommenderpb.Impact_CostProjection{
 				CostProjection: &recommenderpb.CostProjection{
-					Cost: &money.Money{Units: -100, CurrencyCode: "USD"},
+					Duration: durationpb.New(savingsWindow),
+					Cost:     &money.Money{Units: -100, CurrencyCode: "USD"},
 				},
 			},
 		},
@@ -1865,7 +1870,8 @@ func TestGetRecommendations_FiltersNonActiveStates(t *testing.T) {
 				Category: recommenderpb.Impact_COST,
 				Projection: &recommenderpb.Impact_CostProjection{
 					CostProjection: &recommenderpb.CostProjection{
-						Cost: &money.Money{Units: -100, CurrencyCode: "USD"},
+						Duration: durationpb.New(savingsWindow),
+						Cost:     &money.Money{Units: -100, CurrencyCode: "USD"},
 					},
 				},
 			},
@@ -1915,7 +1921,8 @@ func TestGetRecommendations_ActiveRecIncluded(t *testing.T) {
 					Category: recommenderpb.Impact_COST,
 					Projection: &recommenderpb.Impact_CostProjection{
 						CostProjection: &recommenderpb.CostProjection{
-							Cost: &money.Money{Units: -50, CurrencyCode: "USD"},
+							Duration: durationpb.New(savingsWindow),
+							Cost:     &money.Money{Units: -50, CurrencyCode: "USD"},
 						},
 					},
 				},
@@ -2175,7 +2182,8 @@ func commitmentOnlyCUDRecommendation() *recommenderpb.Recommendation {
 			Category: recommenderpb.Impact_COST,
 			Projection: &recommenderpb.Impact_CostProjection{
 				CostProjection: &recommenderpb.CostProjection{
-					Cost: &money.Money{Units: -200, CurrencyCode: "USD"},
+					Duration: durationpb.New(savingsWindow),
+					Cost:     &money.Money{Units: -200, CurrencyCode: "USD"},
 				},
 			},
 		},

providers/gcp/services/computeengine/savings_window_test.go

@@ -0,0 +1,75 @@
+package computeengine
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"cloud.google.com/go/recommender/apiv1/recommenderpb"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/genproto/googleapis/type/money"
+	"google.golang.org/protobuf/types/known/durationpb"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+)
+
+// Fixture-based: recommenderpb-shaped, not a captured Recommender response.
+func costImpact(cost *money.Money, duration *durationpb.Duration) *recommenderpb.Recommendation {
+	return &recommenderpb.Recommendation{
+		PrimaryImpact: &recommenderpb.Impact{
+			Category: recommenderpb.Impact_COST,
+			Projection: &recommenderpb.Impact_CostProjection{CostProjection: &recommenderpb.CostProjection{
+				Cost: cost, Duration: duration,
+			}},
+		},
+	}
+}
+
+func TestExtractCostImpact_NormalizesToMonthly(t *testing.T) {
+	cases := map[string]struct {
+		duration time.Duration
+		want     float64
+	}{
+		"a 730h month is already monthly": {730 * time.Hour, 120},
+		"a 365-day projection":            {365 * 24 * time.Hour, 120 * 730.0 / 8760},
+		"a 15-day projection":             {15 * 24 * time.Hour, 120 * 730.0 / 360},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			var rec common.Recommendation
+			err := extractCostImpactFromRecommendation(costImpact(&money.Money{Units: -120, CurrencyCode: "USD"}, durationpb.New(tc.duration)), &rec)
+			require.NoError(t, err)
+			assert.InDelta(t, tc.want, rec.EstimatedSavings, 1e-9)
+		})
+	}
+}
+
+func TestExtractCostImpact_UnknownWindowIsAnError(t *testing.T) {
+	cost := &money.Money{Units: -120, CurrencyCode: "USD"}
+	for name, duration := range map[string]*durationpb.Duration{"missing": nil, "zero": durationpb.New(0), "negative": durationpb.New(-time.Hour)} {
+		t.Run(name, func(t *testing.T) {
+			var rec common.Recommendation
+			err := extractCostImpactFromRecommendation(costImpact(cost, duration), &rec)
+			require.ErrorIs(t, err, errBadAmount)
+			assert.Zero(t, rec.EstimatedSavings)
+		})
+	}
+}
+
+func TestExtractCostImpact_ZeroCostNeedsNoDuration(t *testing.T) {
+	var rec common.Recommendation
+	require.NoError(t, extractCostImpactFromRecommendation(costImpact(&money.Money{CurrencyCode: "USD"}, nil), &rec))
+}
+
+// A recommendation without a duration is skipped, not reported as monthly.
+func TestConvertGCPRecommendation_MissingDurationSkipsRecommendation(t *testing.T) {
+	ctx := context.Background()
+	client, _ := NewClient(ctx, "test-project", "us-central1")
+	gcpRec := realisticCUDRecommendation()
+	gcpRec.PrimaryImpact.GetCostProjection().Duration = nil
+
+	converted, err := client.convertOrSkipBadAmount(ctx, gcpRec, common.RecommendationParams{})
+	require.NoError(t, err)
+	assert.Nil(t, converted)
+}