Skip to content

perf: getLabelName & store legends graph efficiently #9967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
H4ad wants to merge 2 commits into
base: main
Choose a base branch
from
+98 −55
Open

perf: getLabelName & store legends graph efficiently #9967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bdc7409
perf(label-name): reduce number of loops and allocations when generat…
H4ad Jan 10, 2026
4fd98bd
perf(legend): store the selected/ignored more efficiently
H4ad Jan 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion frontend/src/container/GridCardLayout/GridCard/FullView/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,17 @@ export interface DataSetProps {

export interface LegendEntryProps {
label: string;
show: boolean;
show?: boolean;
}

export interface SavedLegendGraph {
name: string;
inverted: boolean;
dataIndex: SavedLegendEntry[];
}

export interface SavedLegendEntry {
label: string;
}

export type ExtendedChartDataset = uPlot.Series & {
Expand Down
26 changes: 18 additions & 8 deletions frontend/src/container/GridCardLayout/GridCard/FullView/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import uPlot from 'uplot';
import {
ExtendedChartDataset,
LegendEntryProps,
SavedLegendGraph,
SaveLegendEntriesToLocalStoreProps,
} from './types';

Expand Down Expand Up @@ -86,17 +87,26 @@ export const saveLegendEntriesToLocalStorage = ({
graphVisibilityState,
name,
}: SaveLegendEntriesToLocalStoreProps): void => {
const newLegendEntry = {
let amountOfFalse = 0;
graphVisibilityState.forEach((state) => {
if (!state) {
amountOfFalse++;
}
});
const storeInverted = amountOfFalse > options.series.length / 2;
const newLegendEntry: SavedLegendGraph = {
name,
dataIndex: options.series.map(
(item, index): LegendEntryProps => ({
inverted: storeInverted,
dataIndex: options.series
.filter((_, index) =>
storeInverted ? graphVisibilityState[index] : !graphVisibilityState[index],
)
.map((item) => ({
label: item.label || '',
show: graphVisibilityState[index],
}),
),
})),
};

let existingEntries: { name: string; dataIndex: LegendEntryProps[] }[] = [];
let existingEntries: SavedLegendGraph[] = [];

try {
existingEntries = JSON.parse(
Expand All @@ -111,7 +121,7 @@ export const saveLegendEntriesToLocalStorage = ({
if (entryIndex >= 0) {
existingEntries[entryIndex] = newLegendEntry;
} else {
existingEntries = [...existingEntries, newLegendEntry];
existingEntries.push(newLegendEntry);
}

try {
Expand Down
24 changes: 10 additions & 14 deletions frontend/src/container/GridCardLayout/GridCard/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { DataSource } from 'types/common/queryBuilder';

import { GraphClickProps } from '../useGraphClickToShowButton';
import { NavigateToExplorerPagesProps } from '../useNavigateToExplorerPages';
import { LegendEntryProps } from './FullView/types';
import { LegendEntryProps, SavedLegendGraph } from './FullView/types';
import {
showAllDataSet,
showAllDataSetFromApiResponse,
Expand Down Expand Up @@ -44,14 +44,12 @@ export const getLocalStorageGraphVisibilityState = ({
],
};

if (localStorage.getItem(LOCALSTORAGE.GRAPH_VISIBILITY_STATES) !== null) {
const legendGraphFromLocalStore = localStorage.getItem(
LOCALSTORAGE.GRAPH_VISIBILITY_STATES,
);
let legendFromLocalStore: {
name: string;
dataIndex: LegendEntryProps[];
}[] = [];
const legendGraphFromLocalStore = localStorage.getItem(
LOCALSTORAGE.GRAPH_VISIBILITY_STATES,
);

if (legendGraphFromLocalStore !== null) {
let legendFromLocalStore: SavedLegendGraph[] = [];

try {
legendFromLocalStore = JSON.parse(legendGraphFromLocalStore || '[]');
Expand All @@ -62,7 +60,6 @@ export const getLocalStorageGraphVisibilityState = ({
);
}

const newGraphVisibilityStates = Array(apiResponse.length + 1).fill(true);
Copy link

@cursor cursor bot Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legend entries replaced with incomplete subset from storage

Medium Severity

The legendEntry property is directly assigned item.dataIndex, which only contains a subset of legends (either visible or hidden items, depending on the inverted flag) stored for efficiency. This replaces the complete legend list (initialized from showAllDataSetFromApiResponse) with an incomplete subset, potentially causing missing legend entries in the UI. The legendEntry needs to be reconstructed to include all entries, not just the stored subset.

🔬 Verification Test

Why verification test was not possible: This requires a browser environment with localStorage and the full React application context. The bug can be verified by tracing the logic:

  1. Initial legendEntry is set with ALL legends from API response (lines 38-44)
  2. When restoring from localStorage, item.dataIndex contains only a subset (e.g., if 3/10 items are hidden and inverted=false, only those 3 hidden items are stored)
  3. Line 66 replaces the complete list with just this subset: visibilityStateAndLegendEntry.legendEntry = item.dataIndex
  4. The result is legendEntry now only has 3 items instead of 10

Why this proves the bug: The storage optimization stores only a subset of legends, but the restore logic incorrectly uses this subset as the complete legend list.

Fix in Cursor Fix in Web

legendFromLocalStore.forEach((item) => {
const newName = name;
if (item.name === newName) {
Expand All @@ -73,11 +70,10 @@ export const getLocalStorageGraphVisibilityState = ({
dataKey.label ===
getLabelName(datasets.metric, datasets.queryName, datasets.legend || ''),
);
if (index !== -1) {
newGraphVisibilityStates[i + 1] = item.dataIndex[index].show;
}
visibilityStateAndLegendEntry.graphVisibilityStates[i + 1] = item.inverted
? index === -1
: index !== -1;
Copy link

@cursor cursor bot Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Visibility state restoration logic is completely inverted

High Severity

The visibility state restoration from localStorage produces the opposite of the saved state. When inverted is true (meaning dataIndex stores visible items), the code returns index === -1 (not found = visible) when it should return index !== -1 (found = visible). Similarly when inverted is false (meaning dataIndex stores hidden items), the code returns index !== -1 (found = visible) when it should return index === -1 (not found = visible). This causes all legend visibility states to be completely inverted when the page is refreshed.

🔬 Verification Test

Why verification test was not possible: This is a frontend React application that requires a browser environment and localStorage. The bug can be verified by tracing the logic manually:

  1. SAVE with storeInverted=true: If user hides 6/8 items (leaves items A, B visible):

    • amountOfFalse = 6, storeInverted = 6 > 4 = true
    • Stores inverted: true with dataIndex = [{label: "A"}, {label: "B"}] (visible items)

  2. RESTORE for item A (found in dataIndex, index=0):

    • item.inverted ? index === -1 : index !== -1
    • true ? (0 === -1) : (0 !== -1) = true ? false : true = false
    • But A was VISIBLE, so result should be true

  3. RESTORE for item C (not in dataIndex, index=-1):

    • item.inverted ? index === -1 : index !== -1
    • true ? (-1 === -1) : (-1 !== -1) = true ? true : false = true
    • But C was HIDDEN, so result should be false

Why this proves the bug: The ternary conditions are swapped. The correct logic would be item.inverted ? index !== -1 : index === -1.

Fix in Cursor Fix in Web

Copy link

@cursor cursor bot Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old localStorage format ignored during migration

Medium Severity

When reading localStorage data saved in the old format (which included show: boolean for each entry and no inverted flag), the new code ignores the show property entirely. Since old data doesn't have inverted, it's undefined (falsy), causing the expression to evaluate to index !== -1. Because old data stored ALL items in dataIndex, every item gets index !== -1 → true, making all series visible regardless of what was actually saved. Users upgrading will lose their saved legend visibility preferences.

🔬 Verification Test

Why verification test was not possible: This requires a browser environment with localStorage. The bug can be verified by tracing the logic:

  1. Old format in localStorage:

    { "name": "widget1", "dataIndex": [{ "label": "A", "show": true }, { "label": "B", "show": false }] }

  2. New code reading old format:

    • item.inverted is undefined (doesn't exist in old format)
    • Expression: undefined ? index === -1 : index !== -1
    • Since undefined is falsy: result is index !== -1
    • Both A and B are in dataIndex, so both get index !== -1 = true
    • Result: Both A and B are visible, even though B had show: false

Why this proves the bug: The old show: false value is completely ignored, causing all previously hidden series to become visible after upgrade.

Fix in Cursor Fix in Web

});
visibilityStateAndLegendEntry.graphVisibilityStates = newGraphVisibilityStates;
}
});
}
Expand Down
91 changes: 59 additions & 32 deletions frontend/src/lib/getLabelName.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,51 @@
import { SeriesItem } from 'types/api/widgets/getQuery';

// eslint-disable-next-line sonarjs/cognitive-complexity
function getLabelNameFromMetric(
metric: SeriesItem['labels'],
query: string,
): string {
let metricName = '';
let pre = '';
let post = '';
let foundName = false;
let hasPreLabels = false;
let hasPostLabels = false;

// eslint-disable-next-line no-restricted-syntax
for (const [key, value] of Object.entries(metric)) {
if (key === '__name__') {
metricName = value || '';
foundName = true;
} else if (foundName) {
if (hasPostLabels) {
post += ',';
}
post += `${key}="${value}"`;
hasPostLabels = true;
} else {
if (hasPreLabels) {
pre += ',';
}
pre += `${key}="${value}"`;
hasPreLabels = true;
}
}

const result = metricName;

if (!foundName && !hasPreLabels && hasPostLabels) {
return query;
}

if (post.length === 0 && pre.length === 0) {
if (result) return result;
if (query) return query;
return result;
}
return `${result}{${pre}${post}}`;
}

const getLabelName = (
metric: SeriesItem['labels'],
query: string,
Expand All @@ -9,46 +55,27 @@ const getLabelName = (
return '';
}

const keysArray = Object.keys(metric);
if (legends.length !== 0) {
const variables = legends
.split('{{')
.filter((e) => e)
.map((e) => e.split('}}')[0]);

const results = variables.map((variable) => metric[variable]);

let endResult = legends;

variables.forEach((e, index) => {
endResult = endResult.replace(`{{${e}}}`, results[index]);
});

return endResult;
}
const startingVariables = legends.split('{{');

const index = keysArray.findIndex((e) => e === '__name__');
// eslint-disable-next-line no-restricted-syntax
for (const variable of startingVariables) {
if (variable) {
const variableName = variable.split('}}')[0];
const variableValue = metric[variableName] || '';

const preArray = index !== -1 ? keysArray.slice(0, index) : [];
const postArray = keysArray.slice(index + 1, keysArray.length);
if (variableValue) {
endResult = endResult.replace(`{{${variableName}}}`, variableValue);
}
}
}

if (index === undefined && preArray.length === 0 && postArray.length) {
return query;
return endResult;
}

const post = postArray.map((e) => `${e}="${metric[e]}"`).join(',');
const pre = preArray.map((e) => `${e}="${metric[e]}"`).join(',');

const value = metric[keysArray[index]];

const result = `${value === undefined ? '' : value}`;

if (post.length === 0 && pre.length === 0) {
if (result) return result;
if (query) return query;
return result;
}
return `${result}{${pre}${post}}`;
return getLabelNameFromMetric(metric, query);
};

export default getLabelName;
Loading