refactor(gcp): share term price scaling and cache the Cloud Storage catalog

CHANGELOG.md

@@ -155,6 +155,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Changed
 
+- GCP Cloud Storage pricing lists the SKU catalog once per client, like Cloud
+  SQL and Memorystore.
 - Azure multi-subscription collection keeps skipped recommendations when a
   subscription also fails. They are returned by the new
   `PartialSubscriptionFailureError.Skips()` (a `*PartialResultError`, or nil)

providers/gcp/internal/skumatch/savings.go

@@ -1,5 +1,11 @@
 package skumatch
 
+import (
+	"fmt"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+)
+
 // SavingsPercent returns the percentage a commitment saves against on-demand
 // over a term. commitmentTotal must already be a term total (per-hour price *
 // hoursInTerm), on the same basis as onDemandPrice * hoursInTerm. A zero
@@ -9,3 +15,59 @@ func SavingsPercent(onDemandPrice, hoursInTerm, commitmentTotal float64) float64
 	onDemandTotal := onDemandPrice * hoursInTerm
 	return ((onDemandTotal - commitmentTotal) / onDemandTotal) * 100
 }
+
+// TermPrices holds per-hour catalog prices scaled to a commitment term.
+type TermPrices struct {
+	HourlyRate        float64
+	CommitmentPrice   float64
+	OnDemandPrice     float64
+	SavingsPercentage float64
+}
+
+// ScaleToTerm rejects missing catalog prices and scales both per-hour prices
+// to term totals, so the savings percentage compares like with like (a
+// per-hour commitment against a term-total on-demand would read near 100%).
+func ScaleToTerm(what, region string, onDemandHourly, commitmentHourly float64, termYears int) (TermPrices, error) {
+	if onDemandHourly <= 0 {
+		return TermPrices{}, fmt.Errorf("no pricing found for %s", what)
+	}
+	if commitmentHourly <= 0 {
+		return TermPrices{}, fmt.Errorf("no commitment pricing found for %s in region %s: catalog has no CUD SKU; cannot compute savings percentage", what, region)
+	}
+	hoursInTerm := common.HoursPerYear * float64(termYears)
+	commitmentTotal := commitmentHourly * hoursInTerm
+	return TermPrices{
+		HourlyRate:        commitmentHourly,
+		CommitmentPrice:   commitmentTotal,
+		OnDemandPrice:     onDemandHourly * hoursInTerm,
+		SavingsPercentage: SavingsPercent(onDemandHourly, hoursInTerm, commitmentTotal),
+	}, nil
+}
+
+// OfferingDetails builds the offering for a commitment total, paying it up
+// front or spreading it monthly by payment option. An unknown option pays the
+// total up front.
+func OfferingDetails(rec common.Recommendation, offeringID string, termYears int, commitmentTotal, hourlyRate float64, currency string) *common.OfferingDetails {
+	var upfrontCost, recurringCost float64
+
+	switch rec.PaymentOption {
+	case "all-upfront", "upfront":
+		upfrontCost = commitmentTotal
+	case "monthly", "no-upfront":
+		recurringCost = commitmentTotal / (float64(termYears) * 12)
+	default:
+		upfrontCost = commitmentTotal
+	}
+
+	return &common.OfferingDetails{
+		OfferingID:          offeringID,
+		ResourceType:        rec.ResourceType,
+		Term:                rec.Term,
+		PaymentOption:       rec.PaymentOption,
+		UpfrontCost:         upfrontCost,
+		RecurringCost:       recurringCost,
+		TotalCost:           commitmentTotal,
+		EffectiveHourlyRate: hourlyRate,
+		Currency:            currency,
+	}
+}

providers/gcp/internal/skumatch/savings_test.go

@@ -1,9 +1,11 @@
 package skumatch
 
 import (
+	"fmt"
 	"math"
 	"testing"
 
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
 	"github.com/stretchr/testify/assert"
 	"google.golang.org/api/cloudbilling/v1"
 )
@@ -69,3 +71,76 @@ func TestMatchesTier(t *testing.T) {
 		})
 	}
 }
