Skip to content

Commit

Permalink
ui: make custom chart tool work at store level
Browse files Browse the repository at this point in the history
Fixes: #121364

This patch fixes a bug in the DB Console custom chart tool, where
selecting the "Per Node" checkbox on a metric would not properly display
store-level metrics. The previous expected behavior was that the check
box would cause the metric to aggregate across stores at the node level
(e.g. if the node had 3 stores, it'd SUM the store-level timeseries
together and return a single timeseries for the node). Instead, the
feature was only showing the 1st store associated with the node.

This was due to a bug in the code used to determine if a metric was
store-level. A function was used that improperly assumed that the
`cr.node.*` or `cr.store.*` prefix had been stripped from the metric
name, which was not always the case. This led to us improperly
interpret store-level metrics as node-level.

The fix is to fix the logic used to determine if a metric is
store-level.

Additionally, this patch updates the code to no longer aggregate
store-level metrics across each node. Instead, we will now show a single
timeseries per-store to provide finer-grained observability into
store-level metrics within the custom chart tool.

Release note (bug fix): A bug has been fixed in the DB Console's custom
chart tool, where store-level metrics were not being displayed properly.
Previously, if a store-level metric was selected to be displayed at the
store-level on a multi-store node, only data for the 1st store ID
associated with that node would be displayed.

This patch ensures that data is displayed for all stores present on a
node. Additionally, it updates the behavior to show a single timeseries
for each store, as opposed to aggregating (e.g. SUM) all stores across
the node. This allows finer-grained observability into store-level
metrics when using the custom chart tool in DB Console.
  • Loading branch information
abarganier committed Apr 26, 2024
1 parent aea98c1 commit 1a444b2
Show file tree
Hide file tree
Showing 2 changed files with 63 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
Expand Up @@ -208,56 +208,79 @@ 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) => {
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 = this.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 = this.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 = this.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={this.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 1a444b2

Please sign in to comment.