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) + } + } +}