Skip to content

fix(cli): count every Savings Plan and RI type in the run summary #2239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cristim merged 1 commit into from
Oct 16, 2026
Merged

fix(cli): count every Savings Plan and RI type in the run summary #2239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 82 additions & 49 deletions cmd/multi_service_stats_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,44 +1,80 @@
package main

import (
"strings"

"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
)

// SPTypeBreakdown holds savings information broken down by Savings Plan type.
// Other collects every Savings Plan recommendation whose type is not one of
// the four known plan types, so the per-type counts and savings always add up
// to the number of Savings Plan recommendations.
type SPTypeBreakdown struct {
ComputeSavings float64
EC2InstanceSavings float64
SageMakerSavings float64
DatabaseSavings float64
OtherSavings float64
ComputeCount int
EC2InstanceCount int
SageMakerCount int
DatabaseCount int
OtherCount int
}

// spPlanKind resolves a Savings Plan recommendation to one of the four
// per-plan-type service slugs, or ServiceSavingsPlansAll when it is unknown.
// The per-plan-type slug on the recommendation wins (the AWS parser and the
// purchase path both key on it); the umbrella slug falls back to
// SavingsPlanDetails.PlanType, matched case-insensitively because CSV input is
// hand-edited.
func spPlanKind(rec common.Recommendation) common.ServiceType {
for _, slug := range common.SavingsPlansPlanTypes() {
if rec.Service == slug {
return slug
}
}
if details, ok := rec.Details.(*common.SavingsPlanDetails); ok && details != nil {
switch strings.ToLower(strings.TrimSpace(details.PlanType)) {
case "compute":
return common.ServiceSavingsPlansCompute
case "ec2instance":
return common.ServiceSavingsPlansEC2Instance
case "sagemaker":
return common.ServiceSavingsPlansSageMaker
case "database":
return common.ServiceSavingsPlansDatabase
}
}
return common.ServiceSavingsPlansAll
}

