Skip to content

fix(aws): request daily reservation coverage by instance filter #160

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cristim merged 1 commit into from
Sep 29, 2026
Merged

fix(aws): request daily reservation coverage by instance filter #160

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
69 changes: 16 additions & 53 deletions providers/aws/recommendations/usage_history.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package recommendations
import (
"context"
"fmt"
"strings"
"time"

"github.com/aws/aws-sdk-go-v2/aws"
Expand Down Expand Up @@ -33,7 +32,7 @@ const usageHistoryLookbackDays = 7
//
// serviceFilter is the canonical CE SERVICE dimension value (e.g.
// "Amazon Elastic Compute Cloud - Compute"). The resourceType is matched
// against the INSTANCE_TYPE dimension so we only pick up coverage for the
// by the INSTANCE_TYPE dimension so we only pick up coverage for the
// exact SKU the recommendation targets.
func (c *Client) GetDailyUsagePcts(ctx context.Context, serviceFilter, resourceType, region string) ([]float64, error) {
if serviceFilter == "" || resourceType == "" || region == "" {
Expand Down Expand Up @@ -83,14 +82,8 @@ func (c *Client) fetchDailyCoverage(ctx context.Context, serviceFilter, resource
End: aws.String(end.Format("2006-01-02")),
},
Granularity: types.GranularityDaily,
GroupBy: []types.GroupDefinition{
{
Type: types.GroupDefinitionTypeDimension,
Key: aws.String(string(types.DimensionInstanceType)),
},
},
Filter: dailyUsageFilter(serviceFilter, region),
Metrics: []string{"Hour"},
Filter: dailyUsageFilter(serviceFilter, resourceType, region),
Metrics: []string{"Hour"},
}

anyData := false
Expand All @@ -101,7 +94,7 @@ func (c *Client) fetchDailyCoverage(ctx context.Context, serviceFilter, resource
if err != nil {
return false, fmt.Errorf("failed to get daily coverage for %s/%s/%s: %w", serviceFilter, resourceType, region, err)
}
if applyPeriodsToDayMap(result.CoveragesByTime, resourceType, dayPct) {
if applyPeriodsToDayMap(result.CoveragesByTime, dayPct) {
anyData = true
}
if result.NextPageToken == nil || *result.NextPageToken == "" {
Expand All @@ -112,28 +105,19 @@ func (c *Client) fetchDailyCoverage(ctx context.Context, serviceFilter, resource
return anyData, nil
}

// applyPeriodsToDayMap writes CE coverage percentages from periods into dayPct,
// matching only the given resourceType. Reports whether any data was written.
func applyPeriodsToDayMap(periods []types.CoverageByTime, resourceType string, dayPct map[string]float64) bool {
func applyPeriodsToDayMap(periods []types.CoverageByTime, dayPct map[string]float64) bool {
anyData := false
for _, period := range periods {
if period.TimePeriod == nil || period.TimePeriod.Start == nil {
continue
}
day := aws.ToString(period.TimePeriod.Start)
for _, group := range period.Groups {
instType := extractInstanceTypeAttr(group.Attributes)
if !strings.EqualFold(instType, resourceType) {
continue
}
if group.Coverage == nil || group.Coverage.CoverageHours == nil ||
group.Coverage.CoverageHours.CoverageHoursPercentage == nil {
continue
}
pct := parseFloat(aws.ToString(group.Coverage.CoverageHours.CoverageHoursPercentage))
dayPct[day] = pct
anyData = true
if period.Total == nil || period.Total.CoverageHours == nil ||
period.Total.CoverageHours.CoverageHoursPercentage == nil {
continue
}
dayPct[day] = parseFloat(aws.ToString(period.Total.CoverageHours.CoverageHoursPercentage))
anyData = true
}
return anyData
}
Expand Down Expand Up @@ -193,31 +177,10 @@ func (c *Client) AttachDailyUsageHistory(ctx context.Context, recs []common.Reco
}
}

// dailyUsageFilter builds a CE filter that scopes GetReservationCoverage to
// a single (service, region) tuple for the daily-sparkline fetch.
func dailyUsageFilter(serviceFilter, region string) *types.Expression {
return &types.Expression{
And: []types.Expression{
{Dimensions: &types.DimensionValues{
Key: types.DimensionService,
Values: []string{serviceFilter},
}},
{Dimensions: &types.DimensionValues{
Key: types.DimensionRegion,
Values: []string{region},
}},
},
}
}

// extractInstanceTypeAttr extracts the INSTANCE_TYPE value from CE's
// Attributes map. CE encodes the key in camelCase ("instanceType"); we
// lower-case both sides to match extractGroupAttributes in coverage.go.
func extractInstanceTypeAttr(attrs map[string]string) string {
for k, v := range attrs {
if strings.ToLower(strings.ReplaceAll(k, "_", "")) == "instancetype" {
return v
}
}
return ""
func dailyUsageFilter(serviceFilter, resourceType, region string) *types.Expression {
filter := serviceRegionFilter(serviceFilter, region)
filter.And = append(filter.And, types.Expression{Dimensions: &types.DimensionValues{
Key: types.DimensionInstanceType, Values: []string{resourceType},
}})
return filter
}
132 changes: 115 additions & 17 deletions providers/aws/recommendations/usage_history_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ package recommendations

import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"sync/atomic"
"testing"
"time"

Expand All @@ -14,11 +19,111 @@ import (
"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
)

// buildDailyOutput constructs a GetReservationCoverageOutput that has one
// CoverageByTime entry per day in the last usageHistoryLookbackDays window,
// each reporting 100% coverage for instType. Used to exercise the happy path
// without hitting AWS.
func buildDailyOutput(instType string) *costexplorer.GetReservationCoverageOutput {
func TestAttachDailyUsageHistory_SDKDailyCoverage(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name string
mode string
want []float64
}{
{name: "paginated daily totals", want: []float64{10, 0, 30, 0, 0, 0, 70}},
{name: "no data", mode: "empty"},
{name: "missing total", mode: "missing"},
{name: "service error", mode: "error"},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
var calls atomic.Int32
var firstWindow *types.DateInterval
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
call := calls.Add(1)
var input costexplorer.GetReservationCoverageInput
if err := json.NewDecoder(r.Body).Decode(&input); err != nil {
t.Errorf("decode request: %v", err)
http.Error(w, "invalid JSON", http.StatusBadRequest)
return
}
valid := assert.Equal(t, "AWSInsightsIndexService.GetReservationCoverage", r.Header.Get("X-Amz-Target"))
valid = assert.Equal(t, types.GranularityDaily, input.Granularity) && valid
valid = assert.Empty(t, input.GroupBy) && valid
valid = assert.Equal(t, []string{"Hour"}, input.Metrics) && valid
valid = assert.Equal(t, &types.Expression{And: []types.Expression{
{Dimensions: &types.DimensionValues{Key: types.DimensionService, Values: []string{"Amazon Elastic Compute Cloud - Compute"}}},
{Dimensions: &types.DimensionValues{Key: types.DimensionRegion, Values: []string{"us-east-1"}}},
{Dimensions: &types.DimensionValues{Key: types.DimensionInstanceType, Values: []string{"m5.xlarge"}}},
}}, input.Filter) && valid
if !valid || input.TimePeriod == nil {
http.Error(w, `{"__type":"ValidationException","message":"invalid coverage request"}`, http.StatusBadRequest)
return
}
if call == 1 {
firstWindow = input.TimePeriod
assert.Empty(t, aws.ToString(input.NextPageToken))
} else {
assert.Equal(t, firstWindow, input.TimePeriod)
assert.Equal(t, "page2", aws.ToString(input.NextPageToken))
}
start, err := time.Parse("2006-01-02", aws.ToString(input.TimePeriod.Start))
if !assert.NoError(t, err) {
http.Error(w, "invalid date", http.StatusBadRequest)
return
}
assert.Equal(t, start.AddDate(0, 0, usageHistoryLookbackDays).Format("2006-01-02"), aws.ToString(input.TimePeriod.End))
w.Header().Set("Content-Type", "application/x-amz-json-1.1")
if tc.mode == "error" {
w.WriteHeader(http.StatusBadRequest)
_, _ = w.Write([]byte(`{"__type":"DataUnavailableException","message":"no coverage available"}`))
return
}
periods := []types.CoverageByTime{}
var token *string
if tc.mode != "empty" {
days := []int{6, 0}
if call == 1 && tc.mode == "" {
token = aws.String("page2")
} else if call == 2 {
days = []int{2}
}
for _, day := range days {
period := types.CoverageByTime{TimePeriod: &types.DateInterval{
Start: aws.String(start.AddDate(0, 0, day).Format("2006-01-02")),
End: aws.String(start.AddDate(0, 0, day+1).Format("2006-01-02")),
}}
if tc.mode != "missing" {
period.Total = &types.Coverage{CoverageHours: &types.CoverageHours{
CoverageHoursPercentage: aws.String(fmt.Sprint((day + 1) * 10)),
}}
}
periods = append(periods, period)
}
}
assert.NoError(t, json.NewEncoder(w).Encode(map[string]any{
"CoveragesByTime": periods, "NextPageToken": token,
"Total": &types.Coverage{CoverageHours: &types.CoverageHours{CoverageHoursPercentage: aws.String("99")}},
}))
}))
defer server.Close()
sdk := costexplorer.New(costexplorer.Options{
Region: "us-east-1", BaseEndpoint: aws.String(server.URL),
Credentials: aws.AnonymousCredentials{}, HTTPClient: server.Client(),
})
client := NewClientWithAPI(sdk, "us-east-1")
rec := common.Recommendation{Service: common.ServiceEC2, Region: "us-east-1", ResourceType: "m5.xlarge"}
recs := []common.Recommendation{rec, rec}
client.AttachDailyUsageHistory(context.Background(), recs)
for _, r := range recs {
assert.Equal(t, tc.want, r.UsageHistory)
}
wantCalls := int32(1)
if tc.mode == "" {
wantCalls = 2
}
assert.Equal(t, wantCalls, calls.Load())
})
}
}

func buildDailyOutput() *costexplorer.GetReservationCoverageOutput {
now := time.Now().UTC()
start := now.AddDate(0, 0, -usageHistoryLookbackDays)
periods := make([]types.CoverageByTime, 0, usageHistoryLookbackDays)
Expand All @@ -29,16 +134,9 @@ func buildDailyOutput(instType string) *costexplorer.GetReservationCoverageOutpu
Start: aws.String(day.Format("2006-01-02")),
End: aws.String(day.AddDate(0, 0, 1).Format("2006-01-02")),
},
Groups: []types.ReservationCoverageGroup{
{
Attributes: map[string]string{
"instanceType": instType,
},
Coverage: &types.Coverage{
CoverageHours: &types.CoverageHours{
CoverageHoursPercentage: aws.String("80.0"),
},
},
Total: &types.Coverage{
CoverageHours: &types.CoverageHours{
CoverageHoursPercentage: aws.String("80.0"),
},
},
})
Expand All @@ -51,7 +149,7 @@ func buildDailyOutput(instType string) *costexplorer.GetReservationCoverageOutpu
// when CE reports coverage for every day.
func TestGetDailyUsagePcts_ReturnsNDailyPoints(t *testing.T) {
mock := &mockCoverageCE{
coverageOutput: buildDailyOutput("m5.large"),
coverageOutput: buildDailyOutput(),
}

client := NewClientWithAPI(mock, "us-east-1")
Expand Down Expand Up @@ -102,7 +200,7 @@ func TestGetDailyUsagePcts_EmptyInputsReturnNil(t *testing.T) {
// coverage percentages returned by CE.
func TestAttachDailyUsageHistory_PopulatesUsageHistory(t *testing.T) {
mock := &mockCoverageCE{
coverageOutput: buildDailyOutput("m5.xlarge"),
coverageOutput: buildDailyOutput(),
}

client := NewClientWithAPI(mock, "us-east-1")
Expand Down
Loading