Skip to content

Commit

Permalink
Merge pull request #122705 from cockroachdb/blathers/backport-release…
Browse files Browse the repository at this point in the history
…-24.1-122151

release-24.1: ui: make custom chart tool work at store level
  • Loading branch information
abarganier authored Apr 26, 2024
2 parents aea98c1 + 8746ed5 commit ed31fab
Show file tree
Hide file tree
Showing 3 changed files with 243 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ export class CustomMetricState {
downsampler = TimeSeriesQueryAggregator.AVG;
aggregator = TimeSeriesQueryAggregator.SUM;
derivative = TimeSeriesQueryDerivative.NONE;
perNode = false;
perSource = false;
perTenant = false;
source = "";
nodeSource = "";
tenantSource = "";
}

Expand Down Expand Up @@ -112,9 +112,9 @@ export class CustomMetricRow extends React.Component<CustomMetricRowProps> {
});
};

changeSource = (selectedOption: DropdownOption) => {
changeNodeSource = (selectedOption: DropdownOption) => {
this.changeState({
source: selectedOption.value,
nodeSource: selectedOption.value,
});
};

Expand All @@ -124,9 +124,9 @@ export class CustomMetricRow extends React.Component<CustomMetricRowProps> {
});
};

changePerNode = (selection: React.FormEvent<HTMLInputElement>) => {
changePerSource = (selection: React.FormEvent<HTMLInputElement>) => {
this.changeState({
perNode: selection.currentTarget.checked,
perSource: selection.currentTarget.checked,
});
};

Expand All @@ -151,8 +151,8 @@ export class CustomMetricRow extends React.Component<CustomMetricRowProps> {
downsampler,
aggregator,
derivative,
source,
perNode,
nodeSource,
perSource,
tenantSource,
perTenant,
},
Expand Down Expand Up @@ -217,17 +217,17 @@ export class CustomMetricRow extends React.Component<CustomMetricRowProps> {
className="metric-table-dropdown__select"
clearable={false}
searchable={false}
value={source}
value={nodeSource}
options={nodeOptions}
onChange={this.changeSource}
onChange={this.changeNodeSource}
/>
</div>
</td>
<td className="metric-table__cell">
<input
type="checkbox"
checked={perNode}
onChange={this.changePerNode}
checked={perSource}
onChange={this.changePerSource}
/>
</td>
{canViewTenantOptions && (
Expand Down Expand Up @@ -340,7 +340,7 @@ export class CustomChartTable extends React.Component<CustomChartTableProps> {
<td className="metric-table__header">Aggregator</td>
<td className="metric-table__header">Rate</td>
<td className="metric-table__header">Source</td>
<td className="metric-table__header">Per Node</td>
<td className="metric-table__header">Per Node/Store</td>
{canViewTenantOptions && (
<td className="metric-table__header">Virtual Cluster</td>
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
// Copyright 2024 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.

import { NodesSummary } from "src/redux/nodes";
import { INodeStatus } from "src/util/proto";
import { GetSources } from "src/views/reports/containers/customChart/index";
import * as protos from "src/js/protos";
import { CustomMetricState } from "src/views/reports/containers/customChart/customMetric";

import TimeSeriesQueryAggregator = protos.cockroach.ts.tspb.TimeSeriesQueryAggregator;
import TimeSeriesQueryDerivative = protos.cockroach.ts.tspb.TimeSeriesQueryDerivative;

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([]);
});

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([]);
});

it("returns empty when no specific node source is requested, nor per-source metrics", function () {
const nodesSummary = new testNodesSummaryBuilder().build();
const metricState = new testCustomMetricStateBuilder()
.setNodeSource("")
.setIsPerSource(false)
.build();
expect(GetSources(nodesSummary, metricState)).toStrictEqual([]);
});

describe("The metric is at the store-level", function () {
const storeMetricName = "cr.store.metric";

it("returns the store IDs associated with a specific node when a node source is set", function () {
const expectedSources = ["1", "2", "3"];
const metricState = new testCustomMetricStateBuilder()
.setName(storeMetricName)
.setNodeSource("1")
.build();
const nodesSummary = new testNodesSummaryBuilder()
.setStoreIDsByNodeID({
"1": expectedSources,
})
.build();
expect(GetSources(nodesSummary, metricState)).toStrictEqual(
expectedSources,
);
});

it("returns all known store IDs for the cluster when no node source is set", function () {
const expectedSources = ["1", "2", "3", "4", "5", "6", "7", "8", "9"];
const metricState = new testCustomMetricStateBuilder()
.setName(storeMetricName)
.build();
const nodesSummary = new testNodesSummaryBuilder()
.setStoreIDsByNodeID({
"1": ["1", "2", "3"],
"2": ["4", "5", "6"],
"3": ["7", "8", "9"],
})
.build();
const actualSources = GetSources(nodesSummary, metricState).sort();
expect(actualSources).toStrictEqual(expectedSources);
});
});

describe("The metric is at the node-level", function () {
const nodeMetricName = "cr.node.metric";

it("returns the specified node source when a node source is set", function () {
const expectedSources = ["1"];
const metricState = new testCustomMetricStateBuilder()
.setName(nodeMetricName)
.setNodeSource("1")
.build();
const nodesSummary = new testNodesSummaryBuilder().build();
expect(GetSources(nodesSummary, metricState)).toStrictEqual(
expectedSources,
);
});

it("returns all known node IDs when no node source is set", function () {
const expectedSources = ["1", "2", "3"];
const metricState = new testCustomMetricStateBuilder()
.setName(nodeMetricName)
.build();
const nodesSummary = new testNodesSummaryBuilder()
.setNodeIDs(["1", "2", "3"])
.build();
expect(GetSources(nodesSummary, metricState)).toStrictEqual(
expectedSources,
);
});
});
});
});

