diff --git a/docs/generated/http/full.md b/docs/generated/http/full.md index 31d96353028d..e99dd0c9628f 100644 --- a/docs/generated/http/full.md +++ b/docs/generated/http/full.md @@ -7271,6 +7271,7 @@ MetricMetadataResponse contains the metadata for all metrics. | Field | Type | Label | Description | Support status | | ----- | ---- | ----- | ----------- | -------------- | | metadata | [MetricMetadataResponse.MetadataEntry](#cockroach.server.serverpb.MetricMetadataResponse-cockroach.server.serverpb.MetricMetadataResponse.MetadataEntry) | repeated | | [reserved](#support-status) | +| recordedNames | [MetricMetadataResponse.RecordedNamesEntry](#cockroach.server.serverpb.MetricMetadataResponse-cockroach.server.serverpb.MetricMetadataResponse.RecordedNamesEntry) | repeated | Maps of metric metadata names to the tsdb recorded metric names | [reserved](#support-status) | @@ -7291,6 +7292,20 @@ MetricMetadataResponse contains the metadata for all metrics. + +#### MetricMetadataResponse.RecordedNamesEntry + + + +| Field | Type | Label | Description | Support status | +| ----- | ---- | ----- | ----------- | -------------- | +| key | [string](#cockroach.server.serverpb.MetricMetadataResponse-string) | | | | +| value | [string](#cockroach.server.serverpb.MetricMetadataResponse-string) | | | | + + + + + ## ChartCatalog diff --git a/pkg/server/admin.go b/pkg/server/admin.go index bba4c33a9724..9b408e9e55f6 100644 --- a/pkg/server/admin.go +++ b/pkg/server/admin.go @@ -301,8 +301,10 @@ func (s *adminServer) AllMetricMetadata( ) (*serverpb.MetricMetadataResponse, error) { md, _, _ := s.metricsRecorder.GetMetricsMetadata(true /* combine */) + metricNames := s.metricsRecorder.GetRecordedMetricNames(md) resp := &serverpb.MetricMetadataResponse{ - Metadata: md, + Metadata: md, + RecordedNames: metricNames, } return resp, nil diff --git a/pkg/server/application_api/metrics_test.go b/pkg/server/application_api/metrics_test.go index 85566131645b..798612075563 100644 --- a/pkg/server/application_api/metrics_test.go +++ b/pkg/server/application_api/metrics_test.go @@ -56,6 +56,20 @@ func TestMetricsMetadata(t *testing.T) { } } +func TestGetRecordedMetricNames(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + s := serverutils.StartServerOnly(t, base.TestServerArgs{}) + defer s.Stopper().Stop(context.Background()) + metricsMetadata, _, _ := s.MetricsRecorder().GetMetricsMetadata(true /* combine */) + recordedNames := s.MetricsRecorder().GetRecordedMetricNames(metricsMetadata) + + require.Equal(t, len(metricsMetadata), len(recordedNames)) + for _, v := range recordedNames { + require.True(t, strings.HasPrefix(v, "cr.node") || strings.HasPrefix(v, "cr.store")) + } +} + // TestStatusVars verifies that prometheus metrics are available via the // /_status/vars and /_status/load endpoints. func TestStatusVars(t *testing.T) { diff --git a/pkg/server/serverpb/admin.proto b/pkg/server/serverpb/admin.proto index 36eb545c3c15..2a320cabb54d 100644 --- a/pkg/server/serverpb/admin.proto +++ b/pkg/server/serverpb/admin.proto @@ -846,6 +846,8 @@ message MetricMetadataRequest { // MetricMetadataResponse contains the metadata for all metrics. message MetricMetadataResponse { map metadata = 1 [(gogoproto.nullable) = false]; + // Maps of metric metadata names to the tsdb recorded metric names + map recordedNames = 2 [(gogoproto.nullable) = false]; } message EnqueueRangeRequest { diff --git a/pkg/server/status/recorder.go b/pkg/server/status/recorder.go index 14b1b82b898b..2baa5c815142 100644 --- a/pkg/server/status/recorder.go +++ b/pkg/server/status/recorder.go @@ -486,22 +486,42 @@ func (mr *MetricsRecorder) GetMetricsMetadata( mr.mu.logRegistry.WriteMetricsMetadata(srvMetrics) mr.mu.sysRegistry.WriteMetricsMetadata(srvMetrics) - // Get a random storeID. - var sID roachpb.StoreID + mr.writeStoreMetricsMetadata(nodeMetrics) + return nodeMetrics, appMetrics, srvMetrics +} + +// GetRecordedMetricNames takes a map of metric metadata and returns a map +// of the metadata name to the name the metric is recorded with in tsdb. +func (mr *MetricsRecorder) GetRecordedMetricNames( + allMetadata map[string]metric.Metadata, +) map[string]string { + storeMetricsMap := make(map[string]metric.Metadata) + tsDbMetricNames := make(map[string]string, len(allMetadata)) + mr.writeStoreMetricsMetadata(storeMetricsMap) + for k := range allMetadata { + prefix := nodeTimeSeriesPrefix + if _, ok := storeMetricsMap[k]; ok { + prefix = storeTimeSeriesPrefix + } - storeFound := false - for storeID := range mr.mu.storeRegistries { - sID = storeID - storeFound = true - break + tsDbMetricNames[k] = fmt.Sprintf(prefix, k) } + return tsDbMetricNames +} - // Get metric metadata from that store because all stores have the same metadata. - if storeFound { - mr.mu.storeRegistries[sID].WriteMetricsMetadata(nodeMetrics) +// writeStoreMetricsMetadata Gets a store from mr.mu.storeRegistries and writes +// the metrics metadata to the provided map. +func (mr *MetricsRecorder) writeStoreMetricsMetadata(metricsMetadata map[string]metric.Metadata) { + if len(mr.mu.storeRegistries) == 0 { + return } - return nodeMetrics, appMetrics, srvMetrics + // All store registries should have the same metadata, so only the metadata + // from the first store is used to write to metricsMetadata. + for _, registry := range mr.mu.storeRegistries { + registry.WriteMetricsMetadata(metricsMetadata) + return + } } // getNetworkActivity produces a map of network activity from this node to all diff --git a/pkg/ui/workspaces/db-console/src/redux/metricMetadata.ts b/pkg/ui/workspaces/db-console/src/redux/metricMetadata.ts index bc8cf6b098b4..def900cf14a5 100644 --- a/pkg/ui/workspaces/db-console/src/redux/metricMetadata.ts +++ b/pkg/ui/workspaces/db-console/src/redux/metricMetadata.ts @@ -8,7 +8,7 @@ import { createSelector } from "reselect"; import { AdminUIState } from "src/redux/state"; import { MetricMetadataResponseMessage } from "src/util/api"; -export type MetricsMetadata = MetricMetadataResponseMessage["metadata"]; +export type MetricsMetadata = MetricMetadataResponseMessage; // State selectors const metricsMetadataStateSelector = (state: AdminUIState) => @@ -16,6 +16,5 @@ const metricsMetadataStateSelector = (state: AdminUIState) => export const metricsMetadataSelector = createSelector( metricsMetadataStateSelector, - (metricsMetadata): MetricsMetadata => - metricsMetadata ? metricsMetadata.metadata : undefined, + (metricsMetadata): MetricsMetadata => metricsMetadata, ); diff --git a/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.spec.ts b/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.spec.ts index 07baeaab2c8e..5702079a6d9e 100644 --- a/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.spec.ts +++ b/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.spec.ts @@ -4,26 +4,35 @@ // included in the /LICENSE file. import * as protos from "src/js/protos"; +import { MetricsMetadata } from "src/redux/metricMetadata"; import { NodesSummary } from "src/redux/nodes"; import { INodeStatus } from "src/util/proto"; import { CustomMetricState } from "src/views/reports/containers/customChart/customMetric"; -import { GetSources } from "src/views/reports/containers/customChart/index"; +import { getSources } from "src/views/reports/containers/customChart/index"; import TimeSeriesQueryAggregator = protos.cockroach.ts.tspb.TimeSeriesQueryAggregator; import TimeSeriesQueryDerivative = protos.cockroach.ts.tspb.TimeSeriesQueryDerivative; +const emptyMetricsMetadata: MetricsMetadata = { + metadata: {}, + recordedNames: {}, +}; describe("Custom charts page", function () { describe("Getting metric sources", function () { it("returns empty when nodesSummary is undefined", function () { const metricState = new testCustomMetricStateBuilder().build(); - expect(GetSources(undefined, metricState)).toStrictEqual([]); + expect( + getSources(undefined, metricState, emptyMetricsMetadata), + ).toStrictEqual([]); }); it("returns empty when the nodeStatuses collection is empty", function () { const nodesSummary = new testNodesSummaryBuilder().build(); nodesSummary.nodeStatuses = []; const metricState = new testCustomMetricStateBuilder().build(); - expect(GetSources(nodesSummary, metricState)).toStrictEqual([]); + expect( + getSources(nodesSummary, metricState, emptyMetricsMetadata), + ).toStrictEqual([]); }); it("returns empty when no specific node source is requested, nor per-source metrics", function () { @@ -32,7 +41,9 @@ describe("Custom charts page", function () { .setNodeSource("") .setIsPerSource(false) .build(); - expect(GetSources(nodesSummary, metricState)).toStrictEqual([]); + expect( + getSources(nodesSummary, metricState, emptyMetricsMetadata), + ).toStrictEqual([]); }); describe("The metric is at the store-level", function () { @@ -49,9 +60,9 @@ describe("Custom charts page", function () { "1": expectedSources, }) .build(); - expect(GetSources(nodesSummary, metricState)).toStrictEqual( - expectedSources, - ); + expect( + getSources(nodesSummary, metricState, emptyMetricsMetadata), + ).toStrictEqual(expectedSources); }); it("returns all known store IDs for the cluster when no node source is set", function () { @@ -66,7 +77,11 @@ describe("Custom charts page", function () { "3": ["7", "8", "9"], }) .build(); - const actualSources = GetSources(nodesSummary, metricState).sort(); + const actualSources = getSources( + nodesSummary, + metricState, + emptyMetricsMetadata, + ).sort(); expect(actualSources).toStrictEqual(expectedSources); }); }); @@ -81,9 +96,9 @@ describe("Custom charts page", function () { .setNodeSource("1") .build(); const nodesSummary = new testNodesSummaryBuilder().build(); - expect(GetSources(nodesSummary, metricState)).toStrictEqual( - expectedSources, - ); + expect( + getSources(nodesSummary, metricState, emptyMetricsMetadata), + ).toStrictEqual(expectedSources); }); it("returns all known node IDs when no node source is set", function () { @@ -94,9 +109,9 @@ describe("Custom charts page", function () { const nodesSummary = new testNodesSummaryBuilder() .setNodeIDs(["1", "2", "3"]) .build(); - expect(GetSources(nodesSummary, metricState)).toStrictEqual( - expectedSources, - ); + expect( + getSources(nodesSummary, metricState, emptyMetricsMetadata), + ).toStrictEqual(expectedSources); }); }); }); diff --git a/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.tsx b/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.tsx index c9c1f4045bb5..c78b02a38ae1 100644 --- a/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.tsx +++ b/pkg/ui/workspaces/db-console/src/views/reports/containers/customChart/index.tsx @@ -6,7 +6,6 @@ import { AxisUnits, TimeScale } from "@cockroachlabs/cluster-ui"; import flatMap from "lodash/flatMap"; import flow from "lodash/flow"; -import has from "lodash/has"; import isEmpty from "lodash/isEmpty"; import keys from "lodash/keys"; import map from "lodash/map"; @@ -76,9 +75,10 @@ interface UrlState { charts: string; } -export const GetSources = ( +export const getSources = ( nodesSummary: NodesSummary, metricState: CustomMetricState, + metricsMetadata: MetricsMetadata, ): string[] => { if (!(nodesSummary?.nodeStatuses?.length > 0)) { return []; @@ -89,7 +89,7 @@ export const GetSources = ( if (metricState.nodeSource === "" && !metricState.perSource) { return []; } - if (isStoreMetric(nodesSummary.nodeStatuses[0], metricState.metric)) { + if (isStoreMetric(metricsMetadata.recordedNames, metricState.metric)) { // If a specific node is selected, return the storeIDs associated with that node. // Otherwise, we're at the cluster level, so we grab each store ID. return metricState.nodeSource @@ -133,23 +133,18 @@ export class CustomChart extends React.Component< // Selector which computes dropdown options based on the metrics which are // currently being stored on the cluster. private metricOptions = createSelector( - (summary: NodesSummary) => summary.nodeStatuses, - (_summary: NodesSummary, metricsMetadata: MetricsMetadata) => - metricsMetadata, - (nodeStatuses, metadata = {}): DropdownOption[] => { - if (isEmpty(nodeStatuses)) { + (metricsMetadata: MetricsMetadata) => metricsMetadata, + (metricsMetadata): DropdownOption[] => { + if (isEmpty(metricsMetadata?.metadata)) { return []; } - return keys(nodeStatuses[0].metrics).map(k => { - const fullMetricName = isStoreMetric(nodeStatuses[0], k) - ? "cr.store." + k - : "cr.node." + k; - + return keys(metricsMetadata.metadata).map(k => { + const fullMetricName = metricsMetadata.recordedNames[k]; return { value: fullMetricName, label: k, - description: metadata[k] && metadata[k].help, + description: metricsMetadata.metadata[k]?.help, }; }); }, @@ -243,7 +238,7 @@ export class CustomChart extends React.Component< // This function handles the logic related to creating Metric components // based on perNode and perTenant flags. renderMetricComponents = (metrics: CustomMetricState[], index: number) => { - const { nodesSummary, tenantOptions } = this.props; + const { nodesSummary, tenantOptions, metricsMetadata } = this.props; // We require nodes information to determine sources (storeIDs/nodeIDs) down below. if (!(nodesSummary?.nodeStatuses?.length > 0)) { return; @@ -253,8 +248,8 @@ export class CustomChart extends React.Component< if (m.metric === "") { return ""; } + const sources = getSources(nodesSummary, m, metricsMetadata); if (m.perSource && m.perTenant) { - const sources = GetSources(nodesSummary, m); return flatMap(sources, source => { return tenants.map(tenant => ( ( )); } else if (m.perTenant) { - const sources = GetSources(nodesSummary, m); return tenants.map(tenant => ( ); @@ -356,7 +349,7 @@ export class CustomChart extends React.Component< <> {charts.map((chart, i) => ( , + metricName: string, +) { if (metricName?.startsWith("cr.store")) { return true; } - return has(nodeStatus.store_statuses[0].metrics, metricName); + return recordedNames[metricName]?.startsWith("cr.store") || false; }