+
+func TestScaleToTerm(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name        string
+		onDemand    float64
+		commitment  float64
+		years       int
+		want        TermPrices
+		wantErrText string
+	}{
+		{name: "zero on-demand", onDemand: 0, commitment: 0.05, years: 1, wantErrText: "no pricing found for Thing x"},
+		{name: "negative on-demand", onDemand: -1, commitment: 0.05, years: 1, wantErrText: "no pricing found for Thing x"},
+		{name: "zero commitment", onDemand: 0.1, commitment: 0, years: 1, wantErrText: "no commitment pricing found for Thing x in region r1: catalog has no CUD SKU; cannot compute savings percentage"},
+		{name: "negative commitment", onDemand: 0.1, commitment: -0.05, years: 3, wantErrText: "no commitment pricing found for Thing x in region r1: catalog has no CUD SKU; cannot compute savings percentage"},
+		{name: "1 year", onDemand: 0.1, commitment: 0.05, years: 1, want: TermPrices{HourlyRate: 0.05, CommitmentPrice: 0.05 * 8760, OnDemandPrice: 0.1 * 8760, SavingsPercentage: 50}},
+		{name: "3 year", onDemand: 0.2, commitment: 0.1, years: 3, want: TermPrices{HourlyRate: 0.1, CommitmentPrice: 0.1 * 26280, OnDemandPrice: 0.2 * 26280, SavingsPercentage: 50}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got, err := ScaleToTerm("Thing x", "r1", tt.onDemand, tt.commitment, tt.years)
+			if tt.wantErrText != "" {
+				assert.EqualError(t, err, tt.wantErrText)
+				return
+			}
+			assert.NoError(t, err)
+			assert.InDelta(t, tt.want.HourlyRate, got.HourlyRate, 1e-9)
+			assert.InDelta(t, tt.want.CommitmentPrice, got.CommitmentPrice, 1e-6)
+			assert.InDelta(t, tt.want.OnDemandPrice, got.OnDemandPrice, 1e-6)
+			assert.InDelta(t, tt.want.SavingsPercentage, got.SavingsPercentage, 1e-9)
+		})
+	}
+}
+
+func TestOfferingDetails(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		option        string
+		years         int
+		wantUpfront   float64
+		wantRecurring float64
+	}{
+		{"all-upfront", 1, 1200, 0},
+		{"upfront", 3, 1200, 0},
+		{"monthly", 1, 0, 100},
+		{"monthly", 3, 0, 1200.0 / 36},
+		{"no-upfront", 1, 0, 100},
+		{"no-upfront", 3, 0, 1200.0 / 36},
+		{"bogus", 1, 1200, 0},
+		{"bogus", 3, 1200, 0},
+		{"", 1, 1200, 0},
+		{"", 3, 1200, 0},
+	}
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("%q/%dyr", tt.option, tt.years), func(t *testing.T) {
+			t.Parallel()
+			rec := common.Recommendation{ResourceType: "rt", Term: "term-x", PaymentOption: tt.option}
+			got := OfferingDetails(rec, "id-1", tt.years, 1200, 0.5, "EUR")
+			assert.Equal(t, &common.OfferingDetails{
+				OfferingID:          "id-1",
+				ResourceType:        "rt",
+				Term:                "term-x",
+				PaymentOption:       tt.option,
+				UpfrontCost:         tt.wantUpfront,
+				RecurringCost:       tt.wantRecurring,
+				TotalCost:           1200,
+				EffectiveHourlyRate: 0.5,
+				Currency:            "EUR",
+			}, got)
+		})
+	}
+}

providers/gcp/services/cloudsql/client.go

