Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ require (
go.opentelemetry.io/otel/metric v1.45.0 // indirect
go.opentelemetry.io/otel/trace v1.45.0 // indirect
golang.org/x/crypto v0.58.0
golang.org/x/net v0.61.0 // indirect
golang.org/x/net v0.62.0 // indirect
golang.org/x/oauth2 v0.37.0
golang.org/x/sync v0.24.0
golang.org/x/sys v0.49.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,8 @@ golang.org/x/crypto v0.58.0 h1:COkYLr4k7nDI4r6QtMML2AfzeIswWHAJthLS6K/M/54=
golang.org/x/crypto v0.58.0/go.mod h1:Xh+kl5A+M0gMiWDytH2Hhr5Z7WbNCpv/eB/EFgfSk+w=
golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg=
golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs=
golang.org/x/net v0.61.0 h1:RR+6j/BTBCrggOLugxa+66R8FvoN6MHlzDlZIbN9YFw=
golang.org/x/net v0.61.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI=
golang.org/x/net v0.62.0 h1:18ELalfM5+XNRwzM2nYqXsZQ9ByS1uhj2O/U7+5jqRY=
golang.org/x/net v0.62.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI=
golang.org/x/oauth2 v0.37.0 h1:JUlcxA8oAtauLfiH8FX2/FkAWHAdi0QtGCGc+hofE98=
golang.org/x/oauth2 v0.37.0/go.mod h1:IxwZNxUULJmpBFf9K/9NTMSIfZZuvuTy1gGxhigP/58=
golang.org/x/sync v0.24.0 h1:iqiDrGoDgyuvAKrhLQthUEzU/R378N78gXbBjxed7oA=
Expand Down
20 changes: 14 additions & 6 deletions internal/analytics/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,20 @@ type MonthlySummary struct {
// AvgCoverage is the average reservation coverage for the month.
// A nil value means coverage data is absent for this period and must
// not be treated as zero coverage by consumers.
AvgCoverage *float64 `json:"avg_coverage,omitempty"`
AccountID string `json:"account_id"`
Provider string `json:"provider"`
Service string `json:"service"`
TotalSavings float64 `json:"total_savings"`
SnapshotCount int `json:"snapshot_count"`
AvgCoverage *float64 `json:"avg_coverage,omitempty"`
// TotalCommitment and TotalUsage are the month's representative monthly
// run-rates of committed spend and covered usage: the bucket's per-
// collection totals averaged over the month, as for TotalSavings. nil
// means unknown, because at least one snapshot in the bucket had no
// figure (monthly_savings_summary propagates that NULL, migration
// 000143), and must never be read as 0.
TotalCommitment *float64 `json:"total_commitment,omitempty"`
TotalUsage *float64 `json:"total_usage,omitempty"`
AccountID string `json:"account_id"`
Provider string `json:"provider"`
Service string `json:"service"`
TotalSavings float64 `json:"total_savings"`
SnapshotCount int `json:"snapshot_count"`
}

// ProviderBreakdown represents savings breakdown by provider.
Expand Down
5 changes: 4 additions & 1 deletion internal/analytics/postgres_analytics.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ func (s *PostgresAnalyticsStore) QueryMonthlyTotals(ctx context.Context, account
accountClause, args := accountFilterClause(accountUUIDs, accountExternalIDsByProvider, []any{months})

query := `
SELECT month, account_id, cloud_account_id, provider, service, total_savings, avg_coverage, snapshot_count
SELECT month, account_id, cloud_account_id, provider, service, total_savings, avg_coverage,
total_commitment, total_usage, snapshot_count
FROM monthly_savings_summary
WHERE month >= DATE_TRUNC('month', NOW()) - make_interval(months => $1 - 1)
AND ` + accountClause + `
Expand All @@ -321,6 +322,8 @@ func (s *PostgresAnalyticsStore) QueryMonthlyTotals(ctx context.Context, account
&summary.Service,
&summary.TotalSavings,
&summary.AvgCoverage,
&summary.TotalCommitment,
&summary.TotalUsage,
&summary.SnapshotCount,
)
if err != nil {
Expand Down
62 changes: 62 additions & 0 deletions internal/analytics/postgres_analytics_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,11 @@ func TestPostgresAnalyticsStore_QueryMonthlyTotals(t *testing.T) {
assert.Equal(t, "rds", s.Service)
assert.InDelta(t, 200.00, s.TotalSavings, 0.01)
assert.Equal(t, 1, s.SnapshotCount)
// Issue #30: committed spend and usage reach the monthly series.
require.NotNil(t, s.TotalCommitment)
assert.InDelta(t, 1000.00, *s.TotalCommitment, 0.01)
require.NotNil(t, s.TotalUsage)
assert.InDelta(t, 800.00, *s.TotalUsage, 0.01)
})

t.Run("query monthly totals for non-existent account", func(t *testing.T) {
Expand All @@ -268,6 +273,63 @@ func TestPostgresAnalyticsStore_QueryMonthlyTotals(t *testing.T) {
})
}

// TestPostgresAnalyticsStore_QueryMonthlyTotals_UnknownStaysUnknown pins issue
// #30 on real Postgres: QueryMonthlyTotals returns the view's total_commitment
// and total_usage, a bucket with one region lacking a figure reads as unknown
// (nil) rather than the known regions' sum, and a known 0 stays a real 0.
func TestPostgresAnalyticsStore_QueryMonthlyTotals_UnknownStaysUnknown(t *testing.T) {
ctx := context.Background()
container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath())
store := analytics.NewPostgresAnalyticsStore(container.DB)

now := time.Now().UTC().Truncate(time.Microsecond)
const acct = "123456789012" // placeholder; a named constant keeps the line clear of the account-ID detector
snap := func(service, region string, commitment, usage *float64) *analytics.SavingsSnapshot {
return &analytics.SavingsSnapshot{
AccountID: acct, Timestamp: now, Provider: "aws", Service: service, Region: region,
CommitmentType: "RI", TotalCommitment: commitment, TotalUsage: usage, TotalSavings: 10,
}
}
for _, s := range []*analytics.SavingsSnapshot{
// rds: commitment known in both regions; usage unknown in one.
snap("rds", "us-east-1", f64ptr(300), f64ptr(250)),
snap("rds", "eu-west-1", f64ptr(100), nil),
// ec2: commitment unknown in one region; usage known in both.
snap("ec2", "us-east-1", nil, f64ptr(40)),
snap("ec2", "eu-west-1", f64ptr(70), f64ptr(60)),
// elasticache: a known zero in both columns.
snap("elasticache", "us-east-1", f64ptr(0), f64ptr(0)),
} {
require.NoError(t, store.SaveSnapshot(ctx, s))
}
require.NoError(t, store.RefreshMaterializedViews(ctx))

// Two months so a run just after a month boundary still sees the rows.
summaries, err := store.QueryMonthlyTotals(ctx, nil, map[string][]string{"aws": {"123456789012"}}, 2)
require.NoError(t, err)
byService := make(map[string]analytics.MonthlySummary, len(summaries))
for _, s := range summaries {
byService[s.Service] = s
}
require.Len(t, byService, 3)

rds := byService["rds"]
require.NotNil(t, rds.TotalCommitment)
assert.InDelta(t, 400.0, *rds.TotalCommitment, 0.001, "regions at one timestamp sum")
assert.Nil(t, rds.TotalUsage, "one region without usage makes the bucket's usage unknown, not 250")

ec2 := byService["ec2"]
assert.Nil(t, ec2.TotalCommitment, "one region without a commitment makes it unknown, not 70")
require.NotNil(t, ec2.TotalUsage)
assert.InDelta(t, 100.0, *ec2.TotalUsage, 0.001)

zero := byService["elasticache"]
require.NotNil(t, zero.TotalCommitment, "a known 0 is not unknown")
require.NotNil(t, zero.TotalUsage, "a known 0 is not unknown")
assert.Zero(t, *zero.TotalCommitment)
assert.Zero(t, *zero.TotalUsage)
}

func TestPostgresAnalyticsStore_QueryByProvider(t *testing.T) {
ctx := context.Background()

Expand Down
27 changes: 22 additions & 5 deletions internal/analytics/postgres_analytics_pgxmock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func TestPostgresAnalyticsStore_QueryMonthlyTotals_Empty(t *testing.T) {

rows := pgxmock.NewRows([]string{
"month", "account_id", "cloud_account_id", "provider", "service",
"total_savings", "avg_coverage", "snapshot_count",
"total_savings", "avg_coverage", "total_commitment", "total_usage", "snapshot_count",
})
mock.ExpectQuery(`SELECT month`).
WithArgs(anyArgs(2)...).
Expand Down Expand Up @@ -263,17 +263,34 @@ func TestPostgresAnalyticsStore_QueryMonthlyTotals_WithRows(t *testing.T) {

rows := pgxmock.NewRows([]string{
"month", "account_id", "cloud_account_id", "provider", "service",
"total_savings", "avg_coverage", "snapshot_count",
}).AddRow(time.Now(), "acct1", strPtr("cloud-1"), "aws", "ec2", 500.0, f64ptr(85.0), 10)
"total_savings", "avg_coverage", "total_commitment", "total_usage", "snapshot_count",
}).
AddRow(time.Now(), "acct1", strPtr("cloud-1"), "aws", "ec2", 500.0, f64ptr(85.0), f64ptr(720.0), f64ptr(640.0), 10).
AddRow(time.Now(), "acct1", strPtr("cloud-1"), "aws", "rds", 50.0, nil, nil, nil, 3).
AddRow(time.Now(), "acct1", strPtr("cloud-1"), "aws", "elasticache", 0.0, nil, f64ptr(0), f64ptr(0), 1)

mock.ExpectQuery(`SELECT month`).
mock.ExpectQuery(`SELECT month, .*total_commitment, total_usage, snapshot_count\s+FROM monthly_savings_summary`).
WithArgs(anyArgs(2)...).
WillReturnRows(rows)

result, err := store.QueryMonthlyTotals(ctx, []string{"acct1"}, nil, 6)
require.NoError(t, err)
require.Len(t, result, 1)
require.Len(t, result, 3)
assert.Equal(t, "aws", result[0].Provider)
assert.Equal(t, 10, result[0].SnapshotCount)
// Issue #30: committed spend and usage are scanned, a NULL stays nil
// (unknown) and a known 0 stays a non-nil 0.
require.NotNil(t, result[0].TotalCommitment)
require.NotNil(t, result[0].TotalUsage)
assert.InDelta(t, 720.0, *result[0].TotalCommitment, 0.001)
assert.InDelta(t, 640.0, *result[0].TotalUsage, 0.001)
assert.Nil(t, result[1].TotalCommitment)
assert.Nil(t, result[1].TotalUsage)
assert.Equal(t, 3, result[1].SnapshotCount)
require.NotNil(t, result[2].TotalCommitment)
require.NotNil(t, result[2].TotalUsage)
assert.Zero(t, *result[2].TotalCommitment)
assert.Zero(t, *result[2].TotalUsage)
}

func TestPostgresAnalyticsStore_QueryMonthlyTotals_ScanError(t *testing.T) {
Expand Down
38 changes: 32 additions & 6 deletions internal/analytics/postgres_analytics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,11 @@ func TestQueryMonthlyTotals(t *testing.T) {
month := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)

rows := pgxmock.NewRows([]string{
"month", "account_id", "cloud_account_id", "provider", "service", "total_savings", "avg_coverage", "snapshot_count",
"month", "account_id", "cloud_account_id", "provider", "service", "total_savings", "avg_coverage",
"total_commitment", "total_usage", "snapshot_count",
}).
AddRow(month, "account-123", strPtr("cloud-1"), "aws", "rds", 1500.0, f64ptr(85.0), 720).
AddRow(month, "account-123", strPtr("cloud-1"), "aws", "elasticache", 800.0, f64ptr(75.0), 720)
AddRow(month, "account-123", strPtr("cloud-1"), "aws", "rds", 1500.0, f64ptr(85.0), f64ptr(720.0), f64ptr(600.0), 720).
AddRow(month, "account-123", strPtr("cloud-1"), "aws", "elasticache", 800.0, f64ptr(75.0), nil, nil, 720)

mock.ExpectQuery(`SELECT month, account_id`).
WithArgs(6, []string{"account-123"}).
Expand All @@ -524,6 +525,12 @@ func TestQueryMonthlyTotals(t *testing.T) {
assert.Len(t, summaries, 2)
assert.Equal(t, "rds", summaries[0].Service)
assert.Equal(t, 1500.0, summaries[0].TotalSavings)
require.NotNil(t, summaries[0].TotalCommitment)
assert.Equal(t, 720.0, *summaries[0].TotalCommitment)
require.NotNil(t, summaries[0].TotalUsage)
assert.Equal(t, 600.0, *summaries[0].TotalUsage)
assert.Nil(t, summaries[1].TotalCommitment, "NULL stays unknown")
assert.Nil(t, summaries[1].TotalUsage, "NULL stays unknown")
assert.NoError(t, mock.ExpectationsWereMet())
})

Expand Down Expand Up @@ -552,7 +559,8 @@ func TestQueryMonthlyTotals(t *testing.T) {
store := newTestableStore(mock)

rows := pgxmock.NewRows([]string{
"month", "account_id", "cloud_account_id", "provider", "service", "total_savings", "avg_coverage", "snapshot_count",
"month", "account_id", "cloud_account_id", "provider", "service", "total_savings", "avg_coverage",
"total_commitment", "total_usage", "snapshot_count",
})

mock.ExpectQuery(`SELECT month, account_id`).
Expand Down Expand Up @@ -995,6 +1003,23 @@ func TestMonthlySummary(t *testing.T) {
assert.Equal(t, summary.Provider, unmarshaled.Provider)
assert.InDelta(t, summary.TotalSavings, unmarshaled.TotalSavings, 0.001)
})

// Issue #30: an unknown commitment or usage is omitted (never 0) and a
// known 0 is sent as 0.
t.Run("json keeps unknown commitment and usage distinct from zero", func(t *testing.T) {
unknown, err := json.Marshal(MonthlySummary{Provider: "aws"})
require.NoError(t, err)
var fields map[string]any
require.NoError(t, json.Unmarshal(unknown, &fields))
assert.NotContains(t, fields, "total_commitment")
assert.NotContains(t, fields, "total_usage")

known, err := json.Marshal(MonthlySummary{TotalCommitment: f64ptr(0), TotalUsage: f64ptr(12.5)})
require.NoError(t, err)
require.NoError(t, json.Unmarshal(known, &fields))
assert.Equal(t, 0.0, fields["total_commitment"])
assert.Equal(t, 12.5, fields["total_usage"])
})
}

// TestProviderBreakdown tests the ProviderBreakdown struct.
Expand Down Expand Up @@ -1239,8 +1264,9 @@ func TestQueryMonthlyTotalsRowsErr(t *testing.T) {
month := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)

rows := pgxmock.NewRows([]string{
"month", "account_id", "cloud_account_id", "provider", "service", "total_savings", "avg_coverage", "snapshot_count",
}).AddRow(month, "account-123", strPtr("cloud-1"), "aws", "rds", 1500.0, f64ptr(85.0), 720)
"month", "account_id", "cloud_account_id", "provider", "service", "total_savings", "avg_coverage",
"total_commitment", "total_usage", "snapshot_count",
}).AddRow(month, "account-123", strPtr("cloud-1"), "aws", "rds", 1500.0, f64ptr(85.0), f64ptr(720.0), f64ptr(600.0), 720)

mock.ExpectQuery(`SELECT month, account_id`).
WithArgs(6, []string{"account-123"}).
Expand Down
39 changes: 39 additions & 0 deletions internal/api/handler_analytics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package api

import (
"context"
"encoding/json"
"errors"
"testing"
"time"
Expand Down Expand Up @@ -606,6 +607,44 @@ func TestHandler_getAnalyticsTrends_Success_ScopesByAccount(t *testing.T) {
assert.Len(t, resp.Service, 1)
}

// TestHandler_getAnalyticsTrends_MonthlyCommitmentAndUsage pins issue #30 on
// the wire: each monthly row carries total_commitment and total_usage, an
// unknown value is omitted rather than sent as 0, and a known 0 is sent.
func TestHandler_getAnalyticsTrends_MonthlyCommitmentAndUsage(t *testing.T) {
ctx := context.Background()
commitment, usage, zero := 720.0, 640.0, 0.0
mockSnap := new(MockAnalyticsSnapshotStore)
mockSnap.On("QueryMonthlyTotals", ctx, mock.Anything, mock.Anything, mock.Anything).
Return([]analytics.MonthlySummary{
{Provider: "aws", Service: "ec2", TotalSavings: 100, TotalCommitment: &commitment, TotalUsage: &usage},
{Provider: "aws", Service: "rds", TotalSavings: 50},
{Provider: "aws", Service: "elasticache", TotalCommitment: &zero, TotalUsage: &zero},
}, nil)
mockSnap.On("QueryByProvider", ctx, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return([]analytics.ProviderBreakdown{}, nil)
mockSnap.On("QueryByService", ctx, mock.Anything, mock.Anything, "", mock.Anything, mock.Anything).
Return([]analytics.ServiceBreakdown{}, nil)

mockAuth, req := adminAnalyticsReq(ctx)
handler := &Handler{auth: mockAuth, analyticsSnapshots: mockSnap, config: new(MockConfigStore)}

result, err := handler.getAnalyticsTrends(ctx, req, map[string]string{})
require.NoError(t, err)
raw, err := json.Marshal(result)
require.NoError(t, err)
var body struct {
Monthly []map[string]any `json:"monthly"`
}
require.NoError(t, json.Unmarshal(raw, &body))
require.Len(t, body.Monthly, 3)
assert.Equal(t, 720.0, body.Monthly[0]["total_commitment"])
assert.Equal(t, 640.0, body.Monthly[0]["total_usage"])
assert.NotContains(t, body.Monthly[1], "total_commitment", "unknown is omitted, never 0")
assert.NotContains(t, body.Monthly[1], "total_usage", "unknown is omitted, never 0")
assert.Equal(t, 0.0, body.Monthly[2]["total_commitment"], "a known 0 is sent")
assert.Equal(t, 0.0, body.Monthly[2]["total_usage"], "a known 0 is sent")
}

// TestHandler_getAnalyticsTrends_NoStore returns 503 when the snapshot store is
// not configured.
func TestHandler_getAnalyticsTrends_NoStore(t *testing.T) {
Expand Down
22 changes: 22 additions & 0 deletions internal/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8658,6 +8658,28 @@ components:
type: number
format: double
description: Average coverage percentage. Omitted when coverage data is absent (not zero).
total_commitment:
type: number
format: double
description: >
Amortized upfront cost as a monthly run-rate: each active
purchase's upfront payment spread evenly over its term, summed
per collection and averaged over the month (the same averaging as
total_savings). Recurring fees are not included; they are in
total_usage, so total committed spend is total_commitment plus
total_usage. Omitted when unknown, which is when any snapshot in
the bucket had no upfront figure; never reported as 0 in that
case. A known zero (for example a no-upfront portfolio) is
returned as 0.
total_usage:
type: number
format: double
description: >
Recurring commitment fees (the purchases' monthly cost) as a
monthly run-rate, averaged like total_commitment. Omitted when unknown, which is when any
snapshot in the bucket had no usage figure (for example an
all-upfront purchase); never reported as 0 in that case. A known
zero is returned as 0.
snapshot_count:
type: integer

Expand Down
Loading