From a9f1026656858713d16d8496a803b79d2e39fa1f Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Sat, 17 Oct 2026 12:41:12 +0100 Subject: [PATCH] Keep the metrics DB path override on the default org PULSE_METRICS_DB_PATH sets Config.MetricsDBPath, and GetMonitor builds each non-default tenant from a deep copy of the base config, which kept that path. monitoring.New then opened the same metrics.db for every organization, so per-org charts, reports and Assistant summaries read from one shared store. Clear MetricsDBPath on non-default tenant configs so each org opens metrics.db under its own data directory. Org deletion already removes that directory, so its metrics go with it. The override still moves the default org's database, which is the single-install case it exists for. --- docs/CONFIGURATION.md | 3 +- .../v6/internal/subsystems/monitoring.md | 25 ++++- frontend-modern/public/docs/CONFIGURATION.md | 3 +- .../monitoring/canonical_guardrails_test.go | 13 +++ internal/monitoring/multi_tenant_monitor.go | 8 ++ .../multi_tenant_monitor_metrics_path_test.go | 99 +++++++++++++++++++ 6 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 internal/monitoring/multi_tenant_monitor_metrics_path_test.go diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index aa362722ce..7002eae391 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -55,7 +55,8 @@ Path overrides: - `PULSE_DATA_DIR` sets the base directory for `system.json`, encrypted files, and the bootstrap token. - `PULSE_METRICS_DB_PATH` sets only the metrics SQLite database path. Use this for tmpfs-backed metrics history without moving secrets or config off the - persistent data directory. + persistent data directory. It applies to the default org only; each + non-default org keeps its own `metrics.db` in `/orgs//`. Multi-tenant layout: - Default org uses the root data directory for backward compatibility. diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 7b5731b673..10c4eace7f 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -624,6 +624,27 @@ These synthetic source controls reproduce a family matching #2382's second operator response, not that operator's cause, the original downgrade-resolved regression, native appliance acceptance or release availability. +### Metrics database override stays on the default organization + +`PULSE_METRICS_DB_PATH` names one SQLite file, so it applies only to the +default organization's monitor. `MultiTenantMonitor.GetMonitor` clears +`MetricsDBPath` on each non-default tenant's detached config copy after +setting its `DataPath`, and `monitoring.New` then opens that organization's +store at `/metrics.db`. Before this, the deep copy carried the +override into every tenant and all organizations wrote to and read from one +metrics database, so per-org charts, reports and Pulse Assistant summaries +could return another organization's series. Keeping tenant stores inside the +org directory also means `DeleteOrganization` removes that org's metrics with +the rest of its data. The trade-off is that tmpfs or RAM-disk placement +through the override covers the default org only; non-default org metrics +stay on the data volume. The default org's override path, rollup interval and +retention handling are unchanged. Regression coverage: +`internal/monitoring/multi_tenant_monitor_metrics_path_test.go` sets the +override, provisions two orgs, and proves each opens its own `metrics.db` +and cannot read series written by the default org or by the other org. +`TestMetricsStoreRuntimeOverridesStayOnMonitorBoundary` also requires the +clear to sit between the tenant `DataPath` assignment and `New`. + ### Lightweight detached History reads Guest, node, disk and storage History readers allocate one exact-sized snapshot @@ -5186,7 +5207,9 @@ aggregation cadence: `internal/monitoring/monitor.go` may pass the resolved data path, `PULSE_METRICS_DB_PATH`, and `PULSE_METRICS_ROLLUP_INTERVAL` through to `pkg/metrics`, but the SQLite path normalization, rollup bounds, and write amplification policy stay owned by the metrics store rather than by a -monitoring-local helper. +monitoring-local helper. The `PULSE_METRICS_DB_PATH` handoff reaches only the +default org's monitor; non-default tenants always resolve `metrics.db` from +their own data path. Install-wide telemetry counts are also monitoring-owned now. Any telemetry or reporting surface that claims installation totals must aggregate across the provisioned tenant set through the reloadable multi-tenant monitor boundary, diff --git a/frontend-modern/public/docs/CONFIGURATION.md b/frontend-modern/public/docs/CONFIGURATION.md index aa362722ce..7002eae391 100644 --- a/frontend-modern/public/docs/CONFIGURATION.md +++ b/frontend-modern/public/docs/CONFIGURATION.md @@ -55,7 +55,8 @@ Path overrides: - `PULSE_DATA_DIR` sets the base directory for `system.json`, encrypted files, and the bootstrap token. - `PULSE_METRICS_DB_PATH` sets only the metrics SQLite database path. Use this for tmpfs-backed metrics history without moving secrets or config off the - persistent data directory. + persistent data directory. It applies to the default org only; each + non-default org keeps its own `metrics.db` in `/orgs//`. Multi-tenant layout: - Default org uses the root data directory for backward compatibility. diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index 87b9b9ae78..6ee5e04d3c 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -681,6 +681,19 @@ func TestMetricsStoreRuntimeOverridesStayOnMonitorBoundary(t *testing.T) { t.Fatalf("monitor.go must contain %q", snippet) } } + + // The override names one file, so non-default tenants must not inherit it. + data, err = os.ReadFile("multi_tenant_monitor.go") + if err != nil { + t.Fatalf("failed to read multi_tenant_monitor.go: %v", err) + } + tenantSource := string(data) + dataPathIdx := strings.Index(tenantSource, "tenantConfig.DataPath = tenantPersistence.GetConfigDir()") + clearIdx := strings.Index(tenantSource, "tenantConfig.MetricsDBPath = \"\"") + newIdx := strings.Index(tenantSource, "monitor, err = New(tenantConfig)") + if dataPathIdx < 0 || clearIdx < 0 || newIdx < 0 || !(dataPathIdx < clearIdx && clearIdx < newIdx) { + t.Fatal("multi_tenant_monitor.go must clear MetricsDBPath for non-default tenants after setting DataPath and before New") + } } func TestEscalationDeliveryDefersToCanonicalAlertSuppression(t *testing.T) { diff --git a/internal/monitoring/multi_tenant_monitor.go b/internal/monitoring/multi_tenant_monitor.go index a11950ad1f..430a391ab0 100644 --- a/internal/monitoring/multi_tenant_monitor.go +++ b/internal/monitoring/multi_tenant_monitor.go @@ -220,6 +220,14 @@ func (mtm *MultiTenantMonitor) GetMonitor(orgID string) (*Monitor, error) { return nil, fmt.Errorf("failed to get persistence for org %s: %w", orgID, err) } tenantConfig.DataPath = tenantPersistence.GetConfigDir() + if orgID != "default" { + // PULSE_METRICS_DB_PATH names one database file. Inherited by every + // tenant it would put all organizations' series in the same store, so + // the override applies to the default org only. Other orgs keep + // metrics.db under their own data directory, where org deletion also + // removes it. + tenantConfig.MetricsDBPath = "" + } // Load tenant-specific nodes from /nodes.enc nodesConfig, err := tenantPersistence.LoadNodesConfig() diff --git a/internal/monitoring/multi_tenant_monitor_metrics_path_test.go b/internal/monitoring/multi_tenant_monitor_metrics_path_test.go new file mode 100644 index 0000000000..8a5028e126 --- /dev/null +++ b/internal/monitoring/multi_tenant_monitor_metrics_path_test.go @@ -0,0 +1,99 @@ +package monitoring + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/pkg/metrics" +) + +// PULSE_METRICS_DB_PATH names a single database file. Only the default org may +// use it; every other org must keep its own metrics.db under its data +// directory so organizations never read each other's series. +func TestMultiTenantMonitorMetricsDBPathOverrideStaysOnDefaultOrg(t *testing.T) { + mtp, defaultPersistence := newTestTenantPersistence(t) + overridePath := filepath.Join(t.TempDir(), "metrics-tmpfs", "metrics.db") + baseCfg := &config.Config{ + DataPath: defaultPersistence.GetConfigDir(), + MetricsDBPath: overridePath, + } + mtm := NewMultiTenantMonitor(baseCfg, mtp, nil) + t.Cleanup(mtm.Stop) + + orgIDs := []string{"org-a", "org-b"} + for _, orgID := range orgIDs { + if err := mtp.SaveOrganization(&models.Organization{ID: orgID, DisplayName: orgID}); err != nil { + t.Fatalf("SaveOrganization(%s) error = %v", orgID, err) + } + } + + stores := make(map[string]*metrics.Store) + for _, orgID := range append([]string{"default"}, orgIDs...) { + monitor, err := mtm.GetMonitor(orgID) + if err != nil { + t.Fatalf("GetMonitor(%s) error = %v", orgID, err) + } + store := monitor.GetMetricsStore() + if store == nil { + t.Fatalf("GetMonitor(%s) has no metrics store", orgID) + } + stores[orgID] = store + } + + if baseCfg.MetricsDBPath != overridePath { + t.Fatalf("base config MetricsDBPath = %q, want %q", baseCfg.MetricsDBPath, overridePath) + } + if _, err := os.Stat(overridePath); err != nil { + t.Fatalf("default org should open the override path %q: %v", overridePath, err) + } + for _, orgID := range orgIDs { + orgPersistence, err := mtp.GetPersistence(orgID) + if err != nil { + t.Fatalf("GetPersistence(%s) error = %v", orgID, err) + } + orgDBPath := filepath.Join(orgPersistence.GetConfigDir(), "metrics.db") + if _, err := os.Stat(orgDBPath); err != nil { + t.Fatalf("%s should open its own metrics database at %q: %v", orgID, orgDBPath, err) + } + } + if stores["org-a"] == stores["org-b"] || stores["org-a"] == stores["default"] { + t.Fatal("expected a distinct metrics store per organization") + } + + now := time.Now().Truncate(time.Second) + for _, orgID := range []string{"default", "org-a"} { + stores[orgID].WriteBatchSync([]metrics.WriteMetric{{ + ResourceType: "vm", + ResourceID: "vm-" + orgID, + MetricType: "cpu", + Value: 42, + Timestamp: now, + Tier: metrics.TierRaw, + }}) + } + + for _, tc := range []struct { + orgID string + resourceID string + wantPoints bool + }{ + {orgID: "default", resourceID: "vm-default", wantPoints: true}, + {orgID: "default", resourceID: "vm-org-a", wantPoints: false}, + {orgID: "org-a", resourceID: "vm-org-a", wantPoints: true}, + {orgID: "org-a", resourceID: "vm-default", wantPoints: false}, + {orgID: "org-b", resourceID: "vm-org-a", wantPoints: false}, + {orgID: "org-b", resourceID: "vm-default", wantPoints: false}, + } { + points, err := stores[tc.orgID].Query("vm", tc.resourceID, "cpu", now.Add(-time.Minute), now.Add(time.Minute), 0) + if err != nil { + t.Fatalf("%s Query(%s) error = %v", tc.orgID, tc.resourceID, err) + } + if got := len(points) > 0; got != tc.wantPoints { + t.Fatalf("%s store points for %s = %d, want present=%v", tc.orgID, tc.resourceID, len(points), tc.wantPoints) + } + } +}