refactor: move VM metrics to dedicated lib/vm_metrics package

This addresses PR review feedback:

- Create new lib/vm_metrics package for better separation of concerns
- Export GenerateTAPName from lib/network (fixes TAP name mismatch bug)
- Simplify API handler to use vm_metrics.Manager
- Add comprehensive README.md for the feature
- Remove utilization code from lib/resources package

The TAP name bug was causing network stats to always be zero because
the API handler used different truncation logic (10 chars, no lowercase)
than the canonical implementation (8 chars, lowercase).

Author: hiroTamada

cmd/api/api/api.go

@@ -10,20 +10,22 @@ import (
 	"github.com/kernel/hypeman/lib/network"
 	"github.com/kernel/hypeman/lib/oapi"
 	"github.com/kernel/hypeman/lib/resources"
+	"github.com/kernel/hypeman/lib/vm_metrics"
 	"github.com/kernel/hypeman/lib/volumes"
 )
 
 // ApiService implements the oapi.StrictServerInterface
 type ApiService struct {
-	Config          *config.Config
-	ImageManager    images.Manager
-	InstanceManager instances.Manager
-	VolumeManager   volumes.Manager
-	NetworkManager  network.Manager
-	DeviceManager   devices.Manager
-	IngressManager  ingress.Manager
-	BuildManager    builds.Manager
-	ResourceManager *resources.Manager
+	Config           *config.Config
+	ImageManager     images.Manager
+	InstanceManager  instances.Manager
+	VolumeManager    volumes.Manager
+	NetworkManager   network.Manager
+	DeviceManager    devices.Manager
+	IngressManager   ingress.Manager
+	BuildManager     builds.Manager
+	ResourceManager  *resources.Manager
+	VMMetricsManager *vm_metrics.Manager
 }
 
 var _ oapi.StrictServerInterface = (*ApiService)(nil)
@@ -39,16 +41,18 @@ func New(
 	ingressManager ingress.Manager,
 	buildManager builds.Manager,
 	resourceManager *resources.Manager,
+	vmMetricsManager *vm_metrics.Manager,
 ) *ApiService {
 	return &ApiService{
-		Config:          config,
-		ImageManager:    imageManager,
-		InstanceManager: instanceManager,
-		VolumeManager:   volumeManager,
-		NetworkManager:  networkManager,
-		DeviceManager:   deviceManager,
-		IngressManager:  ingressManager,
-		BuildManager:    buildManager,
-		ResourceManager: resourceManager,
+		Config:           config,
+		ImageManager:     imageManager,
+		InstanceManager:  instanceManager,
+		VolumeManager:    volumeManager,
+		NetworkManager:   networkManager,
+		DeviceManager:    deviceManager,
+		IngressManager:   ingressManager,
+		BuildManager:     buildManager,
+		ResourceManager:  resourceManager,
+		VMMetricsManager: vmMetricsManager,
 	}
 }

cmd/api/api/instances.go

@@ -17,6 +17,7 @@ import (
 	"github.com/kernel/hypeman/lib/network"
 	"github.com/kernel/hypeman/lib/oapi"
 	"github.com/kernel/hypeman/lib/resources"
+	"github.com/kernel/hypeman/lib/vm_metrics"
 	"github.com/samber/lo"
 )
 
