From 4dd0304c30f58d1cb290bc6edce66939a8e30bd9 Mon Sep 17 00:00:00 2001 From: Alex Barganier Date: Tue, 28 Mar 2023 16:28:41 -0400 Subject: [PATCH] tsdb: add tenant-level store metrics to tsdb Previously, while we had the ability to show tenant-level store metrics in the `/_status/vars` page, these metrics were never written to tsdb. This is despite the changes in https://github.com/cockroachdb/cockroach/pull/98077, which did a great job at writing all the metrics in the tenant-specific metric registries, but didn't pull the tenant-specific store metrics out of the store registries. This is because these metrics exist as *child* metrics on the store registry metrics, and we did not previously have logic to individually pick these metrics out of their parent AggGauge/Counter metrics. This patch adds the logic to do so. Now, for each tenant ID that exists in the recorder's `tenantRegistries` map, we will attempt to pick that tenant's individual child metric & values from all the metrics that exist in TenantsStorageMetrics. This will limit the writing of these tenant-level metrics to only happen in deployments where multiple tenants are running in-process, as environments such as serverless clusters are expected to have an empty `tenantRegistries` map in this recorder. Release note: none --- pkg/kv/kvbase/BUILD.bazel | 5 +- pkg/kv/kvbase/metrics.go | 17 +++ pkg/kv/kvserver/metrics.go | 36 ++++++ pkg/server/status/BUILD.bazel | 6 + pkg/server/status/recorder.go | 79 ++++++++++++++ pkg/server/status/recorder_test.go | 145 +++++++++++++++++++++++++ pkg/util/metric/prometheus_exporter.go | 2 +- pkg/util/metric/registry.go | 4 +- 8 files changed, 290 insertions(+), 4 deletions(-) create mode 100644 pkg/kv/kvbase/metrics.go diff --git a/pkg/kv/kvbase/BUILD.bazel b/pkg/kv/kvbase/BUILD.bazel index f83524b0e182..ee08aa656d19 100644 --- a/pkg/kv/kvbase/BUILD.bazel +++ b/pkg/kv/kvbase/BUILD.bazel @@ -3,7 +3,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "kvbase", - srcs = ["constants.go"], + srcs = [ + "constants.go", + "metrics.go", + ], importpath = "github.com/cockroachdb/cockroach/pkg/kv/kvbase", visibility = ["//visibility:public"], ) diff --git a/pkg/kv/kvbase/metrics.go b/pkg/kv/kvbase/metrics.go new file mode 100644 index 000000000000..8a363426496f --- /dev/null +++ b/pkg/kv/kvbase/metrics.go @@ -0,0 +1,17 @@ +// Copyright 2023 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package kvbase + +// TenantsStorageMetricsSet is the set of all metric names contained +// within TenantsStorageMetrics, recorded at the individual tenant level. +// +// Made available in kvbase to help avoid import cycles. +var TenantsStorageMetricsSet map[string]struct{} diff --git a/pkg/kv/kvserver/metrics.go b/pkg/kv/kvserver/metrics.go index ce180148f8ee..47ca43ca1463 100644 --- a/pkg/kv/kvserver/metrics.go +++ b/pkg/kv/kvserver/metrics.go @@ -18,6 +18,7 @@ import ( "time" "unsafe" + "github.com/cockroachdb/cockroach/pkg/kv/kvbase" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/allocatorimpl" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval/result" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb" @@ -37,6 +38,12 @@ import ( "go.etcd.io/raft/v3/raftpb" ) +func init() { + // Inject the TenantStorageMetricsSet available in the kvbase pkg to + // avoid import cycles. + kvbase.TenantsStorageMetricsSet = tenantsStorageMetricsSet() +} + var ( // Replica metrics. metaReplicaCount = metric.Metadata{ @@ -2204,6 +2211,8 @@ func (ref *tenantMetricsRef) assert(ctx context.Context) { // call acquire and release to properly reference count the metrics for // individual tenants. type TenantsStorageMetrics struct { + // NB: If adding more metrics to this struct, be sure to + // also update tenantsStorageMetricsSet(). LiveBytes *aggmetric.AggGauge KeyBytes *aggmetric.AggGauge ValBytes *aggmetric.AggGauge @@ -2232,6 +2241,33 @@ type TenantsStorageMetrics struct { tenants syncutil.IntMap // map[int64(roachpb.TenantID)]*tenantStorageMetrics } +// tenantsStorageMetricsSet returns the set of all metric names contained +// within TenantsStorageMetrics. +// +// see kvbase.TenantsStorageMetricsSet for public access. Assigned in init(). +func tenantsStorageMetricsSet() map[string]struct{} { + return map[string]struct{}{ + metaLiveBytes.Name: {}, + metaKeyBytes.Name: {}, + metaValBytes.Name: {}, + metaRangeKeyBytes.Name: {}, + metaRangeValBytes.Name: {}, + metaTotalBytes.Name: {}, + metaIntentBytes.Name: {}, + metaLiveCount.Name: {}, + metaKeyCount.Name: {}, + metaValCount.Name: {}, + metaRangeKeyCount.Name: {}, + metaRangeValCount.Name: {}, + metaIntentCount.Name: {}, + metaIntentAge.Name: {}, + metaGcBytesAge.Name: {}, + metaSysBytes.Name: {}, + metaSysCount.Name: {}, + metaAbortSpanBytes.Name: {}, + } +} + var _ metric.Struct = (*TenantsStorageMetrics)(nil) // MetricStruct makes TenantsStorageMetrics a metric.Struct. diff --git a/pkg/server/status/BUILD.bazel b/pkg/server/status/BUILD.bazel index 9f4f93cc7dad..ff6f2674be63 100644 --- a/pkg/server/status/BUILD.bazel +++ b/pkg/server/status/BUILD.bazel @@ -44,8 +44,10 @@ go_library( "//pkg/build", "//pkg/keys", "//pkg/kv", + "//pkg/kv/kvbase", "//pkg/kv/kvpb", "//pkg/kv/kvserver/liveness", + "//pkg/multitenant", "//pkg/roachpb", "//pkg/rpc", "//pkg/server/status/statuspb", @@ -69,6 +71,7 @@ go_library( "@com_github_cockroachdb_redact//:redact", "@com_github_dustin_go_humanize//:go-humanize", "@com_github_elastic_gosigar//:gosigar", + "@com_github_prometheus_client_model//go", "@com_github_shirou_gopsutil_v3//cpu", "@com_github_shirou_gopsutil_v3//net", ] + select({ @@ -134,6 +137,7 @@ go_test( deps = [ "//pkg/base", "//pkg/build", + "//pkg/multitenant", "//pkg/roachpb", "//pkg/security/securityassets", "//pkg/security/securitytest", @@ -143,6 +147,7 @@ go_test( "//pkg/sql/sem/catconstants", "//pkg/testutils/serverutils", "//pkg/ts/tspb", + "//pkg/ts/tsutil", "//pkg/util/leaktest", "//pkg/util/log", "//pkg/util/log/eventpb", @@ -151,6 +156,7 @@ go_test( "//pkg/util/system", "//pkg/util/timeutil", "@com_github_kr_pretty//:pretty", + "@com_github_prometheus_client_model//go", "@com_github_shirou_gopsutil_v3//net", "@com_github_stretchr_testify//require", ], diff --git a/pkg/server/status/recorder.go b/pkg/server/status/recorder.go index b20606c60e2a..31da9a4c8677 100644 --- a/pkg/server/status/recorder.go +++ b/pkg/server/status/recorder.go @@ -27,8 +27,10 @@ import ( "github.com/cockroachdb/cockroach/pkg/build" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/kv/kvbase" "github.com/cockroachdb/cockroach/pkg/kv/kvpb" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness" + "github.com/cockroachdb/cockroach/pkg/multitenant" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/rpc" "github.com/cockroachdb/cockroach/pkg/server/status/statuspb" @@ -49,6 +51,7 @@ import ( "github.com/cockroachdb/redact" "github.com/dustin/go-humanize" "github.com/elastic/gosigar" + prometheusgo "github.com/prometheus/client_model/go" ) const ( @@ -385,6 +388,7 @@ func (mr *MetricsRecorder) GetTimeSeriesData() []tspb.TimeSeriesData { } // Record time series from store-level registries. + tIDLabel := multitenant.TenantIDLabel for storeID, r := range mr.mu.storeRegistries { storeRecorder := registryRecorder{ registry: r, @@ -393,6 +397,21 @@ func (mr *MetricsRecorder) GetTimeSeriesData() []tspb.TimeSeriesData { timestampNanos: now.UnixNano(), } storeRecorder.record(&data) + + // Now record secondary tenant store metrics, if any exist in the process. + for tenantID := range mr.mu.tenantRegistries { + tenantStoreRecorder := registryRecorder{ + registry: r, + format: storeTimeSeriesPrefix, + source: tsutil.MakeTenantSource(storeID.String(), tenantID.String()), + timestampNanos: now.UnixNano(), + } + tenantID := tenantID.String() + tenantStoreRecorder.recordChild(&data, kvbase.TenantsStorageMetricsSet, &prometheusgo.LabelPair{ + Name: &tIDLabel, + Value: &tenantID, + }) + } } atomic.CompareAndSwapInt64(&mr.lastDataCount, lastDataCount, int64(len(data))) return data @@ -659,6 +678,66 @@ func (rr registryRecorder) record(dest *[]tspb.TimeSeriesData) { }) } +// recordChild filters the metrics in the registry down to those provided in +// the metricsFilter argument, and iterates through any child metrics that +// may exist on said metric. Child metrics whose label sets contains a match +// against the childLabelFilter are recorded into the provided dest slice of +// type tspb.TimeSeriesData. +// +// NB: Only available for Counter and Gauge metrics. +func (rr registryRecorder) recordChild( + dest *[]tspb.TimeSeriesData, + metricsFilter map[string]struct{}, + childLabelFilter *prometheusgo.LabelPair, +) { + labels := rr.registry.GetLabels() + rr.registry.Select(metricsFilter, func(name string, v interface{}) { + prom, ok := v.(metric.PrometheusExportable) + if !ok { + return + } + promIter, ok := v.(metric.PrometheusIterable) + if !ok { + return + } + m := prom.ToPrometheusMetric() + m.Label = append(labels, prom.GetLabels()...) + + processChildMetric := func(metric *prometheusgo.Metric) { + found := false + for _, label := range metric.Label { + if label.GetName() == childLabelFilter.GetName() && + label.GetValue() == childLabelFilter.GetValue() { + found = true + break + } + } + if !found { + return + } + var value float64 + if metric.Gauge != nil { + value = *metric.Gauge.Value + } else if metric.Counter != nil { + value = *metric.Counter.Value + } else { + return + } + *dest = append(*dest, tspb.TimeSeriesData{ + Name: fmt.Sprintf(rr.format, prom.GetName()), + Source: rr.source, + Datapoints: []tspb.TimeSeriesDatapoint{ + { + TimestampNanos: rr.timestampNanos, + Value: value, + }, + }, + }) + } + promIter.Each(m.Label, processChildMetric) + }) +} + // GetTotalMemory returns either the total system memory (in bytes) or if // possible the cgroups available memory. func GetTotalMemory(ctx context.Context) (int64, error) { diff --git a/pkg/server/status/recorder_test.go b/pkg/server/status/recorder_test.go index d5f36fe353df..e6414e71b15f 100644 --- a/pkg/server/status/recorder_test.go +++ b/pkg/server/status/recorder_test.go @@ -23,17 +23,20 @@ import ( "time" "github.com/cockroachdb/cockroach/pkg/build" + "github.com/cockroachdb/cockroach/pkg/multitenant" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/server/status/statuspb" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants" "github.com/cockroachdb/cockroach/pkg/ts/tspb" + "github.com/cockroachdb/cockroach/pkg/ts/tsutil" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/metric" "github.com/cockroachdb/cockroach/pkg/util/metric/aggmetric" "github.com/cockroachdb/cockroach/pkg/util/system" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/kr/pretty" + prometheusgo "github.com/prometheus/client_model/go" "github.com/stretchr/testify/require" ) @@ -241,6 +244,148 @@ func TestMetricsRecorderLabels(t *testing.T) { } } +func TestRegistryRecorder_RecordChild(t *testing.T) { + defer leaktest.AfterTest(t)() + store1 := fakeStore{ + storeID: roachpb.StoreID(1), + desc: roachpb.StoreDescriptor{ + StoreID: roachpb.StoreID(1), + Capacity: roachpb.StoreCapacity{ + Capacity: 100, + Available: 50, + Used: 50, + }, + }, + registry: metric.NewRegistry(), + } + store2 := fakeStore{ + storeID: roachpb.StoreID(2), + desc: roachpb.StoreDescriptor{ + StoreID: roachpb.StoreID(2), + Capacity: roachpb.StoreCapacity{ + Capacity: 200, + Available: 75, + Used: 125, + }, + }, + registry: metric.NewRegistry(), + } + systemTenantNameContainer := roachpb.NewTenantNameContainer(catconstants.SystemTenantName) + manual := timeutil.NewManualTime(timeutil.Unix(0, 100)) + st := cluster.MakeTestingClusterSettings() + recorder := NewMetricsRecorder(roachpb.SystemTenantID, systemTenantNameContainer, nil, nil, manual, st) + recorder.AddStore(store1) + recorder.AddStore(store2) + + tenantIDs := []string{"2", "3"} + type childMetric struct { + tenantID string + value int64 + } + type testMetric struct { + name string + typ string + children []childMetric + } + // Each registry will have a copy of the following metrics. + metrics := []testMetric{ + { + name: "testAggGauge", + typ: "agggauge", + children: []childMetric{ + { + tenantID: "2", + value: 2, + }, + { + tenantID: "3", + value: 5, + }, + }, + }, + { + name: "testAggCounter", + typ: "aggcounter", + children: []childMetric{ + { + tenantID: "2", + value: 10, + }, + { + tenantID: "3", + value: 17, + }, + }, + }, + } + + var expected []tspb.TimeSeriesData + // addExpected generates expected TimeSeriesData for all child metrics. + addExpected := func(storeID string, metric *testMetric) { + for _, child := range metric.children { + expect := tspb.TimeSeriesData{ + Name: "cr.store." + metric.name, + Source: tsutil.MakeTenantSource(storeID, child.tenantID), + Datapoints: []tspb.TimeSeriesDatapoint{ + { + TimestampNanos: 100, + Value: float64(child.value), + }, + }, + } + expected = append(expected, expect) + } + } + + tIDLabel := multitenant.TenantIDLabel + for _, store := range []fakeStore{store1, store2} { + for _, m := range metrics { + switch m.typ { + case "aggcounter": + ac := aggmetric.NewCounter(metric.Metadata{Name: m.name}, tIDLabel) + store.registry.AddMetric(ac) + for _, cm := range m.children { + c := ac.AddChild(cm.tenantID) + c.Inc(cm.value) + } + addExpected(store.storeID.String(), &m) + case "agggauge": + ag := aggmetric.NewGauge(metric.Metadata{Name: m.name}, tIDLabel) + store.registry.AddMetric(ag) + for _, cm := range m.children { + c := ag.AddChild(cm.tenantID) + c.Inc(cm.value) + } + addExpected(store.storeID.String(), &m) + } + } + } + metricFilter := map[string]struct{}{ + "testAggGauge": {}, + "testAggCounter": {}, + } + actual := make([]tspb.TimeSeriesData, 0) + for _, store := range []fakeStore{store1, store2} { + for _, tID := range tenantIDs { + tenantStoreRecorder := registryRecorder{ + registry: store.registry, + format: storeTimeSeriesPrefix, + source: tsutil.MakeTenantSource(store.storeID.String(), tID), + timestampNanos: 100, + } + tenantStoreRecorder.recordChild(&actual, metricFilter, &prometheusgo.LabelPair{ + Name: &tIDLabel, + Value: &tID, + }) + } + } + sort.Sort(byTimeAndName(actual)) + sort.Sort(byTimeAndName(expected)) + if !reflect.DeepEqual(actual, expected) { + t.Errorf("registryRecorder did not yield expected time series collection for child metrics; diff:\n %v", pretty.Diff(actual, expected)) + } +} + // TestMetricsRecorder verifies that the metrics recorder properly formats the // statistics from various registries, both for Time Series and for Status // Summaries. diff --git a/pkg/util/metric/prometheus_exporter.go b/pkg/util/metric/prometheus_exporter.go index 3d6e3341f418..5468dda217f2 100644 --- a/pkg/util/metric/prometheus_exporter.go +++ b/pkg/util/metric/prometheus_exporter.go @@ -78,7 +78,7 @@ func (pm *PrometheusExporter) findOrCreateFamily( // connected to the registry and metrics within) when returning from the the // call. It creates new families as needed. func (pm *PrometheusExporter) ScrapeRegistry(registry *Registry, includeChildMetrics bool) { - labels := registry.getLabels() + labels := registry.GetLabels() f := func(name string, v interface{}) { prom, ok := v.(PrometheusExportable) if !ok { diff --git a/pkg/util/metric/registry.go b/pkg/util/metric/registry.go index e8f5e903851d..8a9e071328c9 100644 --- a/pkg/util/metric/registry.go +++ b/pkg/util/metric/registry.go @@ -33,7 +33,7 @@ type Registry struct { labels []labelPair tracked map[string]Iterable - // computedLabels get filled in by getLabels(). + // computedLabels get filled in by GetLabels(). // We hold onto the slice to avoid a re-allocation every // time the metrics get scraped. computedLabels []*prometheusgo.LabelPair @@ -67,7 +67,7 @@ func (r *Registry) AddLabel(name string, value interface{}) { r.computedLabels = append(r.computedLabels, &prometheusgo.LabelPair{}) } -func (r *Registry) getLabels() []*prometheusgo.LabelPair { +func (r *Registry) GetLabels() []*prometheusgo.LabelPair { r.Lock() defer r.Unlock() for i, l := range r.labels {