From 583d1777896c830fadf8fb7230904adcebb3d680 Mon Sep 17 00:00:00 2001 From: "grafana-delivery-bot[bot]" <132647405+grafana-delivery-bot[bot]@users.noreply.github.com> Date: Mon, 6 Oct 2025 14:56:29 +0200 Subject: [PATCH] [release-12.1.3] FlameGraph: Ensure total is only counted once for recursive function calls (#111605) FlameGraph: Ensure total is only counted once for recursive function calls (#111548) grafana-flamegraph: Ensure total is only counted once for recursive function calls Example flamegraph: https://flamegraph.com/share/2bb59df3-9930-11f0-94ec-760777e76ccd (cherry picked from commit c5f6318b7b82e3e89e2ba1f583e3695cc40c2db7) Co-authored-by: Christian Simon --- .../FlameGraphTopTableContainer.test.tsx | 116 +++++++++++++++++- .../TopTable/FlameGraphTopTableContainer.tsx | 68 ++++++---- 2 files changed, 160 insertions(+), 24 deletions(-) diff --git a/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.test.tsx b/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.test.tsx index 0396e536535..2e9de6e44aa 100644 --- a/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.test.tsx +++ b/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.test.tsx @@ -5,9 +5,10 @@ import { createDataFrame } from '@grafana/data'; import { FlameGraphDataContainer } from '../FlameGraph/dataTransform'; import { data } from '../FlameGraph/testData/dataNestedSet'; +import { textToDataContainer } from '../FlameGraph/testHelpers'; import { ColorScheme } from '../types'; -import FlameGraphTopTableContainer from './FlameGraphTopTableContainer'; +import FlameGraphTopTableContainer, { buildFilteredTable } from './FlameGraphTopTableContainer'; describe('FlameGraphTopTableContainer', () => { const setup = () => { @@ -48,7 +49,10 @@ describe('FlameGraphTopTableContainer', () => { expect(cells).toHaveLength(60); // 16 rows expect(cells[1].textContent).toEqual('net/http.HandlerFunc.ServeHTTP'); expect(cells[2].textContent).toEqual('31.7 K'); - expect(cells[3].textContent).toEqual('31.7 Bil'); + expect(cells[3].textContent).toEqual('5.58 Bil'); + expect(cells[5].textContent).toEqual('total'); + expect(cells[6].textContent).toEqual('16.5 K'); + expect(cells[7].textContent).toEqual('16.5 Bil'); expect(cells[25].textContent).toEqual('net/http.(*conn).serve'); expect(cells[26].textContent).toEqual('5.63 K'); expect(cells[27].textContent).toEqual('5.63 Bil'); @@ -74,3 +78,111 @@ describe('FlameGraphTopTableContainer', () => { expect(mocks.onSandwich).toHaveBeenCalledWith('net/http.HandlerFunc.ServeHTTP'); }); }); + +describe('buildFilteredTable', () => { + it('should group data by label and sum values', () => { + const container = textToDataContainer(` +[0////] +[1][2] +[3][4] + `); + + const result = buildFilteredTable(container!); + + expect(result).toEqual({ + '0': { self: 1, total: 7, totalRight: 0 }, + '1': { self: 0, total: 3, totalRight: 0 }, + '2': { self: 0, total: 3, totalRight: 0 }, + '3': { self: 3, total: 3, totalRight: 0 }, + '4': { self: 3, total: 3, totalRight: 0 }, + }); + }); + + it('should sum values for duplicate labels', () => { + const container = textToDataContainer(` +[0///] +[1][1] + `); + + const result = buildFilteredTable(container!); + + expect(result).toEqual({ + '0': { self: 0, total: 6, totalRight: 0 }, + '1': { self: 6, total: 6, totalRight: 0 }, + }); + }); + + it('should filter by matchedLabels when provided', () => { + const container = textToDataContainer(` +[0////] +[1][2] +[3][4] + `); + + const matchedLabels = new Set(['1', '3']); + const result = buildFilteredTable(container!, matchedLabels); + + expect(result).toEqual({ + '1': { self: 0, total: 3, totalRight: 0 }, + '3': { self: 3, total: 3, totalRight: 0 }, + }); + }); + + it('should handle empty matchedLabels set', () => { + const container = textToDataContainer(` +[0////] +[1][2] +[3][4] + `); + + const matchedLabels = new Set(); + const result = buildFilteredTable(container!, matchedLabels); + + expect(result).toEqual({}); + }); + + it('should handle data with no matches', () => { + const container = textToDataContainer(` +[0////] +[1][2] +[3][4] + `); + + const matchedLabels = new Set(['9']); + const result = buildFilteredTable(container!, matchedLabels); + + expect(result).toEqual({}); + }); + + it('should work without matchedLabels filter', () => { + const container = textToDataContainer(` +[0] +[1] + `); + + const result = buildFilteredTable(container!); + + expect(result).toEqual({ + '0': { self: 0, total: 3, totalRight: 0 }, + '1': { self: 3, total: 3, totalRight: 0 }, + }); + }); + it('should not inflate totals for recursive calls', () => { + const container = textToDataContainer(` +[0////] +[1][2] +[3][4] +[0] + `); + + const result = buildFilteredTable(container!); + + expect(result).toEqual({ + '0': { self: 4, total: 7, totalRight: 0 }, + '1': { self: 0, total: 3, totalRight: 0 }, + '2': { self: 0, total: 3, totalRight: 0 }, + '3': { self: 0, total: 3, totalRight: 0 }, + '4': { self: 3, total: 3, totalRight: 0 }, + }); + }); +}); diff --git a/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx b/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx index b3a1be9a3b3..8c5461bb053 100644 --- a/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx +++ b/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx @@ -53,28 +53,7 @@ const FlameGraphTopTableContainer = memo( onTableSort, colorScheme, }: Props) => { - const table = useMemo(() => { - // Group the data by label, we show only one row per label and sum the values - // TODO: should be by filename + funcName + linenumber? - let filteredTable: { [key: string]: TableData } = Object.create(null); - for (let i = 0; i < data.data.length; i++) { - const value = data.getValue(i); - const valueRight = data.getValueRight(i); - const self = data.getSelf(i); - const label = data.getLabel(i); - - // If user is doing text search we filter out labels in the same way we highlight them in flame graph. - if (!matchedLabels || matchedLabels.has(label)) { - filteredTable[label] = filteredTable[label] || {}; - filteredTable[label].self = filteredTable[label].self ? filteredTable[label].self + self : self; - filteredTable[label].total = filteredTable[label].total ? filteredTable[label].total + value : value; - filteredTable[label].totalRight = filteredTable[label].totalRight - ? filteredTable[label].totalRight + valueRight - : valueRight; - } - } - return filteredTable; - }, [data, matchedLabels]); + const table = useMemo(() => buildFilteredTable(data, matchedLabels), [data, matchedLabels]); const styles = useStyles2(getStyles); const theme = useTheme2(); @@ -124,6 +103,49 @@ const FlameGraphTopTableContainer = memo( FlameGraphTopTableContainer.displayName = 'FlameGraphTopTableContainer'; +function buildFilteredTable(data: FlameGraphDataContainer, matchedLabels?: Set) { + // Group the data by label, we show only one row per label and sum the values + // TODO: should be by filename + funcName + linenumber? + let filteredTable: { [key: string]: TableData } = Object.create(null); + + // Track call stack to detect recursive calls + const callStack: string[] = []; + + for (let i = 0; i < data.data.length; i++) { + const value = data.getValue(i); + const valueRight = data.getValueRight(i); + const self = data.getSelf(i); + const label = data.getLabel(i); + const level = data.getLevel(i); + + // Maintain call stack based on level changes + while (callStack.length > level) { + callStack.pop(); + } + + // Check if this is a recursive call (same label already in call stack) + const isRecursive = callStack.some((entry) => entry === label); + + // If user is doing text search we filter out labels in the same way we highlight them in flame graph. + if (!matchedLabels || matchedLabels.has(label)) { + filteredTable[label] = filteredTable[label] || {}; + filteredTable[label].self = filteredTable[label].self ? filteredTable[label].self + self : self; + + // Only add to total if this is not a recursive call + if (!isRecursive) { + filteredTable[label].total = filteredTable[label].total ? filteredTable[label].total + value : value; + filteredTable[label].totalRight = filteredTable[label].totalRight + ? filteredTable[label].totalRight + valueRight + : valueRight; + } + } + + // Add current call to the stack + callStack.push(label); + } + return filteredTable; +} + function buildTableDataFrame( data: FlameGraphDataContainer, table: { [key: string]: TableData }, @@ -365,4 +387,6 @@ const getStylesActionCell = () => { }; }; +export { buildFilteredTable }; + export default FlameGraphTopTableContainer;