From 2b157b6e372e0dab7e8e42a91bce8e6d8620dfe3 Mon Sep 17 00:00:00 2001 From: Andrei Matei Date: Wed, 1 Mar 2023 16:50:19 -0500 Subject: [PATCH] server/status: reference tenant registries in MetricRecorder Before this patch, the MetricRecorded was a recursive structure - a server's Recorder contained references to the recorders of shared-proc tenants. This was confusing - a recorder does not need the functionality of child recorders (no methods of the child recorders were used); it only needs a reference to the child registries. Also, a recorder also had a number of child registries, so having both registries and other recorders as children was extra dubious. This patch makes the MetricRecorder reference the tenants' registries directly. Release note: None --- pkg/server/status/recorder.go | 14 ++++++++------ pkg/server/status/recorder_test.go | 12 ++++++------ pkg/server/tenant.go | 4 ++-- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/pkg/server/status/recorder.go b/pkg/server/status/recorder.go index f0cc6a0bb5fb..22fae0c46b6f 100644 --- a/pkg/server/status/recorder.go +++ b/pkg/server/status/recorder.go @@ -141,7 +141,8 @@ type MetricsRecorder struct { storeRegistries map[roachpb.StoreID]*metric.Registry stores map[roachpb.StoreID]storeMetrics - tenantRecorders map[roachpb.TenantID]*MetricsRecorder + // tenantRegistries contains the registries for shared-process tenants. + tenantRegistries map[roachpb.TenantID]*metric.Registry } // WriteNodeStatus is a potentially long-running method (with a network @@ -176,15 +177,16 @@ func NewMetricsRecorder( } mr.mu.storeRegistries = make(map[roachpb.StoreID]*metric.Registry) mr.mu.stores = make(map[roachpb.StoreID]storeMetrics) - mr.mu.tenantRecorders = make(map[roachpb.TenantID]*MetricsRecorder) + mr.mu.tenantRegistries = make(map[roachpb.TenantID]*metric.Registry) return mr } -func (mr *MetricsRecorder) AddTenantRecorder(rec *MetricsRecorder) { +// AddTenantRegistry adds shared-process tenant's registry. +func (mr *MetricsRecorder) AddTenantRegistry(tenantID roachpb.TenantID, rec *metric.Registry) { mr.mu.Lock() defer mr.mu.Unlock() - mr.mu.tenantRecorders[rec.tenantID] = rec + mr.mu.tenantRegistries[tenantID] = rec } // AddNode adds the Registry from an initialized node, along with its descriptor @@ -274,8 +276,8 @@ func (mr *MetricsRecorder) ScrapeIntoPrometheus(pm *metric.PrometheusExporter) { for _, reg := range mr.mu.storeRegistries { pm.ScrapeRegistry(reg, includeChildMetrics) } - for _, ten := range mr.mu.tenantRecorders { - ten.ScrapeIntoPrometheus(pm) + for _, tenantRegistry := range mr.mu.tenantRegistries { + pm.ScrapeRegistry(tenantRegistry, includeChildMetrics) } } diff --git a/pkg/server/status/recorder_test.go b/pkg/server/status/recorder_test.go index e925f3506c4f..60975b2f3514 100644 --- a/pkg/server/status/recorder_test.go +++ b/pkg/server/status/recorder_test.go @@ -120,31 +120,31 @@ func TestMetricsRecorderTenants(t *testing.T) { nodeDescTenant := roachpb.NodeDescriptor{ NodeID: roachpb.NodeID(1), } - reg2 := metric.NewRegistry() + regTenant := metric.NewRegistry() stTenant := cluster.MakeTestingClusterSettings() - id, err := roachpb.MakeTenantID(123) + tenantID, err := roachpb.MakeTenantID(123) require.NoError(t, err) appNameContainer := roachpb.NewTenantNameContainer("application") recorderTenant := NewMetricsRecorder( - id, + tenantID, appNameContainer, nil, /* nodeLiveness */ nil, /* remoteClocks */ manual, stTenant, ) - recorderTenant.AddNode(reg2, nodeDescTenant, 50, "foo:26257", "foo:26258", "foo:5432") + recorderTenant.AddNode(regTenant, nodeDescTenant, 50, "foo:26257", "foo:26258", "foo:5432") g := metric.NewGauge(metric.Metadata{Name: "some_metric"}) reg1.AddMetric(g) g.Update(123) g2 := metric.NewGauge(metric.Metadata{Name: "some_metric"}) - reg2.AddMetric(g2) + regTenant.AddMetric(g2) g2.Update(456) - recorder.AddTenantRecorder(recorderTenant) + recorder.AddTenantRegistry(tenantID, regTenant) buf := bytes.NewBuffer([]byte{}) err = recorder.PrintAsText(buf) diff --git a/pkg/server/tenant.go b/pkg/server/tenant.go index cf923fbe4953..f110e9b79feb 100644 --- a/pkg/server/tenant.go +++ b/pkg/server/tenant.go @@ -1071,11 +1071,11 @@ func makeTenantSQLServerArgs( sqlCfg.TenantID, tenantNameContainer, nil /* nodeLiveness */, nil, /* remoteClocks */ clock.WallClock(), st) // Note: If the tenant is in-process, we attach this tenant's metric - // recorder to the parentRecorder held by the system tenant. This + // registry to the parentRecorder held by the system tenant. This // ensures that generated Prometheus metrics from the system tenant // include metrics from this in-process tenant. if parentRecorder != nil { - parentRecorder.AddTenantRecorder(recorder) + parentRecorder.AddTenantRegistry(sqlCfg.TenantID, registry) } var runtime *status.RuntimeStatSampler