diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 4fc181d6bb..cd1e819a3d 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -1079,9 +1079,16 @@ clears it. A denied or failed node-status endpoint does not invalidate successfu connectivity or independently accessible datastore inventory. In-process state copies preserve this evidence; JSON deliberately does not carry it. The zero-value compatibility default is not persisted availability evidence. +Only that successful node-status observation writes CPU and memory percentages +to the PBS service's `agent:` history target. Missing or invalid +node measurements append no sample, rather than a false zero; measured zero is +valid. The service series is separate from a correlated Pulse Agent's host +series and contains no inferred disk, network, or I/O measurements. Proof: `internal/models/metrics_types_test.go` and -`internal/monitoring/monitor_pbs_coverage_test.go`. The latter exercises real +`internal/monitoring/monitor_pbs_coverage_test.go`; the latter exercises real HTTP polling through normal alert-manager publication, not destination delivery. +`TestPollPBSNodeStatusRecordsServiceHistory` verifies volatile and persisted +service-history reads plus denial/recovery without fabricated samples. 1. `internal/config/host_continuity.go` shared with `agent-lifecycle`: the durable host identity, report-order watermark, and removal tombstone journal is jointly owned by agent lifecycle admission and monitoring report continuity. diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 7348e0db20..46a04ffb92 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -311,7 +311,7 @@ command-capable profile. Backups hydrates type=pbs,agent with source=pbs and reuses Overview guest inventory. Deduplicate combined snapshots by canonical ID before correlation, avoiding false ambiguity for agents in both queries. Standalone non-PVE PBS telemetry remains available without downloading the guest estate twice. -The same agent can still arrive as two distinct resources: a PVE guest carrying its telemetry and the standalone `source=pbs` host row. Collapse those candidates by agent identity and resolve the Backups PBS row to the guest target, whose persisted host series the drawer charts; keeping the PBS service target leaves History on a key with no host data. Distinct agent identities stay ambiguous, and a missing agent identity is not sameness proof. Do not add a second guest-estate request to compensate. +The same agent can still arrive as two distinct resources: a PVE guest carrying its telemetry and the standalone `source=pbs` host row. Collapse those candidates by agent identity and resolve the Backups PBS row to the guest target, whose persisted host series the drawer charts. Distinct agent identities stay ambiguous, and a missing agent identity is not sameness proof. Do not add a second guest-estate request to compensate. When no host can be safely correlated, a successful PBS node-status poll now records CPU and memory on the separate PBS service target; those partial service observations do not imply host-agent disk, network, or I/O history. For token-auth PBS connections that cannot report `nodeName`, a registry-corroborated `pbs.linkedAgentId` takes precedence over connection labels when selecting the Agent-bearing host row. It must resolve to the same source-native Agent ID on a standalone or PVE-merged host; a PVE-only node with no Agent facet cannot supply host History. The PBS row and service target stay canonical; only its drawer presentation uses the host metrics target. If the explicit link changes, retained correlation to the former host is discarded rather than carrying old charts across a replacement. diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index d34779661a..fcf11a931a 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -508,7 +508,7 @@ PBS presentation correlation accepts standalone agents and VM/system-container r When the registry uniquely corroborates a PBS connection to a host agent by node name, connection name, endpoint hostname or reported interface IP, the PBS service may expose that source-native agent ID as `pbs.linkedAgentId`. This is a presentation link, not a replacement for the PBS service's canonical ID or metrics target. Token-auth connections may lack `nodeName`; consumers must use the explicit link in preference to display-name matching and must not infer one from an IP or name themselves. PVE-only nodes without an actual Agent facet are not host-history candidates, even if their name matches the PBS connection. A changed link invalidates any remembered correlation to an old host. -A single agent surfaced as both a VM/system-container guest and a standalone agent row is one identity, not an ambiguous pair. Collapse candidates that share an agent identity before deciding, and prefer the guest's canonical metricsTarget because it names the persisted host series; the PBS service target names the service key and has no host history. Only distinct agent identities, or candidates whose identity cannot be proven equal, decline to choose. Never use input order or a fallback that discards the PBS row id. +A single agent surfaced as both a VM/system-container guest and a standalone agent row is one identity, not an ambiguous pair. Collapse candidates that share an agent identity before deciding, and prefer the guest's canonical metricsTarget because it names the persisted host series; the PBS service target names the service key and now records only PBS API node-status CPU/memory history, not host-agent disk, network, or I/O. Only distinct agent identities, or candidates whose identity cannot be proven equal, decline to choose. Never use input order or a fallback that discards the PBS row id. A realtime refresh can briefly omit the correlated host row while the PBS server row remains. Retain the last resolved correlation per PBS server across that omission rather than substituting the PBS service target, so the drawer's identity rows and history target stay stable. Reuse the remembered host only while it is still fresh relative to the server and drop it once stale, so a removed or replaced host is not advertised indefinitely; a present but ambiguous host still declines. diff --git a/internal/monitoring/monitor_pbs_coverage_test.go b/internal/monitoring/monitor_pbs_coverage_test.go index a18a4ec6fb..1e1ee9ad44 100644 --- a/internal/monitoring/monitor_pbs_coverage_test.go +++ b/internal/monitoring/monitor_pbs_coverage_test.go @@ -16,6 +16,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + "github.com/rcourtman/pulse-go-rewrite/pkg/metrics" "github.com/rcourtman/pulse-go-rewrite/pkg/pbs" ) @@ -113,6 +114,79 @@ func TestPollPBSInstanceDoesNotQueryExcludedDatastoreDetails(t *testing.T) { } } +// The PBS service's advertised target is agent:, not the +// unrelated Pulse Agent UUID. An API-only PBS or an uncorrelated API+Agent PBS +// must not offer a permanently empty CPU/memory history when node status is +// available. Keep failed node-status polls from writing fabricated zeroes. +func TestPollPBSNodeStatusRecordsServiceHistory(t *testing.T) { + fixture := newPBSHealthTestServer(t) + instance := config.PBSInstance{Name: "pbs-history", Host: fixture.server.URL, MonitorDatastores: true} + monitor := newPBSHealthAuthorityMonitor([]config.PBSInstance{instance}) + monitor.metricsHistory = NewMetricsHistory(128, 24*time.Hour) + store, err := metrics.NewStore(metrics.DefaultConfig(t.TempDir())) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + monitor.metricsStore = store + client := newPBSHealthTestClient(t, instance.Host) + serviceID := PBSMonitorResourceID(instance.Name) + query := func() map[string][]MetricPoint { + return monitor.GetGuestMetricsForChart("agent:"+serviceID, "agent", serviceID, time.Hour) + } + + monitor.pollPBSInstance(context.Background(), instance.Name, client) + inMemory := query() + if got := inMemory["cpu"]; len(got) != 1 || got[0].Value != 15 { + t.Fatalf("PBS CPU history = %v, want one 15%% sample", got) + } + if got := inMemory["memory"]; len(got) != 1 || got[0].Value != 50 { + t.Fatalf("PBS memory history = %v, want one 50%% sample", got) + } + if got := inMemory["disk"]; len(got) != 0 { + t.Fatalf("PBS API node status invented disk samples: %v", got) + } + + // Drop the volatile ring and prove the same API history query can read the + // service samples from the persistent store. + monitor.metricsHistory = nil + persisted := query() + if len(persisted["cpu"]) != 1 || persisted["cpu"][0].Value != 15 || + len(persisted["memory"]) != 1 || persisted["memory"][0].Value != 50 { + t.Fatalf("persisted PBS service history = %v, want CPU/memory samples", persisted) + } + + fixture.setMode(pbsHealthTestNodeDenied) + monitor.pollPBSInstance(context.Background(), instance.Name, client) + denied := query() + if len(denied["cpu"]) != 1 || len(denied["memory"]) != 1 { + t.Fatalf("node-status denial added false samples: %v", denied) + } + + fixture.setMode(pbsHealthTestSuccess) + monitor.pollPBSInstance(context.Background(), instance.Name, client) + recovered := query() + if len(recovered["cpu"]) != 2 || len(recovered["memory"]) != 2 { + t.Fatalf("PBS recovery did not append observed samples: %v", recovered) + } +} + +func TestRecordPBSNodeHistoryRejectsInvalidMeasurements(t *testing.T) { + monitor := &Monitor{metricsHistory: NewMetricsHistory(16, time.Hour)} + stamp := time.Now() + for _, status := range []*pbs.NodeStatus{ + nil, + {CPU: -1, Memory: pbs.Memory{Used: 1, Total: 2}}, + {CPU: 0.2, Memory: pbs.Memory{Used: 1, Total: 0}}, + {CPU: 0.2, Memory: pbs.Memory{Used: -1, Total: 2}}, + } { + monitor.recordPBSNodeHistory("pbs-invalid", status, stamp) + } + if got := monitor.GetGuestMetrics("agent:pbs-invalid", time.Hour); len(got) != 0 { + t.Fatalf("invalid PBS status produced history: %v", got) + } +} + // The hostname a PBS node reports about itself (GET /nodes) is // machine-identity evidence for connected-system grouping: it merges the // host agent running on the PBS machine into the PBS connection even when diff --git a/internal/monitoring/monitor_pbs_pmg.go b/internal/monitoring/monitor_pbs_pmg.go index 0b939f2d56..0f3c048ca5 100644 --- a/internal/monitoring/monitor_pbs_pmg.go +++ b/internal/monitoring/monitor_pbs_pmg.go @@ -4,6 +4,7 @@ import ( "context" stderrors "errors" "fmt" + "math" "strings" "time" @@ -11,6 +12,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/logging" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring/errors" + "github.com/rcourtman/pulse-go-rewrite/pkg/metrics" "github.com/rcourtman/pulse-go-rewrite/pkg/pbs" "github.com/rcourtman/pulse-go-rewrite/pkg/pmg" "github.com/rs/zerolog" @@ -345,6 +347,7 @@ func (m *Monitor) pollPBSInstance(ctx context.Context, instanceName string, clie pbsInst.MemoryTotal = nodeStatus.Memory.Total } pbsInst.Uptime = nodeStatus.Uptime + m.recordPBSNodeHistory(pbsInst.ID, nodeStatus, time.Now()) log.Debug(). Str("instance", instanceName). @@ -606,6 +609,37 @@ func (m *Monitor) pollPBSInstance(ctx context.Context, instanceName string, clie } } +// recordPBSNodeHistory writes the API-observed CPU and memory measurements to +// the PBS service's existing metrics target (agent:). When no +// host agent can be corroborated, the Backups drawer still has useful history +// instead of querying an unwritten key. Host-agent history remains separate and +// takes precedence in the drawer when a unique host is linked. A failed or +// incomplete node-status response must never produce an apparent zero sample. +func (m *Monitor) recordPBSNodeHistory(serviceID string, status *pbs.NodeStatus, observedAt time.Time) { + serviceID = strings.TrimSpace(serviceID) + if m == nil || status == nil || serviceID == "" || status.Memory.Total <= 0 || + status.Memory.Used < 0 || status.CPU < 0 || math.IsNaN(status.CPU) || math.IsInf(status.CPU, 0) { + return + } + + cpuPercent := status.CPU + if cpuPercent <= 1 { + cpuPercent *= 100 + } + memoryPercent := float64(status.Memory.Used) / float64(status.Memory.Total) * 100 + if m.metricsHistory != nil { + key := "agent:" + serviceID + m.metricsHistory.AddGuestMetric(key, "cpu", cpuPercent, observedAt) + m.metricsHistory.AddGuestMetric(key, "memory", memoryPercent, observedAt) + } + if m.metricsStore != nil { + m.metricsStore.WriteBatchBounded([]metrics.WriteMetric{ + {ResourceType: "agent", ResourceID: serviceID, MetricType: "cpu", Value: cpuPercent, Timestamp: observedAt, Tier: metrics.TierRaw}, + {ResourceType: "agent", ResourceID: serviceID, MetricType: "memory", Value: memoryPercent, Timestamp: observedAt, Tier: metrics.TierRaw}, + }) + } +} + // publishPBSConnectionOutcome projects the final connectivity result into every // legacy PBS health consumer. Inventory collection may be partial, but the // connection map, dashboard resource, and legacy alert evaluator must all use