diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 2679af1246..18317ecfc2 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -605,6 +605,22 @@ source selection and wire formats are unchanged. The History cleanup tests cover every supported series, byte-array reclamation and connected readers; these controls do not attribute a reporter's native process-memory growth. +### Mock chart history keeps one timeline per resource + +Mock chart history thins every series of a resource onto one wall-clock grid +(`sampleMetricSeriesOnMockChartGrid` in `internal/monitoring/mock_chart_history.go`, +called by `downsampleMetricSeriesForMockChart`). It keeps the first reading in +each epoch-aligned bucket and the newest reading, each at its own timestamp, so +which readings survive depends only on when they were taken, never on their values. +The poller writes all of a resource's metrics at the same times, and the drawer +History panels break a line where another series in the panel has a reading this +one lacks; per-series LTTB picked value-dependent timestamps, so mock guests and +nodes drew as dots with "Missing observations" on the public demo. Live poller +history, the store query path, chart point targets and mock values are unchanged. +`TestMockChartHistorySharesTimestampsAcrossMetrics` in `monitor_metrics_slo_test.go` +pins identical cpu/memory/disk/network/disk-I/O timestamps for a mock VM, container +and node, on synthetic and seeded history, at 1h, 4h, 24h and 7d. + ### Podman interval CPU selection — issue #2447 Every Podman one-shot response uses consecutive cumulative CPU-time nanoseconds diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index e6cd1787e5..49aebf65be 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -2182,7 +2182,7 @@ func TestMockNativePollersDeferToCanonicalMockSampler(t *testing.T) { snippets: []string{ "func mockCanonicalMetricSeries(resourceType, resourceID, metricType string, timestamps []time.Time) []MetricPoint {", "values := canonicalMetricSeries(resourceType, resourceID, metricType, timestamps)", - "return lttb(points, chartDownsampleTarget)", + "func sampleMetricSeriesOnMockChartGrid(points []MetricPoint, interval time.Duration) []MetricPoint {", `usageValues := canonicalMetricSeries("storage", storageID, "usage", timestamps)`, }, }, diff --git a/internal/monitoring/mock_chart_history.go b/internal/monitoring/mock_chart_history.go index 500e1ed165..1bc5df509e 100644 --- a/internal/monitoring/mock_chart_history.go +++ b/internal/monitoring/mock_chart_history.go @@ -67,7 +67,58 @@ func mockCanonicalMetricSeries(resourceType, resourceID, metricType string, time } } - return lttb(points, chartDownsampleTarget) + return points +} + +// mockChartGridInterval is the bucket width that keeps a mock chart within +// mockChartPointTarget points. A window that does not start on a bucket edge +// touches one more bucket than it spans, and the newest reading is kept on top +// of one reading per bucket, hence target-2. +func mockChartGridInterval(duration time.Duration) time.Duration { + if duration <= 0 { + duration = time.Hour + } + buckets := time.Duration(mockChartPointTarget(duration) - 2) + if buckets < 1 { + buckets = 1 + } + interval := (duration + buckets - 1) / buckets + return ((interval + time.Second - 1) / time.Second) * time.Second +} + +// sampleMetricSeriesOnMockChartGrid keeps the first reading in each +// wall-clock bucket of the given width, plus the newest reading so the chart +// ends at the live value. Buckets are aligned to the Unix epoch and readings +// keep their own timestamps, so which readings survive depends only on when +// they were taken, never on their values: every metric of one resource +// sampled at the same times keeps the same timestamps, as the poller writes +// them. points must be in ascending timestamp order. +func sampleMetricSeriesOnMockChartGrid(points []MetricPoint, interval time.Duration) []MetricPoint { + if len(points) == 0 { + return nil + } + width := int64(interval) + if width <= 0 { + return cloneMetricSeries(points) + } + + bucketOf := func(ts time.Time) int64 { + nanos := ts.UnixNano() + bucket := nanos / width + if nanos < 0 && nanos%width != 0 { + bucket-- + } + return bucket + } + + sampled := make([]MetricPoint, 0, len(points)) + for i, point := range points { + if i > 0 && i < len(points)-1 && bucketOf(point.Timestamp) == bucketOf(points[i-1].Timestamp) { + continue + } + sampled = append(sampled, point) + } + return sampled } func mockGuestMetricsForChart(resourceType, resourceID string, duration time.Duration) map[string][]MetricPoint { @@ -224,9 +275,9 @@ func (m *Monitor) mockStorageMetricsForChart(storageID string, duration time.Dur } return map[string][]MetricPoint{ - "usage": lttb(usagePoints, chartDownsampleTarget), - "used": lttb(usedPoints, chartDownsampleTarget), - "avail": lttb(availPoints, chartDownsampleTarget), - "total": lttb(totalPoints, chartDownsampleTarget), + "usage": usagePoints, + "used": usedPoints, + "avail": availPoints, + "total": totalPoints, } } diff --git a/internal/monitoring/mock_metrics_history_test.go b/internal/monitoring/mock_metrics_history_test.go index fbd475d8b2..c9bc0fdf3d 100644 --- a/internal/monitoring/mock_metrics_history_test.go +++ b/internal/monitoring/mock_metrics_history_test.go @@ -1129,8 +1129,8 @@ func TestStartMockMetricsSampler_SeedsCanonicalMockResourceHistory(t *testing.T) if !ok { t.Fatalf("expected disk chart for %q, got keys=%v", proxmoxDiskID, keysDiskCharts(diskCharts)) } - if got, want := len(diskChart.Temperature), mockChartPointTarget(7*24*time.Hour); got != want { - t.Fatalf("expected downsampled proxmox disk chart history to match target %d, got %d points for %q", want, got, proxmoxDiskID) + if got, want := len(diskChart.Temperature), mockChartPointTarget(7*24*time.Hour); got > want || got < want-2 { + t.Fatalf("expected downsampled proxmox disk chart history to stay within two points of target %d, got %d points for %q", want, got, proxmoxDiskID) } dockerMetricID := strings.TrimSpace(graph.State.DockerHosts[0].Containers[0].ID) @@ -1148,11 +1148,11 @@ func TestStartMockMetricsSampler_SeedsCanonicalMockResourceHistory(t *testing.T) ) cpuPoints := workloadMetrics[dockerMetricID]["cpu"] memoryPoints := workloadMetrics[dockerMetricID]["memory"] - if got, want := len(cpuPoints), mockChartPointTarget(7*24*time.Hour); got != want { - t.Fatalf("expected downsampled docker app cpu history to match target %d, got %d points for %q", want, got, dockerMetricID) + if got, want := len(cpuPoints), mockChartPointTarget(7*24*time.Hour); got > want || got < want-2 { + t.Fatalf("expected downsampled docker app cpu history to stay within two points of target %d, got %d points for %q", want, got, dockerMetricID) } - if got, want := len(memoryPoints), mockChartPointTarget(7*24*time.Hour); got != want { - t.Fatalf("expected downsampled docker app memory history to match target %d, got %d points for %q", want, got, dockerMetricID) + if got, want := len(memoryPoints), mockChartPointTarget(7*24*time.Hour); got > want || got < want-2 { + t.Fatalf("expected downsampled docker app memory history to stay within two points of target %d, got %d points for %q", want, got, dockerMetricID) } } diff --git a/internal/monitoring/monitor_metrics.go b/internal/monitoring/monitor_metrics.go index 4b0c811f95..1d4d4d767b 100644 --- a/internal/monitoring/monitor_metrics.go +++ b/internal/monitoring/monitor_metrics.go @@ -953,9 +953,14 @@ func mockChartPointTarget(duration time.Duration) int { } } +// downsampleMetricSeriesForMockChart thins a mock series onto the shared mock +// chart grid (see sampleMetricSeriesOnMockChartGrid). It does not use LTTB: +// LTTB keeps the points whose values stand out, so each metric of a resource +// kept its own timestamps, and drawer history panels, which break a line +// where another series in the panel has a reading this one lacks, drew mock +// charts as scattered dots. func downsampleMetricSeriesForMockChart(points []MetricPoint, duration time.Duration) []MetricPoint { - target := mockChartPointTarget(duration) - return lttb(cloneMetricSeries(points), target) + return sampleMetricSeriesOnMockChartGrid(points, mockChartGridInterval(duration)) } func downsampleMetricMapForMockChart(metrics map[string][]MetricPoint, duration time.Duration) map[string][]MetricPoint { diff --git a/internal/monitoring/monitor_metrics_slo_test.go b/internal/monitoring/monitor_metrics_slo_test.go index 756177dd3a..dc3478495a 100644 --- a/internal/monitoring/monitor_metrics_slo_test.go +++ b/internal/monitoring/monitor_metrics_slo_test.go @@ -3,6 +3,7 @@ package monitoring import ( "context" "fmt" + "math" "os" "sort" "testing" @@ -853,3 +854,207 @@ func TestMockStorageChartReadsTheEstateOnlyForAnUncachedChart(t *testing.T) { t.Fatalf("a cached chart read the estate again: %d reads", reads) } } + +// The poller writes every metric of a resource at the same timestamps, and +// the drawer history panels break a line wherever another series in the panel +// has a reading this one lacks. Mock history must therefore keep one shared +// set of timestamps per resource, or every mock chart draws as dots. +var ( + mockGuestSharedTimestampMetrics = []string{"cpu", "memory", "memoryused", "disk", "diskread", "diskwrite", "netin", "netout"} + mockNodeSharedTimestampMetrics = []string{"cpu", "memory", "disk", "netin", "netout"} + mockSharedTimestampDurations = []time.Duration{time.Hour, 4 * time.Hour, 24 * time.Hour, 7 * 24 * time.Hour} +) + +func TestSampleMetricSeriesOnMockChartGridKeepsTimestampsIndependentOfValues(t *testing.T) { + start := time.Date(2026, time.October, 13, 0, 0, 0, 0, time.UTC) + flat := make([]MetricPoint, 0, 721) + spiky := make([]MetricPoint, 0, 721) + for i := 0; i <= 720; i++ { + ts := start.Add(time.Duration(i) * 2 * time.Minute) + flat = append(flat, MetricPoint{Timestamp: ts, Value: 20 + math.Sin(float64(i)/40)}) + value := 5.0 + if i%37 == 0 || i%53 == 0 { + value = 90 + } + spiky = append(spiky, MetricPoint{Timestamp: ts, Value: value}) + } + + // Control: LTTB picks points by value, so these two series keep + // different timestamps. This is the behaviour that broke mock charts. + if sameMetricTimestamps(lttb(cloneMetricSeries(flat), 48), lttb(cloneMetricSeries(spiky), 48)) { + t.Fatal("control: expected LTTB to keep different timestamps for differently shaped series") + } + + interval := mockChartGridInterval(24 * time.Hour) + gridFlat := sampleMetricSeriesOnMockChartGrid(flat, interval) + gridSpiky := sampleMetricSeriesOnMockChartGrid(spiky, interval) + if !sameMetricTimestamps(gridFlat, gridSpiky) { + t.Fatalf("grid sampling kept different timestamps: %v vs %v", metricTimestamps(gridFlat), metricTimestamps(gridSpiky)) + } + if got, limit := len(gridFlat), mockChartPointTarget(24*time.Hour); got > limit || got < limit-2 { + t.Fatalf("expected grid sampling to stay within two points of %d, got %d", limit, got) + } + if first := gridSpiky[0]; first != spiky[0] { + t.Fatalf("expected the first reading to be kept as-is, got %+v want %+v", first, spiky[0]) + } + if last, newest := gridSpiky[len(gridSpiky)-1], spiky[len(spiky)-1]; last != newest { + t.Fatalf("expected the newest reading to be kept as-is, got %+v want %+v", last, newest) + } + readings := make(map[int64]float64, len(spiky)) + for _, point := range spiky { + readings[point.Timestamp.UnixNano()] = point.Value + } + for i, point := range gridSpiky { + if i > 0 && !point.Timestamp.After(gridSpiky[i-1].Timestamp) { + t.Fatalf("grid timestamps not strictly ascending at %d", i) + } + if value, ok := readings[point.Timestamp.UnixNano()]; !ok || value != point.Value { + t.Fatalf("grid point %+v is not a reading from the input series", point) + } + } +} + +func TestMockChartHistorySharesTimestampsAcrossMetrics(t *testing.T) { + previousEnabled := mock.IsMockEnabled() + previousConfig := mock.GetConfig() + t.Cleanup(func() { + mustSetMockEnabled(t, false) + mock.SetMockConfig(previousConfig) + if previousEnabled { + mustSetMockEnabled(t, true) + mock.SetMockConfig(previousConfig) + } + }) + + // The first mock node is "mixed": at least base-2 VMs and base-3 LXCs. + cfg := compactMockChartFixtureConfig() + cfg.VMsPerNode = 3 + cfg.LXCsPerNode = 4 + mustSetMockEnabled(t, false) + mock.SetMockConfig(cfg) + mustSetMockEnabled(t, true) + + graph := mock.CurrentFixtureGraph() + if len(graph.State.Containers) == 0 || len(graph.State.VMs) == 0 || len(graph.State.Nodes) == 0 { + t.Fatalf( + "expected mock nodes, VMs and containers in the fixture graph, got %d nodes, %d VMs, %d containers", + len(graph.State.Nodes), len(graph.State.VMs), len(graph.State.Containers), + ) + } + container := graph.State.Containers[0].ID + vm := graph.State.VMs[0].ID + node := graph.State.Nodes[0].ID + + newMonitor := func() *Monitor { + resources, _ := mock.UnifiedResourceSnapshot() + registry := unifiedresources.NewRegistry(nil) + registry.IngestResources(resources) + return &Monitor{ + metricsHistory: NewMetricsHistory(1000, 24*time.Hour), + state: models.NewState(), + resourceStore: unifiedresources.NewMonitorAdapter(registry), + } + } + + check := func(t *testing.T, monitor *Monitor) { + for _, duration := range mockSharedTimestampDurations { + // The newest reading sits on the current minute; rerun if the + // minute turned over between two per-metric node reads. + for attempt := 0; ; attempt++ { + minute := time.Now().Truncate(time.Minute) + monitor.invalidateMockChartCaches() + guests := map[string]map[string][]MetricPoint{ + "container " + container: monitor.GetGuestMetricsForChart(container, "container", container, duration), + "vm " + vm: monitor.GetGuestMetricsForChart(vm, "vm", vm, duration), + } + nodeSeries := make(map[string][]MetricPoint, len(mockNodeSharedTimestampMetrics)) + for _, metric := range mockNodeSharedTimestampMetrics { + nodeSeries[metric] = monitor.GetNodeMetricsForChart(node, metric, duration) + } + if !time.Now().Truncate(time.Minute).Equal(minute) && attempt < 2 { + continue + } + for name, series := range guests { + assertMockSeriesShareTimestamps(t, name, duration, series, mockGuestSharedTimestampMetrics) + } + assertMockSeriesShareTimestamps(t, "node "+node, duration, nodeSeries, mockNodeSharedTimestampMetrics) + break + } + } + } + + t.Run("synthetic history", func(t *testing.T) { + check(t, newMonitor()) + }) + + t.Run("seeded history", func(t *testing.T) { + monitor := newMonitor() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + monitor.startMockMetricsSampler(ctx) + t.Cleanup(func() { monitor.stopMockMetricsSampler() }) + check(t, monitor) + }) +} + +func assertMockSeriesShareTimestamps(t *testing.T, resource string, duration time.Duration, series map[string][]MetricPoint, metrics []string) { + t.Helper() + reference := series["cpu"] + if len(reference) < 2 { + t.Fatalf("%s %s: expected a cpu history, got %d points", resource, duration, len(reference)) + } + minValue, maxValue := reference[0].Value, reference[0].Value + for _, point := range reference { + minValue = math.Min(minValue, point.Value) + maxValue = math.Max(maxValue, point.Value) + } + if minValue == maxValue { + t.Fatalf("%s %s: cpu history is flat at %v; mock values should still vary", resource, duration, minValue) + } + for _, metric := range metrics { + points := series[metric] + if len(points) == 0 { + t.Fatalf("%s %s: expected a %s history", resource, duration, metric) + } + if !sameMetricTimestamps(reference, points) { + t.Fatalf( + "%s %s: %s timestamps diverge from cpu (%d vs %d points, %d shared)", + resource, duration, metric, len(points), len(reference), sharedMetricTimestampCount(reference, points), + ) + } + } +} + +func metricTimestamps(points []MetricPoint) []int64 { + out := make([]int64, len(points)) + for i, point := range points { + out[i] = point.Timestamp.UnixNano() + } + return out +} + +func sameMetricTimestamps(a, b []MetricPoint) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if !a[i].Timestamp.Equal(b[i].Timestamp) { + return false + } + } + return true +} + +func sharedMetricTimestampCount(a, b []MetricPoint) int { + seen := make(map[int64]struct{}, len(a)) + for _, point := range a { + seen[point.Timestamp.UnixNano()] = struct{}{} + } + shared := 0 + for _, point := range b { + if _, ok := seen[point.Timestamp.UnixNano()]; ok { + shared++ + } + } + return shared +}