Wo 18/iops idle

internal/rds/cloudwatch.go

@@ -27,6 +27,10 @@ type MetricStats struct {
 	// write I/O is a countersignal against downsizing even when CPU is low,
 	// because burstable instance classes scale EBS bandwidth with size.
 	AvgWriteIOPS float64
+	// WO-18: AvgReadIOPS is average read IOPS over the window. Combined with
+	// AvgWriteIOPS, total IOPS is the activity signal that distinguishes a
+	// genuinely idle instance (pooled connections but no I/O) from an active one.
+	AvgReadIOPS float64
 }
 
 // WO-7: enables tag-based exclusion in rds/scanner.go.
@@ -70,7 +74,8 @@ func FetchInstanceMetrics(ctx context.Context, cw CloudWatchAPI, instanceID stri
 		return nil, err
 	}
 
-	// Fetch connection count
+	// WO-19: use Average for DatabaseConnections so TotalConns is the average
+	// connection count over the window, not a meaningless sum-of-daily-sums.
 	connOut, err := cw.GetMetricStatistics(ctx, &cloudwatch.GetMetricStatisticsInput{
 		Namespace:  aws.String("AWS/RDS"),
 		MetricName: aws.String("DatabaseConnections"),
@@ -80,7 +85,7 @@ func FetchInstanceMetrics(ctx context.Context, cw CloudWatchAPI, instanceID stri
 		StartTime:  aws.Time(start),
 		EndTime:    aws.Time(now),
 		Period:     aws.Int32(period),
-		Statistics: []cwtypes.Statistic{cwtypes.StatisticSum},
+		Statistics: []cwtypes.Statistic{cwtypes.StatisticAverage},
 	})
 	if err != nil {
 		return nil, err
@@ -120,6 +125,23 @@ func FetchInstanceMetrics(ctx context.Context, cw CloudWatchAPI, instanceID stri
 		return nil, err
 	}
 
+	// WO-18: fetch read IOPS; combined with write IOPS, total IOPS is the
+	// activity signal for pooled-connection idle detection.
+	readOut, err := cw.GetMetricStatistics(ctx, &cloudwatch.GetMetricStatisticsInput{
+		Namespace:  aws.String("AWS/RDS"),
+		MetricName: aws.String("ReadIOPS"),
+		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.StatisticAverage},
+	})
+	if err != nil {
+		return nil, err
+	}
+
 	stats := &MetricStats{}
 
 	if len(cpuOut.Datapoints) > 0 {
@@ -138,11 +160,18 @@ func FetchInstanceMetrics(ctx context.Context, cw CloudWatchAPI, instanceID stri
 		stats.MaxCPU = maxVal
 	}
 
+	// WO-19: average the Average datapoints, not sum the Sum datapoints.
+	var connSum float64
+	var connCount int
 	for _, dp := range connOut.Datapoints {
-		if dp.Sum != nil {
-			stats.TotalConns += *dp.Sum
+		if dp.Average != nil {
+			connSum += *dp.Average
+			connCount++
 		}
 	}
+	if connCount > 0 {
+		stats.TotalConns = connSum / float64(connCount)
+	}
 
 	// WO-17@v2: swap GROWTH, not presence. CloudWatch does not guarantee datapoint
 	// ordering, so sort by timestamp before taking the first/last delta.
@@ -161,6 +190,19 @@ func FetchInstanceMetrics(ctx context.Context, cw CloudWatchAPI, instanceID stri
 		stats.AvgWriteIOPS = writeSum / float64(writeCount)
 	}
 
+	// WO-18: average read IOPS across the window.
+	var readSum float64
+	var readCount int
+	for _, dp := range readOut.Datapoints {
+		if dp.Average != nil {
+			readSum += *dp.Average
+			readCount++
+		}
+	}
+	if readCount > 0 {
+		stats.AvgReadIOPS = readSum / float64(readCount)
+	}
+
 	return stats, nil
 }
 

internal/rds/cloudwatch_test.go

@@ -124,7 +124,7 @@ func TestFetchInstanceMetricsMaxCPU(t *testing.T) {
 }
 
 // WO-17@v2: a flat swap profile is parked-page noise and must report zero growth.
-// This is the exact live-account shape (media-view-prod, 8.38MB flat over 14d)
+// This is the exact live-account shape (a write-heavy instance, 8.38MB flat over 14d)
 // that WO-16 wrongly read as memory pressure.
 func TestFetchInstanceMetricsFlatSwapIsNotGrowth(t *testing.T) {
 	cw := newMockCWClient()
@@ -158,7 +158,7 @@ func TestFetchInstanceMetricsDecliningSwap(t *testing.T) {
 }
 
 // WO-17@v2: genuinely growing swap reports the positive delta (live-account shape
-// of saga-service-prod: 0.5MB -> 6.24MB).
+// of a memory-pressured instance: 0.5MB -> 6.24MB).
 func TestFetchInstanceMetricsGrowingSwap(t *testing.T) {
 	cw := newMockCWClient()
 	cw.metrics["CPUUtilization"] = makeCPUDatapoints(8.0, 15.0, 14)
@@ -230,3 +230,23 @@ func TestFetchInstanceMetricsWriteIOPS(t *testing.T) {
 		t.Errorf("AvgWriteIOPS = %.2f, want ~138.88", stats.AvgWriteIOPS)
 	}
 }
+
+// WO-18: average read IOPS is surfaced alongside write IOPS.
+func TestFetchInstanceMetricsReadIOPS(t *testing.T) {
+	cw := newMockCWClient()
+	cw.metrics["CPUUtilization"] = makeCPUDatapoints(8.0, 15.0, 14)
+	cw.metrics["DatabaseConnections"] = makeConnDatapoints(50)
+	cw.metrics["WriteIOPS"] = makeWriteIOPSDatapoints(1.49, 14)
+	cw.metrics["ReadIOPS"] = makeWriteIOPSDatapoints(0.33, 14)
+
+	stats, err := FetchInstanceMetrics(context.Background(), cw, "mydb", now, 14)
+	if err != nil {
+		t.Fatalf("error: %v", err)
+	}
+	if stats.AvgReadIOPS < 0.2 || stats.AvgReadIOPS > 0.4 {
+		t.Errorf("AvgReadIOPS = %.2f, want ~0.33", stats.AvgReadIOPS)
+	}
+	if stats.AvgWriteIOPS < 1.3 || stats.AvgWriteIOPS > 1.6 {
+		t.Errorf("AvgWriteIOPS = %.2f, want ~1.49", stats.AvgWriteIOPS)
+	}
+}

internal/rds/mock_test.go

@@ -157,10 +157,11 @@ func makeCPUDatapoints(avgCPU, maxCPU float64, count int) *cloudwatch.GetMetricS
 	return &cloudwatch.GetMetricStatisticsOutput{Datapoints: dps}
 }
 
-func makeConnDatapoints(totalConns float64) *cloudwatch.GetMetricStatisticsOutput {
+// WO-19: DatabaseConnections now uses Average, not Sum.
+func makeConnDatapoints(avgConns float64) *cloudwatch.GetMetricStatisticsOutput {
 	return &cloudwatch.GetMetricStatisticsOutput{
 		Datapoints: []cwtypes.Datapoint{
-			{Sum: &totalConns},
+			{Average: &avgConns},
 		},
 	}
 }

internal/rds/scanner.go

@@ -25,6 +25,13 @@ const (
 	writeIOPSBusyThreshold = 50.0
 )
 
+// WO-18: idleIOPSThreshold is the total average IOPS (read + write) below which
+// an instance is treated as effectively idle even when connection pools hold
+// connections open. Calibrated from live data: a genuinely dead instance
+// (a dead-app instance) averages ~1.8 IOPS; the lowest-activity live instance
+// (a low-traffic instance) averages ~4.0 IOPS. 5.0 sits between those two clusters.
+const idleIOPSThreshold = 5.0
+
 // WO-17@v2: gradeOversized converts corroborating metrics into a confidence grade
 // plus human-readable countersignals. It never decides whether to emit — that
 // is the caller's job — it only reports how much the evidence agrees.
@@ -229,19 +236,39 @@ func (s *RDSScanner) analyzeInstance(ctx context.Context, cfg database.ScanConfi
 	if s.cw != nil && cfg.MetricDays > 0 {
 		metrics, err := FetchInstanceMetrics(ctx, s.cw, inst.ID, s.now, cfg.MetricDays)
 		if err == nil && metrics.HasData {
-			// Idle check: avg CPU < idle threshold AND zero connections
-			if metrics.AvgCPU < cfg.IdleCPU && metrics.TotalConns == 0 {
+			// WO-18: idle check — low CPU AND (zero connections OR near-zero IOPS).
+			totalIOPS := metrics.AvgReadIOPS + metrics.AvgWriteIOPS
+			// Connection pools hold connections open on dead apps, so TotalConns==0
+			// alone misses them. Total IOPS below the threshold catches a pooled
+			// but effectively dead instance (e.g. a dead-app instance: 1.8 IOPS, 6.7 conns).
+			if metrics.AvgCPU < cfg.IdleCPU && (metrics.TotalConns == 0 || totalIOPS < idleIOPSThreshold) {
+				// WO-18: a zero-connection idle is confident; a pooled-connection
+				// idle is graded needs-review because a warm pool does not prove
+				// the app is permanently dead.
+				confidence := database.ConfidenceConfident
+				var countersignals []string
+				if metrics.TotalConns > 0 {
+					confidence = database.ConfidenceNeedsReview
+					countersignals = append(countersignals, fmt.Sprintf(
+						"%.0f pooled connections but %.1f total IOPS (verify app is decommissioned)",
+						metrics.TotalConns, totalIOPS))
+				}
 				findings = append(findings, database.Finding{
 					ID:                    database.FindingIdleInstance,
 					Severity:              database.SeverityHigh,
 					ResourceType:          database.ResourceInstance,
 					ResourceID:            inst.ID,
 					Region:                s.region,
-					Message:               fmt.Sprintf("Instance idle for %d days (avg CPU %.1f%%, 0 connections)", cfg.MetricDays, metrics.AvgCPU),
+					Message:               fmt.Sprintf("Instance idle for %d days (avg CPU %.1f%%, %.1f total IOPS)", cfg.MetricDays, metrics.AvgCPU, totalIOPS),
 					EstimatedMonthlyWaste: monthlyCost,
+					Confidence:            confidence,
+					Countersignals:        countersignals,
 					Metadata: map[string]any{
 						"avg_cpu":        metrics.AvgCPU,
 						"total_conns":    metrics.TotalConns,
+						"total_iops":     totalIOPS,
+						"read_iops":      metrics.AvgReadIOPS,
+						"write_iops":     metrics.AvgWriteIOPS,
 						"metric_days":    cfg.MetricDays,
 						"instance_class": inst.Class,
 						"engine":         inst.Engine,

internal/rds/scanner_test.go

@@ -249,6 +249,80 @@ func TestScanIdleInstance(t *testing.T) {
 	}
 }
 
+// WO-18: zero-connection idle is graded confident.
+func TestScanIdleInstanceZeroConnsIsConfident(t *testing.T) {
+	mock := newMockRDSClient()
+	mock.instances = []rdstypes.DBInstance{
+		makeInstance("idle-db", "db.t3.small", "postgres", "17.2"),
+	}
+	cw := newMockCWClient()
+	cw.metrics["CPUUtilization"] = makeCPUDatapoints(2.0, 4.0, 14)
+	cw.metrics["DatabaseConnections"] = makeConnDatapoints(0)
+
+	s := newTestScanner(mock, cw)
+	result := s.Scan(context.Background(), defaultCfg(), nil)
+
+	findings := findByID(result.Findings, database.FindingIdleInstance)
+	if len(findings) != 1 {
+		t.Fatalf("expected 1 IDLE_INSTANCE, got %d", len(findings))
+	}
+	if findings[0].Confidence != database.ConfidenceConfident {
+		t.Errorf("Confidence = %q, want %q for zero-connection idle", findings[0].Confidence, database.ConfidenceConfident)
+	}
+}
+
+// WO-18: a pooled-connection instance with near-zero IOPS is flagged idle.
+// Shape observed on a real account: ~1.8 total IOPS with ~7 pooled connections.
+func TestScanIdleInstancePooledButDead(t *testing.T) {
+	mock := newMockRDSClient()
+	mock.instances = []rdstypes.DBInstance{
+		makeInstance("dead-app-db", "db.t4g.small", "postgres", "13.4"),
+	}
+	cw := newMockCWClient()
+	cw.metrics["CPUUtilization"] = makeCPUDatapoints(2.0, 4.0, 14)
+	cw.metrics["DatabaseConnections"] = makeConnDatapoints(7)
+	cw.metrics["ReadIOPS"] = makeWriteIOPSDatapoints(0.33, 14)
+	cw.metrics["WriteIOPS"] = makeWriteIOPSDatapoints(1.49, 14)
+
+	s := newTestScanner(mock, cw)
+	result := s.Scan(context.Background(), defaultCfg(), nil)
+
+	findings := findByID(result.Findings, database.FindingIdleInstance)
+	if len(findings) != 1 {
+		t.Fatalf("expected 1 IDLE_INSTANCE for pooled-but-dead instance, got %d (TotalConns==0 rule alone would have missed this)", len(findings))
+	}
+	if findings[0].Confidence != database.ConfidenceNeedsReview {
+		t.Errorf("Confidence = %q, want %q for pooled-connection idle", findings[0].Confidence, database.ConfidenceNeedsReview)
+	}
+	if len(findings[0].Countersignals) != 1 {
+		t.Fatalf("expected 1 countersignal, got %v", findings[0].Countersignals)
+	}
+	if !strings.Contains(findings[0].Countersignals[0], "pooled connections") {
+		t.Errorf("countersignal = %q, want it to name the pooled connections", findings[0].Countersignals[0])
+	}
+}
+
+// WO-18: a pooled-connection instance with active IOPS is NOT flagged idle.
+// It should fall through to the OVERSIZED_INSTANCE check instead.
+func TestScanIdleInstancePooledAndActiveNotIdle(t *testing.T) {
+	mock := newMockRDSClient()
+	mock.instances = []rdstypes.DBInstance{
+		makeInstance("active-db", "db.t4g.small", "postgres", "17.2"),
+	}
+	cw := newMockCWClient()
+	cw.metrics["CPUUtilization"] = makeCPUDatapoints(8.0, 15.0, 14)
+	cw.metrics["DatabaseConnections"] = makeConnDatapoints(50)
+	cw.metrics["ReadIOPS"] = makeWriteIOPSDatapoints(106.0, 14)
+	cw.metrics["WriteIOPS"] = makeWriteIOPSDatapoints(8.0, 14)
+
+	s := newTestScanner(mock, cw)
+	result := s.Scan(context.Background(), defaultCfg(), nil)
+
+	if len(findByID(result.Findings, database.FindingIdleInstance)) != 0 {
+		t.Error("should not flag an active instance as idle just because CPU is low")
+	}
+}
+
 func TestScanOversizedInstance(t *testing.T) {
 	mock := newMockRDSClient()
 	mock.instances = []rdstypes.DBInstance{
@@ -347,7 +421,7 @@ func TestScanOversizedGrowingSwapNeedsReview(t *testing.T) {
 }
 
 // WO-17@v2: sustained write IOPS downgrades confidence (live-account shape of
-// media-view-prod: 14.6% max CPU but 138.88 avg write IOPS).
+// a write-heavy instance: 14.6% max CPU but 138.88 avg write IOPS).
 func TestScanOversizedHighWriteIOPSNeedsReview(t *testing.T) {
 	mock := newMockRDSClient()
 	mock.instances = []rdstypes.DBInstance{