@@ -4,6 +4,7 @@ package cloudsql
 import (
 	"context"
 	"fmt"
+	"slices"
 	"strings"
 	"time"
 
@@ -156,10 +157,8 @@ func (c *Client) ValidateOffering(ctx context.Context, rec common.Recommendation
 		return fmt.Errorf("failed to get valid tiers: %w", err)
 	}
 
-	for _, tier := range validTiers {
-		if tier == rec.ResourceType {
-			return nil
-		}
+	if slices.Contains(validTiers, rec.ResourceType) {
+		return nil
 	}
 
 	return fmt.Errorf("invalid Cloud SQL tier: %s", rec.ResourceType)
@@ -177,31 +176,8 @@ func (c *Client) GetOfferingDetails(ctx context.Context, rec common.Recommendati
 		return nil, fmt.Errorf("failed to get pricing: %w", err)
 	}
 
-	var upfrontCost, recurringCost float64
-	totalCost := pricing.CommitmentPrice
-
-	switch rec.PaymentOption {
-	case "all-upfront", "upfront":
-		upfrontCost = totalCost
-		recurringCost = 0
-	case "monthly", "no-upfront":
-		upfrontCost = 0
-		recurringCost = totalCost / (float64(termYears) * 12)
-	default:
-		upfrontCost = totalCost
-	}
-
-	return &common.OfferingDetails{
-		OfferingID:          fmt.Sprintf("gcp-cloudsql-%s-%s-%s", rec.ResourceType, c.region, rec.Term),
-		ResourceType:        rec.ResourceType,
-		Term:                rec.Term,
-		PaymentOption:       rec.PaymentOption,
-		UpfrontCost:         upfrontCost,
-		RecurringCost:       recurringCost,
-		TotalCost:           totalCost,
-		EffectiveHourlyRate: pricing.HourlyRate,
-		Currency:            pricing.Currency,
-	}, nil
+	offeringID := fmt.Sprintf("gcp-cloudsql-%s-%s-%s", rec.ResourceType, c.region, rec.Term)
+	return skumatch.OfferingDetails(rec, offeringID, termYears, pricing.CommitmentPrice, pricing.HourlyRate, pricing.Currency), nil
 }
 
 // GetValidResourceTypes returns valid Cloud SQL tiers.
@@ -266,26 +242,17 @@ func (c *Client) getSQLPricing(ctx context.Context, tier, region string, termYea
 	if err != nil {
 		return nil, err
 	}
-	if onDemandPrice <= 0 {
-		return nil, fmt.Errorf("no pricing found for Cloud SQL tier %s", tier)
-	}
-	if commitmentPrice <= 0 {
-		return nil, fmt.Errorf("no commitment pricing found for Cloud SQL tier %s in region %s: catalog has no CUD SKU; cannot compute savings percentage", tier, region)
+	prices, err := skumatch.ScaleToTerm("Cloud SQL tier "+tier, region, onDemandPrice, commitmentPrice, termYears)
+	if err != nil {
+		return nil, err
 	}
 
-	hoursInTerm := common.HoursPerYear * float64(termYears)
-	// Scale the per-hour commitment price to a term total so it is on the
-	// same basis as onDemandPrice * hoursInTerm. Without this, the savings
-	// percentage would be nearly 100% (per-hour price vs term total).
-	commitmentPriceTerm := commitmentPrice * hoursInTerm
-	savingsPercentage := skumatch.SavingsPercent(onDemandPrice, hoursInTerm, commitmentPriceTerm)
-
 	return &SQLPricing{
-		HourlyRate:        commitmentPrice,
-		CommitmentPrice:   commitmentPriceTerm,
-		OnDemandPrice:     onDemandPrice * hoursInTerm,
+		HourlyRate:        prices.HourlyRate,
+		CommitmentPrice:   prices.CommitmentPrice,
+		OnDemandPrice:     prices.OnDemandPrice,
 		Currency:          currency,
-		SavingsPercentage: savingsPercentage,
+		SavingsPercentage: prices.SavingsPercentage,
 	}, nil
 }
 

providers/gcp/services/cloudsql/scale_errors_test.go

@@ -0,0 +1,40 @@
+package cloudsql
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/api/cloudbilling/v1"
+)
+
+// Pins the service label and region that ScaleToTerm puts in its errors.
+func TestGetSQLPricing_ScaleErrorsNameLabelAndRegion(t *testing.T) {
+	t.Parallel()
+	const region = "europe-west4"
+	tests := []struct {
+		name             string
+		onDemand, commit int64
+		wantErr          string
+	}{
+		{"no on-demand", -50000000, 42000000, "no pricing found for Cloud SQL tier db-n1-standard-1"},
+		{"no commitment", 50000000, -42000000, "no commitment pricing found for Cloud SQL tier db-n1-standard-1 in region europe-west4: catalog has no CUD SKU; cannot compute savings percentage"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			ctx := context.Background()
+			client, err := NewClient(ctx, "test-project", region)
+			require.NoError(t, err)
+			skus := negPriceSkus(tt.onDemand, tt.commit)
+			for _, sku := range skus {
+				sku.ServiceRegions = []string{region}
+			}
+			client.SetBillingService(&MockBillingService{skus: &cloudbilling.ListSkusResponse{Skus: skus}})
+
+			_, err = client.getSQLPricing(ctx, "db-n1-standard-1", region, 1)
+			assert.EqualError(t, err, tt.wantErr)
+		})
+	}
+}