// categorizeSPRecommendations categorizes Savings Plan recommendations by type.
func categorizeSPRecommendations(recommendations []common.Recommendation) SPTypeBreakdown {
breakdown := SPTypeBreakdown{}

for _rvc := range recommendations {
rec := recommendations[_rvc]
if common.IsSavingsPlan(rec.Service) {
if details, ok := rec.Details.(*common.SavingsPlanDetails); ok {
switch details.PlanType {
case "Compute":
breakdown.ComputeSavings += rec.EstimatedSavings
breakdown.ComputeCount++
case "EC2Instance":
breakdown.EC2InstanceSavings += rec.EstimatedSavings
breakdown.EC2InstanceCount++
case "SageMaker":
breakdown.SageMakerSavings += rec.EstimatedSavings
breakdown.SageMakerCount++
case "Database":
breakdown.DatabaseSavings += rec.EstimatedSavings
breakdown.DatabaseCount++
}
}
for i := range recommendations {
rec := recommendations[i]
if !common.IsSavingsPlan(rec.Service) {
continue
}
switch spPlanKind(rec) {
case common.ServiceSavingsPlansCompute:
breakdown.ComputeSavings += rec.EstimatedSavings
breakdown.ComputeCount++
case common.ServiceSavingsPlansEC2Instance:
breakdown.EC2InstanceSavings += rec.EstimatedSavings
breakdown.EC2InstanceCount++
case common.ServiceSavingsPlansSageMaker:
breakdown.SageMakerSavings += rec.EstimatedSavings
breakdown.SageMakerCount++
case common.ServiceSavingsPlansDatabase:
breakdown.DatabaseSavings += rec.EstimatedSavings
breakdown.DatabaseCount++
default:
breakdown.OtherSavings += rec.EstimatedSavings
breakdown.OtherCount++
}
}

Expand All @@ -63,6 +99,10 @@ func printSPTypeSummaries(breakdown SPTypeBreakdown) {
AppLogger.Printf(" Database SP | Recs: %3d | Covers: RDS, Aurora, ElastiCache, etc. | $%8.2f/mo\n",
breakdown.DatabaseCount, breakdown.DatabaseSavings)
}
if breakdown.OtherCount > 0 {
AppLogger.Printf(" Other SP | Recs: %3d | Unrecognized plan type | $%8.2f/mo\n",
breakdown.OtherCount, breakdown.OtherSavings)
}
}

// printBestSPOptions prints the best Savings Plan options by category.
Expand Down Expand Up @@ -91,32 +131,23 @@ func printBestSPOptions(breakdown SPTypeBreakdown) {

// SPSavingsByType holds Savings Plan savings categorized by plan type.
type SPSavingsByType struct {
EC2SPSavings float64
ComputeSPSavings float64
DatabaseSPSavings float64
EC2SPSavings float64
ComputeSPSavings float64
DatabaseSPSavings float64
SageMakerSPSavings float64
OtherSPSavings float64
}

// collectSPSavings collects Savings Plan savings by type.
func collectSPSavings(recommendations []common.Recommendation) SPSavingsByType {
savings := SPSavingsByType{}

for _rvc := range recommendations {
rec := recommendations[_rvc]
if common.IsSavingsPlan(rec.Service) {
if details, ok := rec.Details.(*common.SavingsPlanDetails); ok {
switch details.PlanType {
case "EC2Instance":
savings.EC2SPSavings += rec.EstimatedSavings
case "Compute":
savings.ComputeSPSavings += rec.EstimatedSavings
case "Database":
savings.DatabaseSPSavings += rec.EstimatedSavings
}
}
}
b := categorizeSPRecommendations(recommendations)
return SPSavingsByType{
EC2SPSavings: b.EC2InstanceSavings,
ComputeSPSavings: b.ComputeSavings,
DatabaseSPSavings: b.DatabaseSavings,
SageMakerSPSavings: b.SageMakerSavings,
OtherSPSavings: b.OtherSavings,
}

return savings
}

// RISavingsByService holds Reserved Instance savings categorized by service.
Expand All @@ -125,19 +156,21 @@ type RISavingsByService struct {
DBRISavings float64
}

// collectRISavings collects Reserved Instance savings by service.
// collectRISavings collects Reserved Instance savings by service. Both the
// legacy service constants and the canonical ones (a CSV Service column is
// taken verbatim) are recognized.
func collectRISavings(riStats map[common.ServiceType]ServiceProcessingStats) RISavingsByService {
savings := RISavingsByService{}

// EC2 RIs
if stats, ok := riStats[common.ServiceEC2]; ok {
savings.EC2RISavings = stats.TotalEstimatedSavings
}

// Database RIs (RDS, ElastiCache, MemoryDB, Redshift)
for service, stats := range riStats {
if service == common.ServiceRDS || service == common.ServiceElastiCache ||
service == common.ServiceMemoryDB || service == common.ServiceRedshift {
switch service {
case common.ServiceEC2, common.ServiceCompute:
savings.EC2RISavings += stats.TotalEstimatedSavings
// Database RIs (RDS, ElastiCache, MemoryDB, Redshift)
case common.ServiceRDS, common.ServiceRelationalDB,
common.ServiceElastiCache, common.ServiceCache,
common.ServiceMemoryDB,
common.ServiceRedshift, common.ServiceDataWarehouse:
savings.DBRISavings += stats.TotalEstimatedSavings
}
}
Expand Down
102 changes: 102 additions & 0 deletions cmd/multi_service_stats_plan_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package main

import (
"testing"

"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
"github.com/stretchr/testify/assert"
)

func spRec(service common.ServiceType, planType string, savings float64) common.Recommendation {
rec := common.Recommendation{Service: service, EstimatedSavings: savings}
if planType != "" {
rec.Details = &common.SavingsPlanDetails{PlanType: planType}
}
return rec
}

func TestSPSummaryCountsEveryPlanType(t *testing.T) {
tests := []struct {
name string
rec common.Recommendation
want SPTypeBreakdown
}{
{"parser compute", spRec(common.ServiceSavingsPlansCompute, "Compute", 10), SPTypeBreakdown{ComputeCount: 1, ComputeSavings: 10}},
{"parser ec2instance", spRec(common.ServiceSavingsPlansEC2Instance, "EC2Instance", 10), SPTypeBreakdown{EC2InstanceCount: 1, EC2InstanceSavings: 10}},
{"parser sagemaker", spRec(common.ServiceSavingsPlansSageMaker, "SageMaker", 10), SPTypeBreakdown{SageMakerCount: 1, SageMakerSavings: 10}},
{"parser database", spRec(common.ServiceSavingsPlansDatabase, "Database", 10), SPTypeBreakdown{DatabaseCount: 1, DatabaseSavings: 10}},
{"umbrella slug compute", spRec(common.ServiceSavingsPlansAll, "Compute", 10), SPTypeBreakdown{ComputeCount: 1, ComputeSavings: 10}},
{"umbrella slug sagemaker", spRec(common.ServiceSavingsPlansAll, "SageMaker", 10), SPTypeBreakdown{SageMakerCount: 1, SageMakerSavings: 10}},
{"umbrella slug database", spRec(common.ServiceSavingsPlansAll, "Database", 10), SPTypeBreakdown{DatabaseCount: 1, DatabaseSavings: 10}},
{"csv lowercase plan type", spRec(common.ServiceSavingsPlansAll, "sagemaker", 10), SPTypeBreakdown{SageMakerCount: 1, SageMakerSavings: 10}},
{"canonical slug without details", spRec(common.ServiceSavingsPlansDatabase, "", 10), SPTypeBreakdown{DatabaseCount: 1, DatabaseSavings: 10}},
{"unknown plan type", spRec(common.ServiceSavingsPlansAll, "Quantum", 10), SPTypeBreakdown{OtherCount: 1, OtherSavings: 10}},
{"umbrella without details", spRec(common.ServiceSavingsPlansAll, "", 10), SPTypeBreakdown{OtherCount: 1, OtherSavings: 10}},
{"non-SP service ignored", spRec(common.ServiceRDS, "", 10), SPTypeBreakdown{}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
recs := []common.Recommendation{tt.rec}
assert.Equal(t, tt.want, categorizeSPRecommendations(recs))

b := tt.want
assert.Equal(t, SPSavingsByType{
EC2SPSavings: b.EC2InstanceSavings, ComputeSPSavings: b.ComputeSavings,
DatabaseSPSavings: b.DatabaseSavings, SageMakerSPSavings: b.SageMakerSavings,
OtherSPSavings: b.OtherSavings,
}, collectSPSavings(recs))
})
}
}

func TestSPSummaryTotalsAddUp(t *testing.T) {
recs := []common.Recommendation{
spRec(common.ServiceSavingsPlansCompute, "Compute", 1),
spRec(common.ServiceSavingsPlansEC2Instance, "EC2Instance", 2),
spRec(common.ServiceSavingsPlansSageMaker, "SageMaker", 4),
spRec(common.ServiceSavingsPlansDatabase, "Database", 8),
spRec(common.ServiceSavingsPlansAll, "Mystery", 16),
spRec(common.ServiceRDS, "", 100),
}
b := categorizeSPRecommendations(recs)
assert.Equal(t, 5, b.ComputeCount+b.EC2InstanceCount+b.SageMakerCount+b.DatabaseCount+b.OtherCount)
assert.InDelta(t, 31, b.ComputeSavings+b.EC2InstanceSavings+b.SageMakerSavings+b.DatabaseSavings+b.OtherSavings, 1e-9)

s := collectSPSavings(recs)
assert.InDelta(t, 31, s.ComputeSPSavings+s.EC2SPSavings+s.SageMakerSPSavings+s.DatabaseSPSavings+s.OtherSPSavings, 1e-9)
}

func TestPrintSPTypeSummariesShowsOther(t *testing.T) {
out := captureAppOutput(t, func() {
printSPTypeSummaries(SPTypeBreakdown{OtherCount: 2, OtherSavings: 12.5})
})
assert.Contains(t, out, "Other SP")
assert.Contains(t, out, "12.50")
}

func TestCollectRISavingsLegacyAndCanonical(t *testing.T) {
tests := []struct {
service common.ServiceType
wantEC2 float64
wantDB float64
}{
{common.ServiceEC2, 5, 0},
{common.ServiceCompute, 5, 0},
{common.ServiceRDS, 0, 5},
{common.ServiceRelationalDB, 0, 5},
{common.ServiceElastiCache, 0, 5},
{common.ServiceCache, 0, 5},
{common.ServiceMemoryDB, 0, 5},
{common.ServiceRedshift, 0, 5},
{common.ServiceDataWarehouse, 0, 5},
{common.ServiceOpenSearch, 0, 0},
}
for _, tt := range tests {
t.Run(string(tt.service), func(t *testing.T) {
got := collectRISavings(map[common.ServiceType]ServiceProcessingStats{
tt.service: {TotalEstimatedSavings: 5},
})
assert.Equal(t, RISavingsByService{EC2RISavings: tt.wantEC2, DBRISavings: tt.wantDB}, got)
})
}
}
Loading