class testCustomMetricStateBuilder {
name: string;
nodeSource: string;
perSource: boolean;

setName(name: string): testCustomMetricStateBuilder {
this.name = name;
return this;
}

setNodeSource(nodeSource: string): testCustomMetricStateBuilder {
this.nodeSource = nodeSource;
return this;
}

setIsPerSource(perSource: boolean): testCustomMetricStateBuilder {
this.perSource = perSource;
return this;
}

build(): CustomMetricState {
return {
metric: this.name,
downsampler: TimeSeriesQueryAggregator.AVG,
aggregator: TimeSeriesQueryAggregator.SUM,
derivative: TimeSeriesQueryDerivative.NONE,
perSource: this.perSource,
perTenant: false,
nodeSource: this.nodeSource,
tenantSource: "",
};
}
}

class testNodesSummaryBuilder {
nodeStatuses: INodeStatus[];
storeIDsByNodeID: { [key: string]: string[] };
nodeIDs: string[];

setStoreIDsByNodeID(storeIDsByNodeID: {
[key: string]: string[];
}): testNodesSummaryBuilder {
this.storeIDsByNodeID = storeIDsByNodeID;
return this;
}

setNodeIDs(nodeIDs: string[]): testNodesSummaryBuilder {
this.nodeIDs = nodeIDs;
return this;
}

build(): NodesSummary {
return {
// We normally don't care about the nodeStatuses elements, so long as it's not an empty list.
// Populate with an empty object.
nodeStatuses: [
{
// We also need a non-empty list of store_statuses, for the isStoreMetric() call made.
store_statuses: [{}],
},
],
nodeIDs: this.nodeIDs,
nodeStatusByID: { "": {} },
nodeDisplayNameByID: { "": "" },
livenessStatusByNodeID: {},
livenessByNodeID: {},
storeIDsByNodeID: this.storeIDsByNodeID,
nodeLastError: undefined,
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,34 @@ interface UrlState {
charts: string;
}

export const GetSources = (
nodesSummary: NodesSummary,
metricState: CustomMetricState,
): string[] => {
if (!(nodesSummary?.nodeStatuses?.length > 0)) {
return [];
}
// If we have no nodeSource, and we're not asking for perSource metrics,
// then the user is asking for cluster-wide metrics. We can return an empty
// source list.
if (metricState.nodeSource === "" && !metricState.perSource) {
return [];
}
if (isStoreMetric(nodesSummary.nodeStatuses[0], 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
? nodesSummary.storeIDsByNodeID[metricState.nodeSource]
: Object.values(nodesSummary.storeIDsByNodeID).flatMap(s => s);
} else {
// If it's not a store metric, and a specific nodeSource is chosen, just return that.
// Otherwise, return all known node IDs.
return metricState.nodeSource
? [metricState.nodeSource]
: nodesSummary.nodeIDs;
}
};

export class CustomChart extends React.Component<
CustomChartProps & RouteComponentProps
> {
Expand Down Expand Up @@ -212,52 +240,47 @@ export class CustomChart extends React.Component<
// based on perNode and perTenant flags.
renderMetricComponents = (metrics: CustomMetricState[], index: number) => {
const { nodesSummary, tenantOptions } = this.props;
// We require nodes information to determine sources (storeIDs/nodeIDs) down below.
if (!(nodesSummary?.nodeStatuses?.length > 0)) {
return;
}
const tenants = tenantOptions.length > 1 ? tenantOptions.slice(1) : [];
return metrics.map((m, i) => {
if (m.metric === "") {
return "";
}
if (m.perNode && m.perTenant) {
return _.flatMap(nodesSummary.nodeIDs, nodeID => {
if (m.perSource && m.perTenant) {
const sources = GetSources(nodesSummary, m);
return _.flatMap(sources, source => {
return tenants.map(tenant => (
<Metric
key={`${index}${i}${nodeID}${tenant.value}`}
title={`${nodeID}-${tenant.value}: ${m.metric} (${i})`}
key={`${index}${i}${source}${tenant.value}`}
title={`${source}-${tenant.value}: ${m.metric} (${i})`}
name={m.metric}
aggregator={m.aggregator}
downsampler={m.downsampler}
derivative={m.derivative}
sources={
isStoreMetric(nodesSummary.nodeStatuses[0], m.metric)
? _.map(nodesSummary.storeIDsByNodeID[nodeID] || [], n =>
n.toString(),
)
: [nodeID]
}
sources={[source]}
tenantSource={tenant.value}
/>
));
});
} else if (m.perNode) {
return _.map(nodesSummary.nodeIDs, nodeID => (
} else if (m.perSource) {
const sources = GetSources(nodesSummary, m);
return _.map(sources, source => (
<Metric
key={`${index}${i}${nodeID}`}
title={`${nodeID}: ${m.metric} (${i})`}
key={`${index}${i}${source}`}
title={`${source}: ${m.metric} (${i})`}
name={m.metric}
aggregator={m.aggregator}
downsampler={m.downsampler}
derivative={m.derivative}
sources={
isStoreMetric(nodesSummary.nodeStatuses[0], m.metric)
? _.map(nodesSummary.storeIDsByNodeID[nodeID] || [], n =>
n.toString(),
)
: [nodeID]
}
sources={[source]}
tenantSource={m.tenantSource}
/>
));
} else if (m.perTenant) {
const sources = GetSources(nodesSummary, m);
return tenants.map(tenant => (
<Metric
key={`${index}${i}${tenant.value}`}
Expand All @@ -266,7 +289,7 @@ export class CustomChart extends React.Component<
aggregator={m.aggregator}
downsampler={m.downsampler}
derivative={m.derivative}
sources={m.source === "" ? [] : [m.source]}
sources={sources}
tenantSource={tenant.value}
/>
));
Expand All @@ -279,7 +302,7 @@ export class CustomChart extends React.Component<
aggregator={m.aggregator}
downsampler={m.downsampler}
derivative={m.derivative}
sources={m.source === "" ? [] : [m.source]}
sources={GetSources(nodesSummary, m)}
tenantSource={m.tenantSource}
/>
);
Expand Down Expand Up @@ -402,5 +425,8 @@ export default withRouter(
);

function isStoreMetric(nodeStatus: INodeStatus, metricName: string) {
if (metricName?.startsWith("cr.store")) {
return true;
}
return _.has(nodeStatus.store_statuses[0].metrics, metricName);
}

0 comments on commit ed31fab

Please sign in to comment.