diff --git a/internal/rds/cloudwatch.go b/internal/rds/cloudwatch.go index f50e289..b4b0850 100644 --- a/internal/rds/cloudwatch.go +++ b/internal/rds/cloudwatch.go @@ -17,6 +17,10 @@ type MetricStats struct { TotalConns float64 HasData bool DatapointCount int + // WO-16: SwapUsed is true if the instance measurably used swap during the + // window — a memory-pressure countersignal against flagging OVERSIZED_INSTANCE + // on low CPU alone, regardless of instance class. + SwapUsed bool } // WO-7: enables tag-based exclusion in rds/scanner.go. @@ -76,6 +80,24 @@ func FetchInstanceMetrics(ctx context.Context, cw CloudWatchAPI, instanceID stri return nil, err } + // WO-16: fetch swap usage as a memory-pressure countersignal for the + // oversized-instance check; any measurable swap activity means the + // instance is memory-bound regardless of how low its CPU looks. + swapOut, err := cw.GetMetricStatistics(ctx, &cloudwatch.GetMetricStatisticsInput{ + Namespace: aws.String("AWS/RDS"), + MetricName: aws.String("SwapUsage"), + Dimensions: []cwtypes.Dimension{ + {Name: aws.String("DBInstanceIdentifier"), Value: aws.String(instanceID)}, + }, + StartTime: aws.Time(start), + EndTime: aws.Time(now), + Period: aws.Int32(period), + Statistics: []cwtypes.Statistic{cwtypes.StatisticMaximum}, + }) + if err != nil { + return nil, err + } + stats := &MetricStats{} if len(cpuOut.Datapoints) > 0 { @@ -100,5 +122,14 @@ func FetchInstanceMetrics(ctx context.Context, cw CloudWatchAPI, instanceID stri } } + // WO-16: any measurable swap usage during the window is a memory-pressure + // countersignal, independent of instance class. + for _, dp := range swapOut.Datapoints { + if dp.Maximum != nil && *dp.Maximum > 0 { + stats.SwapUsed = true + break + } + } + return stats, nil } diff --git a/internal/rds/cloudwatch_test.go b/internal/rds/cloudwatch_test.go index 33ac135..fb07a51 100644 --- a/internal/rds/cloudwatch_test.go +++ b/internal/rds/cloudwatch_test.go @@ -122,3 +122,50 @@ func TestFetchInstanceMetricsMaxCPU(t *testing.T) { t.Errorf("AvgCPU = %.1f, want ~%.1f", stats.AvgCPU, expectedAvg) } } + +// WO-16: no SwapUsage datapoints (or all zero) means SwapUsed is false. +func TestFetchInstanceMetricsNoSwap(t *testing.T) { + cw := newMockCWClient() + cw.metrics["CPUUtilization"] = makeCPUDatapoints(8.0, 15.0, 14) + cw.metrics["DatabaseConnections"] = makeConnDatapoints(50) + cw.metrics["SwapUsage"] = makeSwapDatapoints(0) + + stats, err := FetchInstanceMetrics(context.Background(), cw, "mydb", now, 14) + if err != nil { + t.Fatalf("error: %v", err) + } + if stats.SwapUsed { + t.Error("SwapUsed = true, want false when swap datapoints are all zero") + } +} + +// WO-16: any measurable swap datapoint sets SwapUsed. +func TestFetchInstanceMetricsSwapDetected(t *testing.T) { + cw := newMockCWClient() + cw.metrics["CPUUtilization"] = makeCPUDatapoints(8.0, 15.0, 14) + cw.metrics["DatabaseConnections"] = makeConnDatapoints(50) + cw.metrics["SwapUsage"] = makeSwapDatapoints(2048) + + stats, err := FetchInstanceMetrics(context.Background(), cw, "mydb", now, 14) + if err != nil { + t.Fatalf("error: %v", err) + } + if !stats.SwapUsed { + t.Error("SwapUsed = false, want true when a nonzero swap datapoint is present") + } +} + +// WO-16: absent SwapUsage metric (no datapoints at all) defaults to false, unchanged behavior. +func TestFetchInstanceMetricsNoSwapMetric(t *testing.T) { + cw := newMockCWClient() + cw.metrics["CPUUtilization"] = makeCPUDatapoints(8.0, 15.0, 14) + cw.metrics["DatabaseConnections"] = makeConnDatapoints(50) + + stats, err := FetchInstanceMetrics(context.Background(), cw, "mydb", now, 14) + if err != nil { + t.Fatalf("error: %v", err) + } + if stats.SwapUsed { + t.Error("SwapUsed = true, want false when no SwapUsage metric is returned at all") + } +} diff --git a/internal/rds/mock_test.go b/internal/rds/mock_test.go index c472fde..44a259c 100644 --- a/internal/rds/mock_test.go +++ b/internal/rds/mock_test.go @@ -164,3 +164,13 @@ func makeConnDatapoints(totalConns float64) *cloudwatch.GetMetricStatisticsOutpu }, } } + +// WO-16: swap-usage datapoints for the oversized-instance memory-pressure countersignal. +func makeSwapDatapoints(maxBytes float64) *cloudwatch.GetMetricStatisticsOutput { + mx := maxBytes + return &cloudwatch.GetMetricStatisticsOutput{ + Datapoints: []cwtypes.Datapoint{ + {Maximum: &mx}, + }, + } +} diff --git a/internal/rds/scanner.go b/internal/rds/scanner.go index 7a8ac82..8919777 100644 --- a/internal/rds/scanner.go +++ b/internal/rds/scanner.go @@ -213,8 +213,11 @@ func (s *RDSScanner) analyzeInstance(ctx context.Context, cfg database.ScanConfi "engine": inst.Engine, }, }) - } else if metrics.MaxCPU < cfg.CPUThreshold && metrics.TotalConns > 0 { - // Oversized check: max CPU < threshold but has connections (active, just oversized) + // WO-16: oversized check requires max CPU < threshold AND no + // measurable swap activity — swap usage means the instance is + // memory-bound despite low CPU, so downsizing on CPU alone + // would be unsafe regardless of instance class. + } else if metrics.MaxCPU < cfg.CPUThreshold && metrics.TotalConns > 0 && !metrics.SwapUsed { findings = append(findings, database.Finding{ ID: database.FindingOversizedInstance, Severity: database.SeverityHigh, diff --git a/internal/rds/scanner_test.go b/internal/rds/scanner_test.go index 1a5388f..2c8f0a1 100644 --- a/internal/rds/scanner_test.go +++ b/internal/rds/scanner_test.go @@ -266,6 +266,47 @@ func TestScanOversizedInstance(t *testing.T) { } } +// WO-16: low CPU alone still flags oversized when no swap activity was measured. +func TestScanOversizedInstanceNoSwap(t *testing.T) { + mock := newMockRDSClient() + mock.instances = []rdstypes.DBInstance{ + makeInstance("big-db", "db.r5.xlarge", "postgres", "17.2"), + } + cw := newMockCWClient() + cw.metrics["CPUUtilization"] = makeCPUDatapoints(8.0, 15.0, 14) + cw.metrics["DatabaseConnections"] = makeConnDatapoints(50) + cw.metrics["SwapUsage"] = makeSwapDatapoints(0) + + s := newTestScanner(mock, cw) + result := s.Scan(context.Background(), defaultCfg(), nil) + + findings := findByID(result.Findings, database.FindingOversizedInstance) + if len(findings) != 1 { + t.Fatalf("expected 1 OVERSIZED_INSTANCE with zero swap, got %d", len(findings)) + } +} + +// WO-16: measurable swap usage suppresses OVERSIZED_INSTANCE even with low CPU, +// since it signals memory pressure the CPU metric alone can't see. +func TestScanOversizedInstanceSuppressedBySwap(t *testing.T) { + mock := newMockRDSClient() + mock.instances = []rdstypes.DBInstance{ + makeInstance("swapping-db", "db.r5.xlarge", "postgres", "17.2"), + } + cw := newMockCWClient() + cw.metrics["CPUUtilization"] = makeCPUDatapoints(8.0, 15.0, 14) + cw.metrics["DatabaseConnections"] = makeConnDatapoints(50) + cw.metrics["SwapUsage"] = makeSwapDatapoints(1048576) // 1 MiB of swap observed + + s := newTestScanner(mock, cw) + result := s.Scan(context.Background(), defaultCfg(), nil) + + findings := findByID(result.Findings, database.FindingOversizedInstance) + if len(findings) != 0 { + t.Fatalf("expected 0 OVERSIZED_INSTANCE when swap was used, got %d", len(findings)) + } +} + func TestScanActiveInstanceNotFlagged(t *testing.T) { mock := newMockRDSClient() mock.instances = []rdstypes.DBInstance{