From b0ad1e5a5f1e347758987243c646846b81543787 Mon Sep 17 00:00:00 2001 From: ppiankov <103106369+ppiankov@users.noreply.github.com> Date: Sun, 2 Aug 2026 12:59:43 +0800 Subject: [PATCH 1/2] fix: require a swap-usage countersignal for OVERSIZED_INSTANCE WO-16: OVERSIZED_INSTANCE flagged on max-CPU alone, which cannot tell a genuinely oversized instance apart from one that is memory-bound despite low CPU. Fetch the CloudWatch SwapUsage metric alongside the existing CPU/connections fetch and suppress the finding when any measurable swap activity was observed during the window -- a countersignal independent of instance class, so no memory-size lookup table is needed. --- internal/rds/cloudwatch.go | 31 ++++++++++++++++++++++ internal/rds/cloudwatch_test.go | 47 +++++++++++++++++++++++++++++++++ internal/rds/mock_test.go | 10 +++++++ internal/rds/scanner.go | 7 +++-- internal/rds/scanner_test.go | 41 ++++++++++++++++++++++++++++ 5 files changed, 134 insertions(+), 2 deletions(-) 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..b4aa7d3 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) + } else if metrics.MaxCPU < cfg.CPUThreshold && metrics.TotalConns > 0 && !metrics.SwapUsed { + // 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. 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{ From 5110d0106785403905034e06db23a12f697c7e8a Mon Sep 17 00:00:00 2001 From: ppiankov <103106369+ppiankov@users.noreply.github.com> Date: Sun, 2 Aug 2026 13:01:01 +0800 Subject: [PATCH 2/2] fix: move WO-16 citation to lead the changed condition line wolint flagged the sentinel-check hunk as unattributed: the comment sat after the changed if-condition, not before it, so it didn't govern the line per the hunk-adjacency rule. --- internal/rds/scanner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/rds/scanner.go b/internal/rds/scanner.go index b4aa7d3..8919777 100644 --- a/internal/rds/scanner.go +++ b/internal/rds/scanner.go @@ -213,11 +213,11 @@ func (s *RDSScanner) analyzeInstance(ctx context.Context, cfg database.ScanConfi "engine": inst.Engine, }, }) - } else if metrics.MaxCPU < cfg.CPUThreshold && metrics.TotalConns > 0 && !metrics.SwapUsed { // 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,