From 664721720809e85e10c08b3befef111d69713775 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Mon, 13 Mar 2023 11:06:04 +0100 Subject: [PATCH] Phlare: Use enum config to send deduplicated func and filenames (#64435) --- pkg/services/ngalert/state/cache_test.go | 8 +-- pkg/tsdb/phlare/query.go | 64 +++++++++++++++---- pkg/tsdb/phlare/query_test.go | 19 +++++- .../components/FlameGraph/FlameGraph.test.tsx | 1 + .../components/FlameGraph/FlameGraph.tsx | 36 +++++++++-- .../FlameGraph/FlameGraphTooltip.tsx | 5 +- .../components/FlameGraph/rendering.test.ts | 9 ++- .../components/FlameGraph/rendering.ts | 10 +-- .../components/FlameGraphContainer.tsx | 25 +++++++- .../FlameGraphTopTableContainer.test.tsx | 1 + .../TopTable/FlameGraphTopTableContainer.tsx | 16 +++-- 11 files changed, 152 insertions(+), 42 deletions(-) diff --git a/pkg/services/ngalert/state/cache_test.go b/pkg/services/ngalert/state/cache_test.go index 521233283c5..aa9384af36d 100644 --- a/pkg/services/ngalert/state/cache_test.go +++ b/pkg/services/ngalert/state/cache_test.go @@ -239,8 +239,8 @@ func Test_getOrCreate(t *testing.T) { result := eval.Result{ Instance: models.GenerateAlertLabels(5, "result-"), Values: map[string]eval.NumberValueCapture{ - "A": eval.NumberValueCapture{Var: "A", Value: util.Pointer(1.0)}, - "B": eval.NumberValueCapture{Var: "B", Value: util.Pointer(2.0)}, + "A": {Var: "A", Value: util.Pointer(1.0)}, + "B": {Var: "B", Value: util.Pointer(2.0)}, }, } rule := generateRule() @@ -253,8 +253,8 @@ func Test_getOrCreate(t *testing.T) { result := eval.Result{ Instance: models.GenerateAlertLabels(5, "result-"), Values: map[string]eval.NumberValueCapture{ - "B0": eval.NumberValueCapture{Var: "B", Value: util.Pointer(1.0)}, - "B1": eval.NumberValueCapture{Var: "B", Value: util.Pointer(2.0)}, + "B0": {Var: "B", Value: util.Pointer(1.0)}, + "B1": {Var: "B", Value: util.Pointer(2.0)}, }, } rule := generateRule() diff --git a/pkg/tsdb/phlare/query.go b/pkg/tsdb/phlare/query.go index 29c38f57f9a..73ea46db69c 100644 --- a/pkg/tsdb/phlare/query.go +++ b/pkg/tsdb/phlare/query.go @@ -15,10 +15,9 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/grafana/grafana-plugin-sdk-go/live" "github.com/grafana/grafana/pkg/tsdb/phlare/kinds/dataquery" - "github.com/xlab/treeprint" - googlev1 "github.com/grafana/phlare/api/gen/proto/go/google/v1" querierv1 "github.com/grafana/phlare/api/gen/proto/go/querier/v1" + "github.com/xlab/treeprint" ) type queryModel struct { @@ -92,7 +91,7 @@ func (d *PhlareDatasource) query(ctx context.Context, pCtx backend.PluginContext response.Error = err return response } - frame := responseToDataFrames(resp, qm.ProfileTypeId) + frame := responseToDataFrames(resp.Msg, qm.ProfileTypeId) response.Frames = append(response.Frames, frame) // If query called with streaming on then return a channel @@ -125,8 +124,8 @@ func makeRequest(qm queryModel, query backend.DataQuery) *connect.Request[querie // responseToDataFrames turns Phlare response to data.Frame. We encode the data into a nested set format where we have // [level, value, label] columns and by ordering the items in a depth first traversal order we can recreate the whole // tree back. -func responseToDataFrames(resp *connect.Response[googlev1.Profile], profileTypeID string) *data.Frame { - tree := profileAsTree(resp.Msg) +func responseToDataFrames(prof *googlev1.Profile, profileTypeID string) *data.Frame { + tree := profileAsTree(prof) return treeToNestedSetDataFrame(tree, profileTypeID) } @@ -354,8 +353,8 @@ type CustomMeta struct { } // treeToNestedSetDataFrame walks the tree depth first and adds items into the dataframe. This is a nested set format -// where by ordering the items in depth first order and knowing the level/depth of each item we can recreate the -// parent - child relationship without explicitly needing parent/child column and we can later just iterate over the +// where ordering the items in depth first order and knowing the level/depth of each item we can recreate the +// parent - child relationship without explicitly needing parent/child column, and we can later just iterate over the // dataFrame to again basically walking depth first over the tree/profile. func treeToNestedSetDataFrame(tree *ProfileTree, profileTypeID string) *data.Frame { frame := data.NewFrame("response") @@ -369,10 +368,11 @@ func treeToNestedSetDataFrame(tree *ProfileTree, profileTypeID string) *data.Fra parts := strings.Split(profileTypeID, ":") valueField.Config = &data.FieldConfig{Unit: normalizeUnit(parts[2])} selfField.Config = &data.FieldConfig{Unit: normalizeUnit(parts[2])} - labelField := data.NewField("label", nil, []string{}) lineNumberField := data.NewField("line", nil, []int64{}) - fileNameField := data.NewField("fileName", nil, []string{}) - frame.Fields = data.Fields{levelField, valueField, selfField, labelField, lineNumberField, fileNameField} + frame.Fields = data.Fields{levelField, valueField, selfField, lineNumberField} + + labelField := NewEnumField("label", nil) + fileNameField := NewEnumField("fileName", nil) walkTree(tree, func(tree *ProfileTree) { levelField.Append(int64(tree.Level)) @@ -380,13 +380,55 @@ func treeToNestedSetDataFrame(tree *ProfileTree, profileTypeID string) *data.Fra selfField.Append(tree.Self) // todo: inline functions // tree.Inlined - labelField.Append(tree.Function.FunctionName) lineNumberField.Append(tree.Function.Line) + labelField.Append(tree.Function.FunctionName) fileNameField.Append(tree.Function.FileName) }) + + frame.Fields = append(frame.Fields, labelField.GetField(), fileNameField.GetField()) return frame } +type EnumField struct { + field *data.Field + valuesMap map[string]int64 + counter int64 +} + +func NewEnumField(name string, labels data.Labels) *EnumField { + return &EnumField{ + field: data.NewField(name, labels, []int64{}), + valuesMap: make(map[string]int64), + } +} + +func (e *EnumField) Append(value string) { + if valueIndex, ok := e.valuesMap[value]; ok { + e.field.Append(valueIndex) + } else { + e.valuesMap[value] = e.counter + e.field.Append(e.counter) + e.counter++ + } +} + +func (e *EnumField) GetField() *data.Field { + s := make([]string, len(e.valuesMap)) + for k, v := range e.valuesMap { + s[v] = k + } + + e.field.SetConfig(&data.FieldConfig{ + TypeConfig: &data.FieldTypeConfig{ + Enum: &data.EnumFieldConfig{ + Text: s, + }, + }, + }) + + return e.field +} + func walkTree(tree *ProfileTree, fn func(tree *ProfileTree)) { fn(tree) stack := tree.Nodes diff --git a/pkg/tsdb/phlare/query_test.go b/pkg/tsdb/phlare/query_test.go index e98413c43aa..e2d76011ec8 100644 --- a/pkg/tsdb/phlare/query_test.go +++ b/pkg/tsdb/phlare/query_test.go @@ -121,14 +121,29 @@ func Test_treeToNestedDataFrame(t *testing.T) { } frame := treeToNestedSetDataFrame(tree, "memory:alloc_objects:count:space:bytes") + + labelConfig := &data.FieldConfig{ + TypeConfig: &data.FieldTypeConfig{ + Enum: &data.EnumFieldConfig{ + Text: []string{"root", "func1", "func2", "func1:func3"}, + }, + }, + } + filenameConfig := &data.FieldConfig{ + TypeConfig: &data.FieldTypeConfig{ + Enum: &data.EnumFieldConfig{ + Text: []string{"", "1", "2", "3"}, + }, + }, + } require.Equal(t, []*data.Field{ data.NewField("level", nil, []int64{0, 1, 1, 2}), data.NewField("value", nil, []int64{100, 40, 30, 15}).SetConfig(&data.FieldConfig{Unit: "short"}), data.NewField("self", nil, []int64{1, 2, 3, 4}).SetConfig(&data.FieldConfig{Unit: "short"}), - data.NewField("label", nil, []string{"root", "func1", "func2", "func1:func3"}), data.NewField("line", nil, []int64{0, 1, 2, 3}), - data.NewField("fileName", nil, []string{"", "1", "2", "3"}), + data.NewField("label", nil, []int64{0, 1, 2, 3}).SetConfig(labelConfig), + data.NewField("fileName", nil, []int64{0, 1, 2, 3}).SetConfig(filenameConfig), }, frame.Fields) } diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx index ceb2855ab39..4fb7d0234fe 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx @@ -47,6 +47,7 @@ describe('FlameGraph', () => { setRangeMin={setRangeMin} setRangeMax={setRangeMax} selectedView={selectedView} + getLabelValue={(val) => val.toString()} /> ); }; diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.tsx index c72381de55a..6b4354c5005 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.tsx @@ -48,6 +48,7 @@ type Props = { setRangeMax: (range: number) => void; selectedView: SelectedView; style?: React.CSSProperties; + getLabelValue: (label: string | number) => string; }; const FlameGraph = ({ @@ -65,11 +66,13 @@ const FlameGraph = ({ setRangeMin, setRangeMax, selectedView, + getLabelValue, }: Props) => { const styles = getStyles(selectedView, app, flameGraphHeight); const totalTicks = data.fields[1].values.get(0); const valueField = data.fields.find((f) => f.name === 'value') ?? data.fields.find((f) => f.type === FieldType.number); + if (!valueField) { throw new Error('Malformed dataFrame: value field of type number is not in the query response'); } @@ -85,7 +88,13 @@ const FlameGraph = ({ }); const uniqueLabels = useMemo(() => { - return [...new Set(data.fields.find((f) => f.name === 'label')?.values.toArray())]; + const labelField = data.fields.find((f) => f.name === 'label'); + const enumConfig = labelField?.config?.type?.enum; + if (enumConfig) { + return enumConfig.text || []; + } else { + return [...new Set(labelField?.values.toArray())]; + } }, [data]); const foundLabels = useMemo(() => { @@ -131,14 +140,33 @@ const FlameGraph = ({ const level = levels[levelIndex]; // Get all the dimensions of the rectangles for the level. We do this by level instead of per rectangle, because // sometimes we collapse multiple bars into single rect. - const dimensions = getRectDimensionsForLevel(level, levelIndex, totalTicks, rangeMin, pixelsPerTick, processor); + const dimensions = getRectDimensionsForLevel( + level, + levelIndex, + totalTicks, + rangeMin, + pixelsPerTick, + processor, + getLabelValue + ); for (const rect of dimensions) { // Render each rectangle based on the computed dimensions renderRect(ctx, rect, totalTicks, rangeMin, rangeMax, search, levelIndex, topLevelIndex, foundLabels); } } }, - [levels, wrapperWidth, valueField, totalTicks, rangeMin, rangeMax, search, topLevelIndex, foundLabels] + [ + levels, + wrapperWidth, + valueField, + totalTicks, + rangeMin, + rangeMax, + search, + topLevelIndex, + foundLabels, + getLabelValue, + ] ); useEffect(() => { @@ -235,7 +263,7 @@ const FlameGraph = ({
- + {contextMenuData && ( ; tooltipData: TooltipData; + getLabelValue: (label: string | number) => string; }; -const FlameGraphTooltip = ({ tooltipRef, tooltipData }: Props) => { +const FlameGraphTooltip = ({ tooltipRef, tooltipData, getLabelValue }: Props) => { const styles = useStyles2(getStyles); return ( @@ -20,7 +21,7 @@ const FlameGraphTooltip = ({ tooltipRef, tooltipData }: Props) => { -

{tooltipData.name}

+

{getLabelValue(tooltipData.name)}

{tooltipData.unitTitle}
diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.test.ts b/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.test.ts index 888a2284b79..6b476e0f3f0 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.test.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.test.ts @@ -12,7 +12,8 @@ describe('getRectDimensionsForLevel', () => { 100, 0, 10, - getDisplayProcessor({ field: { config: {} }, theme: createTheme() }) + getDisplayProcessor({ field: { config: {} }, theme: createTheme() }), + (val) => val.toString() ); expect(result).toEqual([ { @@ -40,7 +41,8 @@ describe('getRectDimensionsForLevel', () => { 100, 0, 10, - getDisplayProcessor({ field: { config: {} }, theme: createTheme() }) + getDisplayProcessor({ field: { config: {} }, theme: createTheme() }), + (val) => val.toString() ); expect(result).toEqual([ { width: 999, height: 22, x: 0, y: 44, collapsed: false, ticks: 100, label: '1', unitLabel: '100' }, @@ -61,7 +63,8 @@ describe('getRectDimensionsForLevel', () => { 100, 0, 1, - getDisplayProcessor({ field: { config: {} }, theme: createTheme() }) + getDisplayProcessor({ field: { config: {} }, theme: createTheme() }), + (val) => val.toString() ); expect(result).toEqual([ { width: 99, height: 22, x: 0, y: 44, collapsed: false, ticks: 100, label: '1', unitLabel: '100' }, diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.ts b/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.ts index 22dbfac9eb8..bf528d42609 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.ts @@ -26,11 +26,6 @@ type RectData = { /** * Compute the pixel coordinates for each bar in a level. We need full level of bars so that we can collapse small bars * into bigger rects. - * @param level - * @param levelIndex - * @param totalTicks - * @param rangeMin - * @param pixelsPerTick */ export function getRectDimensionsForLevel( level: ItemWithStart[], @@ -38,7 +33,8 @@ export function getRectDimensionsForLevel( totalTicks: number, rangeMin: number, pixelsPerTick: number, - processor: DisplayProcessor + processor: DisplayProcessor, + getLabelValue: (value: number | string) => string ): RectData[] { const coordinatesLevel = []; for (let barIndex = 0; barIndex < level.length; barIndex += 1) { @@ -70,7 +66,7 @@ export function getRectDimensionsForLevel( y: levelIndex * PIXELS_PER_LEVEL, collapsed, ticks: curBarTicks, - label: item.label, + label: getLabelValue(item.label), unitLabel: unit, }); } diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx index dc1b41dc31a..0fc5aaef14a 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx @@ -1,8 +1,8 @@ import { css } from '@emotion/css'; -import React, { useEffect, useMemo, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import { useMeasure } from 'react-use'; -import { DataFrame, DataFrameView, CoreApp } from '@grafana/data'; +import { DataFrame, DataFrameView, CoreApp, getEnumDisplayProcessor, createTheme } from '@grafana/data'; import { useStyles2 } from '@grafana/ui'; import { MIN_WIDTH_TO_SHOW_BOTH_TOPTABLE_AND_FLAMEGRAPH, PIXELS_PER_LEVEL } from '../constants'; @@ -14,7 +14,7 @@ import FlameGraphTopTableContainer from './TopTable/FlameGraphTopTableContainer' import { SelectedView } from './types'; type Props = { - data: DataFrame; + data?: DataFrame; app: CoreApp; // Height for flame graph when not used in explore. // This needs to be different to explore flame graph height as we @@ -31,6 +31,23 @@ const FlameGraphContainer = (props: Props) => { const [selectedView, setSelectedView] = useState(SelectedView.Both); const [sizeRef, { width: containerWidth }] = useMeasure(); + const labelField = props.data?.fields.find((f) => f.name === 'label'); + + // Label can actually be an enum field so depending on that we have to access it through display processor. This is + // both a backward compatibility but also to allow using a simple dataFrame without enum config. This would allow + // users to use this panel with correct query from data sources that do not return profiles natively. + const getLabelValue = useCallback( + (label: string | number) => { + const enumConfig = labelField?.config?.type?.enum; + if (enumConfig) { + return getEnumDisplayProcessor(createTheme(), enumConfig)(label).text; + } else { + return label.toString(); + } + }, + [labelField] + ); + // Transform dataFrame with nested set format to array of levels. Each level contains all the bars for a particular // level of the flame graph. We do this temporary as in the end we should be able to render directly by iterating // over the dataFrame rows. @@ -91,6 +108,7 @@ const FlameGraphContainer = (props: Props) => { setSelectedBarIndex={setSelectedBarIndex} setRangeMin={setRangeMin} setRangeMax={setRangeMax} + getLabelValue={getLabelValue} /> )} @@ -110,6 +128,7 @@ const FlameGraphContainer = (props: Props) => { setRangeMin={setRangeMin} setRangeMax={setRangeMax} selectedView={selectedView} + getLabelValue={getLabelValue} /> )} diff --git a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx index 0bf006f262c..4bb58c62063 100644 --- a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx +++ b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx @@ -30,6 +30,7 @@ describe('FlameGraphTopTableContainer', () => { setSelectedBarIndex={jest.fn()} setRangeMin={jest.fn()} setRangeMax={jest.fn()} + getLabelValue={(val) => val.toString()} /> ); }; diff --git a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx index b3ac0f1cbf8..7a5eaffda89 100644 --- a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx +++ b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx @@ -21,6 +21,7 @@ type Props = { setSelectedBarIndex: (bar: number) => void; setRangeMin: (range: number) => void; setRangeMax: (range: number) => void; + getLabelValue: (label: string | number) => string; }; const FlameGraphTopTableContainer = ({ @@ -34,26 +35,29 @@ const FlameGraphTopTableContainer = ({ setSelectedBarIndex, setRangeMin, setRangeMax, + getLabelValue, }: Props) => { const styles = useStyles2(() => getStyles(selectedView, app)); const [topTable, setTopTable] = useState(); const valueField = data.fields.find((f) => f.name === 'value') ?? data.fields.find((f) => f.type === FieldType.number); + const selfField = data.fields.find((f) => f.name === 'self') ?? data.fields.find((f) => f.type === FieldType.number); + const labelsField = data.fields.find((f) => f.name === 'label'); const sortLevelsIntoTable = useCallback(() => { let label, self, value; let table: { [key: string]: TableData } = {}; - if (data.fields.length > 3) { - const valueValues = data.fields[1].values; - const selfValues = data.fields[2].values; - const labelValues = data.fields[3].values; + if (valueField && selfField && labelsField) { + const valueValues = valueField.values; + const selfValues = selfField.values; + const labelValues = labelsField.values; for (let i = 0; i < valueValues.length; i++) { value = valueValues.get(i); self = selfValues.get(i); - label = labelValues.get(i); + label = getLabelValue(labelValues.get(i)); table[label] = table[label] || {}; table[label].self = table[label].self ? table[label].self + self : self; table[label].total = table[label].total ? table[label].total + value : value; @@ -61,7 +65,7 @@ const FlameGraphTopTableContainer = ({ } return table; - }, [data.fields]); + }, [getLabelValue, selfField, valueField, labelsField]); const getTopTableData = (field: Field, value: number) => { const processor = getDisplayProcessor({ field, theme: createTheme() /* theme does not matter for us here */ });