From b4be3bd55bd4da02d01621b6f12485dd67b507bc Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:28:57 +0100 Subject: [PATCH] Keep one timeline per resource in mock chart history Mock chart history thinned each metric with LTTB on its own. LTTB keeps the points whose values stand out, so cpu, memory and disk of one guest kept different timestamps: on the public demo a 24h container history had eight ~48-point series spread over 162 distinct timestamps. The drawer History panels break a line wherever another series in the panel has a reading this one lacks, so every mock chart drew as dots with a "Missing observations" note. Real history does not do this because the poller writes all of a resource's metrics at the same times. Thin mock series onto an epoch-aligned grid instead: keep the first reading in each bucket plus the newest reading, at their own timestamps. Which readings survive now depends only on when they were taken, so series sampled together keep identical timestamps, values stay real readings, and point targets are unchanged. The synthetic generator no longer pre-thins to 500 points, since every caller thins onto the grid. --- .../v6/internal/subsystems/monitoring.md | 16 ++ .../monitoring/canonical_guardrails_test.go | 2 +- internal/monitoring/mock_chart_history.go | 61 +++++- .../monitoring/mock_metrics_history_test.go | 12 +- internal/monitoring/monitor_metrics.go | 9 +- .../monitoring/monitor_metrics_slo_test.go | 205 ++++++++++++++++++ 6 files changed, 291 insertions(+), 14 deletions(-) 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 +}