Skip to content

Commit

Permalink
ui: tests for custom chart logic to determine metric sources
Browse files Browse the repository at this point in the history
#122151 fixed the custom
chart tool to properly display store-level metrics. However, it did not
include any tests. Despite metric charts being difficult to unit test,
we can still test the logic used to determine which metric sources
should be included in the timeseries query.

This commit adds tests for this logic.

Release note: none
  • Loading branch information
abarganier committed Apr 26, 2024
1 parent 1a444b2 commit 8746ed5
Show file tree
Hide file tree
Showing 2 changed files with 212 additions and 32 deletions.
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 @@ -208,34 +236,6 @@ export class CustomChart extends React.Component<
);
};

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;
}
};

// This function handles the logic related to creating Metric components
// based on perNode and perTenant flags.
renderMetricComponents = (metrics: CustomMetricState[], index: number) => {
Expand All @@ -250,7 +250,7 @@ export class CustomChart extends React.Component<
return "";
}
if (m.perSource && m.perTenant) {
const sources = this.getSources(nodesSummary, m);
const sources = GetSources(nodesSummary, m);
return _.flatMap(sources, source => {
return tenants.map(tenant => (
<Metric
Expand All @@ -266,7 +266,7 @@ export class CustomChart extends React.Component<
));
});
} else if (m.perSource) {
const sources = this.getSources(nodesSummary, m);
const sources = GetSources(nodesSummary, m);
return _.map(sources, source => (
<Metric
key={`${index}${i}${source}`}
Expand All @@ -280,7 +280,7 @@ export class CustomChart extends React.Component<
/>
));
} else if (m.perTenant) {
const sources = this.getSources(nodesSummary, m);
const sources = GetSources(nodesSummary, m);
return tenants.map(tenant => (
<Metric
key={`${index}${i}${tenant.value}`}
Expand All @@ -302,7 +302,7 @@ export class CustomChart extends React.Component<
aggregator={m.aggregator}
downsampler={m.downsampler}
derivative={m.derivative}
sources={this.getSources(nodesSummary, m)}
sources={GetSources(nodesSummary, m)}
tenantSource={m.tenantSource}
/>
);
Expand Down

0 comments on commit 8746ed5

Please sign in to comment.