@@ -278,7 +279,6 @@ func (s *ApiService) GetInstance(ctx context.Context, request oapi.GetInstanceRe
 // The id parameter can be an instance ID, name, or ID prefix
 // Note: Resolution is handled by ResolveResource middleware
 func (s *ApiService) GetInstanceStats(ctx context.Context, request oapi.GetInstanceStatsRequestObject) (oapi.GetInstanceStatsResponseObject, error) {
-	log := logger.FromContext(ctx)
 	inst := mw.GetResolvedInstance[instances.Instance](ctx)
 	if inst == nil {
 		return oapi.GetInstanceStats500JSONResponse{
@@ -287,68 +287,38 @@ func (s *ApiService) GetInstanceStats(ctx context.Context, request oapi.GetInsta
 		}, nil
 	}
 
-	// Build stats response
-	stats := oapi.InstanceStats{
-		InstanceId:           inst.Id,
-		InstanceName:         inst.Name,
-		AllocatedVcpus:       inst.Vcpus,
-		AllocatedMemoryBytes: inst.Size + inst.HotplugSize,
-	}
-
-	// Read /proc stats if we have a hypervisor PID
-	if inst.HypervisorPID != nil {
-		pid := *inst.HypervisorPID
-
-		// Read CPU from /proc/<pid>/stat
-		cpuUsec, err := resources.ReadProcStat(pid)
-		if err != nil {
-			log.DebugContext(ctx, "failed to read proc stat", "pid", pid, "error", err)
-		} else {
-			stats.CpuSeconds = float64(cpuUsec) / 1_000_000.0
-		}
+	// Build instance info for metrics collection
+	info := vm_metrics.BuildInstanceInfo(
+		inst.Id,
+		inst.Name,
+		inst.HypervisorPID,
+		inst.NetworkEnabled,
+		inst.Vcpus,
+		inst.Size+inst.HotplugSize,
+	)
 
-		// Read memory from /proc/<pid>/statm
-		rssBytes, vmsBytes, err := resources.ReadProcStatm(pid)
-		if err != nil {
-			log.DebugContext(ctx, "failed to read proc statm", "pid", pid, "error", err)
-		} else {
-			stats.MemoryRssBytes = int64(rssBytes)
-			stats.MemoryVmsBytes = int64(vmsBytes)
-
-			// Compute utilization ratio
-			if stats.AllocatedMemoryBytes > 0 {
-				ratio := float64(rssBytes) / float64(stats.AllocatedMemoryBytes)
-				stats.MemoryUtilizationRatio = &ratio
-			}
-		}
-	}
+	// Collect stats using vm_metrics manager
+	vmStats := s.VMMetricsManager.GetInstanceStats(ctx, info)
 
-	// Read TAP stats if network is enabled
-	if inst.NetworkEnabled {
-		tapName := generateTAPName(inst.Id)
-		rxBytes, txBytes, err := resources.ReadTAPStats(tapName)
-		if err != nil {
-			log.DebugContext(ctx, "failed to read TAP stats", "tap", tapName, "error", err)
-		} else {
-			stats.NetworkRxBytes = int64(rxBytes)
-			stats.NetworkTxBytes = int64(txBytes)
-		}
-	}
-
-	return oapi.GetInstanceStats200JSONResponse(stats), nil
+	// Map domain type to API type
+	return oapi.GetInstanceStats200JSONResponse(vmStatsToOAPI(vmStats)), nil
 }
 
-// generateTAPName generates TAP device name from instance ID
-func generateTAPName(instanceID string) string {
-	// TAP name format: "hype-" + first 10 chars of instance ID
-	// Max TAP name length is 15 chars (IFNAMSIZ - 1)
-	prefix := "hype-"
-	maxIDLen := 15 - len(prefix)
-	idPart := instanceID
-	if len(idPart) > maxIDLen {
-		idPart = idPart[:maxIDLen]
-	}
-	return prefix + idPart
+// vmStatsToOAPI converts vm_metrics.VMStats to oapi.InstanceStats
+func vmStatsToOAPI(s *vm_metrics.VMStats) oapi.InstanceStats {
+	stats := oapi.InstanceStats{
+		InstanceId:           s.InstanceID,
+		InstanceName:         s.InstanceName,
+		CpuSeconds:           s.CPUSeconds(),
+		MemoryRssBytes:       int64(s.MemoryRSSBytes),
+		MemoryVmsBytes:       int64(s.MemoryVMSBytes),
+		NetworkRxBytes:       int64(s.NetRxBytes),
+		NetworkTxBytes:       int64(s.NetTxBytes),
+		AllocatedVcpus:       s.AllocatedVcpus,
+		AllocatedMemoryBytes: s.AllocatedMemoryBytes,
+		MemoryUtilizationRatio: s.MemoryUtilizationRatio(),
+	}
+	return stats
 }
 
 // DeleteInstance stops and deletes an instance

cmd/api/wire.go

@@ -19,25 +19,27 @@ import (
 	"github.com/kernel/hypeman/lib/registry"
 	"github.com/kernel/hypeman/lib/resources"
 	"github.com/kernel/hypeman/lib/system"
+	"github.com/kernel/hypeman/lib/vm_metrics"
 	"github.com/kernel/hypeman/lib/volumes"
 )
 
 // application struct to hold initialized components
 type application struct {
-	Ctx             context.Context
-	Logger          *slog.Logger
-	Config          *config.Config
-	ImageManager    images.Manager
-	SystemManager   system.Manager
-	NetworkManager  network.Manager
-	DeviceManager   devices.Manager
-	InstanceManager instances.Manager
-	VolumeManager   volumes.Manager
-	IngressManager  ingress.Manager
-	BuildManager    builds.Manager
-	ResourceManager *resources.Manager
-	Registry        *registry.Registry
-	ApiService      *api.ApiService
+	Ctx              context.Context
+	Logger           *slog.Logger
+	Config           *config.Config
+	ImageManager     images.Manager
+	SystemManager    system.Manager
+	NetworkManager   network.Manager
+	DeviceManager    devices.Manager
+	InstanceManager  instances.Manager
+	VolumeManager    volumes.Manager
+	IngressManager   ingress.Manager
+	BuildManager     builds.Manager
+	ResourceManager  *resources.Manager
+	VMMetricsManager *vm_metrics.Manager
+	Registry         *registry.Registry
+	ApiService       *api.ApiService
 }
 
 // initializeApp is the injector function
@@ -56,6 +58,7 @@ func initializeApp() (*application, func(), error) {
 		providers.ProvideIngressManager,
 		providers.ProvideBuildManager,
 		providers.ProvideResourceManager,
+		providers.ProvideVMMetricsManager,
 		providers.ProvideRegistry,
 		api.New,
 		wire.Struct(new(application), "*"),

cmd/api/wire_gen.go

@@ -3,7 +3,6 @@ package instances
 import (
 	"context"
 	"fmt"
-	"strings"
 	"sync"
 
 	"github.com/kernel/hypeman/lib/devices"
@@ -359,7 +358,7 @@ func (m *manager) ListRunningInstancesInfo(ctx context.Context) ([]resources.Ins
 
 		// Derive TAP device name if networking is enabled
 		if inst.NetworkEnabled {
-			info.TAPDevice = generateTAPName(inst.Id)
+			info.TAPDevice = network.GenerateTAPName(inst.Id)
 		}
 
 		infos = append(infos, info)
@@ -368,14 +367,3 @@ func (m *manager) ListRunningInstancesInfo(ctx context.Context) ([]resources.Ins
 	return infos, nil
 }
 
-// generateTAPName generates TAP device name from instance ID.
-// This matches the logic in network/allocate.go.
-func generateTAPName(instanceID string) string {
-	// Use first 8 chars of instance ID
-	// hype-{8chars} fits within 15-char Linux interface name limit
-	shortID := instanceID
-	if len(shortID) > 8 {
-		shortID = shortID[:8]
-	}
-	return "hype-" + strings.ToLower(shortID)
-}

lib/instances/manager.go

@@ -1002,38 +1002,3 @@ func (r *testInstanceResolver) ResolveInstance(ctx context.Context, nameOrID str
 	return nameOrID, nameOrID, nil
 }
 
-func TestGenerateTAPName(t *testing.T) {
-	tests := []struct {
-		name       string
-		instanceID string
-		expected   string
-	}{
-		{
-			name:       "standard ID",
-			instanceID: "01HQVX7ABC123DEF456",
-			expected:   "hype-01hqvx7a",
-		},
-		{
-			name:       "short ID",
-			instanceID: "ABC123",
-			expected:   "hype-abc123",
-		},
-		{
-			name:       "exact 8 chars",
-			instanceID: "ABCDEFGH",
-			expected:   "hype-abcdefgh",
-		},
-		{
-			name:       "mixed case",
-			instanceID: "AbCdEfGhIjKl",
-			expected:   "hype-abcdefgh",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			result := generateTAPName(tt.instanceID)
-			assert.Equal(t, tt.expected, result)
-		})
-	}
-}