fix: address remaining code review issues in metrics

- Fix defer in loop: close response body immediately after decode
- Fix memory leak: cleanup stale CPU stats for deleted clones
- Fix CPU rollover: detect counter reset when container restarts
- Remove redundant prevCPUStatsMu mutex (already protected by c.mu)
- Remove unused DatasetInfo metric

Author: claude

PROMETHEUS.md

@@ -75,7 +75,6 @@ The endpoint is publicly accessible (no authentication required) and returns met
 |-------------|------|--------|-------------|
 | `dblab_datasets_total` | Gauge | `pool` | Total number of datasets (slots) in the pool |
 | `dblab_datasets_available` | Gauge | `pool` | Number of available (non-busy) dataset slots for reuse |
-| `dblab_dataset_info` | Gauge | `pool`, `dataset_name` | Information about a dataset (1=busy, 0=available) |
 
 ## Prometheus Configuration
 

engine/internal/srv/metrics/collector.go

@@ -32,16 +32,15 @@ type containerCPUState struct {
 
 // Collector collects metrics from DBLab components.
 type Collector struct {
-	mu             sync.Mutex
-	metrics        *Metrics
-	cloning        *cloning.Base
-	retrieval      *retrieval.Retrieval
-	pm             *pool.Manager
-	engProps       *global.EngineProps
-	dockerClient   *client.Client
-	startedAt      time.Time
-	prevCPUStats   map[string]containerCPUState
-	prevCPUStatsMu sync.RWMutex
+	mu           sync.Mutex
+	metrics      *Metrics
+	cloning      *cloning.Base
+	retrieval    *retrieval.Retrieval
+	pm           *pool.Manager
+	engProps     *global.EngineProps
+	dockerClient *client.Client
+	startedAt    time.Time
+	prevCPUStats map[string]containerCPUState
 }
 
 // NewCollector creates a new metrics collector.
@@ -221,22 +220,27 @@ func (c *Collector) getContainerStats(ctx context.Context, clones []*models.Clon
 		return result
 	}
 
+	activeCloneIDs := make(map[string]struct{})
+
 	for _, clone := range clones {
 		if clone == nil || clone.Status.Code != models.StatusOK {
 			continue
 		}
 
+		activeCloneIDs[clone.ID] = struct{}{}
+
 		stats, err := c.dockerClient.ContainerStatsOneShot(ctx, clone.ID)
 		if err != nil {
 			log.Dbg(fmt.Sprintf("failed to get container stats for clone %s: %v", clone.ID, err))
 			continue
 		}
 
-		defer stats.Body.Close()
-
 		var statsJSON container.StatsResponse
-		if err := json.NewDecoder(stats.Body).Decode(&statsJSON); err != nil {
-			log.Dbg(fmt.Sprintf("failed to decode container stats for clone %s: %v", clone.ID, err))
+		decodeErr := json.NewDecoder(stats.Body).Decode(&statsJSON)
+		stats.Body.Close()
+
+		if decodeErr != nil {
+			log.Dbg(fmt.Sprintf("failed to decode container stats for clone %s: %v", clone.ID, decodeErr))
 			continue
 		}
 
@@ -251,6 +255,8 @@ func (c *Collector) getContainerStats(ctx context.Context, clones []*models.Clon
 		}
 	}
 
+	c.cleanupStaleCPUStats(activeCloneIDs)
+
 	return result
 }
 
@@ -261,17 +267,13 @@ func (c *Collector) calculateCPUPercent(cloneID string, stats *container.StatsRe
 	currentSystemUsage := stats.CPUStats.SystemUsage
 	now := time.Now()
 
-	c.prevCPUStatsMu.RLock()
 	prevStats, hasPrev := c.prevCPUStats[cloneID]
-	c.prevCPUStatsMu.RUnlock()
 
-	c.prevCPUStatsMu.Lock()
 	c.prevCPUStats[cloneID] = containerCPUState{
 		totalUsage:  currentTotalUsage,
 		systemUsage: currentSystemUsage,
 		timestamp:   now,
 	}
-	c.prevCPUStatsMu.Unlock()
 
 	if !hasPrev {
 		return 0
@@ -282,10 +284,14 @@ func (c *Collector) calculateCPUPercent(cloneID string, stats *container.StatsRe
 		return 0
 	}
 
+	if currentTotalUsage < prevStats.totalUsage || currentSystemUsage < prevStats.systemUsage {
+		return 0
+	}
+
 	cpuDelta := float64(currentTotalUsage - prevStats.totalUsage)
 	systemDelta := float64(currentSystemUsage - prevStats.systemUsage)
 
-	if systemDelta <= 0 || cpuDelta < 0 {
+	if systemDelta <= 0 {
 		return 0
 	}
 
@@ -301,6 +307,14 @@ func (c *Collector) calculateCPUPercent(cloneID string, stats *container.StatsRe
 	return (cpuDelta / systemDelta) * cpuCount * 100.0
 }
 
+func (c *Collector) cleanupStaleCPUStats(activeCloneIDs map[string]struct{}) {
+	for cloneID := range c.prevCPUStats {
+		if _, ok := activeCloneIDs[cloneID]; !ok {
+			delete(c.prevCPUStats, cloneID)
+		}
+	}
+}
+
 func (c *Collector) collectSnapshotMetrics() {
 	snapshots, err := c.cloning.GetSnapshots()
 	if err != nil {

engine/internal/srv/metrics/metrics.go

@@ -60,7 +60,6 @@ type Metrics struct {
 	// dataset metrics (for logical mode - non-busy slots)
 	DatasetsTotal     *prometheus.GaugeVec
 	DatasetsAvailable *prometheus.GaugeVec
-	DatasetInfo       *prometheus.GaugeVec
 }
 
 // NewMetrics creates a new Metrics instance with all Prometheus metrics.
@@ -342,14 +341,6 @@ func NewMetrics() *Metrics {
 			},
 			[]string{"pool"},
 		),
-		DatasetInfo: prometheus.NewGaugeVec(
-			prometheus.GaugeOpts{
-				Namespace: namespace,
-				Name:      "dataset_info",
-				Help:      "Information about a dataset (1=busy, 0=available)",
-			},
-			[]string{"pool", "dataset_name"},
-		),
 	}
 }
 
@@ -390,7 +381,6 @@ func (m *Metrics) Register(reg prometheus.Registerer) error {
 		m.BranchInfo,
 		m.DatasetsTotal,
 		m.DatasetsAvailable,
-		m.DatasetInfo,
 	}
 
 	for _, c := range collectors {
@@ -431,5 +421,4 @@ func (m *Metrics) Reset() {
 	m.BranchInfo.Reset()
 	m.DatasetsTotal.Reset()
 	m.DatasetsAvailable.Reset()
-	m.DatasetInfo.Reset()
 }