diff --git a/e2e/smoke-tests-suite/panels_smokescreen.spec.ts b/e2e/smoke-tests-suite/panels_smokescreen.spec.ts index a914014066f..139e4e9502b 100644 --- a/e2e/smoke-tests-suite/panels_smokescreen.spec.ts +++ b/e2e/smoke-tests-suite/panels_smokescreen.spec.ts @@ -28,7 +28,7 @@ e2e.scenario({ // Loop through every panel type and ensure no crash Object.entries(win.grafanaBootData.settings.panels).forEach(([_, panel]) => { // TODO: Remove Flame Graph check as part of addressing #66803 - if (!panel.hideFromList && panel.state !== 'deprecated' && panel.name !== 'Flame Graph') { + if (!panel.hideFromList && panel.state !== 'deprecated') { e2e.components.PanelEditor.toggleVizPicker().click(); e2e.components.PluginVisualization.item(panel.name).scrollIntoView().should('be.visible').click(); diff --git a/public/app/plugins/panel/flamegraph/FlameGraphPanel.tsx b/public/app/plugins/panel/flamegraph/FlameGraphPanel.tsx index 5f3212f6e69..ba1e13a2674 100644 --- a/public/app/plugins/panel/flamegraph/FlameGraphPanel.tsx +++ b/public/app/plugins/panel/flamegraph/FlameGraphPanel.tsx @@ -1,9 +1,17 @@ import React from 'react'; import { CoreApp, PanelProps } from '@grafana/data'; +import { PanelDataErrorView } from '@grafana/runtime'; +import { checkFields, getMessageCheckFieldsResult } from './components/FlameGraph/dataTransform'; import FlameGraphContainer from './components/FlameGraphContainer'; export const FlameGraphPanel = (props: PanelProps) => { + const wrongFields = checkFields(props.data.series[0]); + if (wrongFields) { + return ( + + ); + } return ; }; diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.test.ts b/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.test.ts index d9e28bb2403..397db71d77d 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.test.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.test.ts @@ -1,4 +1,4 @@ -import { createDataFrame } from '@grafana/data'; +import { createDataFrame, FieldType } from '@grafana/data'; import { FlameGraphDataContainer, LevelItem, nestedSetToLevels } from './dataTransform'; @@ -13,7 +13,7 @@ describe('nestedSetToLevels', () => { fields: [ { name: 'level', values: [0, 1, 2, 3, 2, 1, 2, 3, 4] }, { name: 'value', values: [10, 5, 3, 1, 1, 4, 3, 2, 1] }, - { name: 'label', values: ['1', '2', '3', '4', '5', '6', '7', '8', '9'] }, + { name: 'label', values: ['1', '2', '3', '4', '5', '6', '7', '8', '9'], type: FieldType.string }, { name: 'self', values: [0, 0, 0, 0, 0, 0, 0, 0, 0] }, ], }); @@ -50,7 +50,7 @@ describe('nestedSetToLevels', () => { fields: [ { name: 'level', values: [0, 1, 1, 1] }, { name: 'value', values: [10, 5, 3, 1] }, - { name: 'label', values: ['1', '2', '3', '4'] }, + { name: 'label', values: ['1', '2', '3', '4'], type: FieldType.string }, { name: 'self', values: [10, 5, 3, 1] }, ], }); diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.ts b/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.ts index a272e962863..ac100c5c2cf 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.ts @@ -1,4 +1,12 @@ -import { createTheme, DataFrame, DisplayProcessor, Field, getDisplayProcessor, GrafanaTheme2 } from '@grafana/data'; +import { + createTheme, + DataFrame, + DisplayProcessor, + Field, + FieldType, + getDisplayProcessor, + GrafanaTheme2, +} from '@grafana/data'; import { SampleUnit } from '../types'; @@ -69,6 +77,57 @@ export function nestedSetToLevels(container: FlameGraphDataContainer): [LevelIte return [levels, uniqueLabels]; } +export function getMessageCheckFieldsResult(wrongFields: CheckFieldsResult) { + if (wrongFields.missingFields.length) { + return `Data is missing fields: ${wrongFields.missingFields.join(', ')}`; + } + + if (wrongFields.wrongTypeFields.length) { + return `Data has fields of wrong type: ${wrongFields.wrongTypeFields + .map((f) => `${f.name} has type ${f.type} but should be ${f.expectedTypes.join(' or ')}`) + .join(', ')}`; + } + + return ''; +} + +export type CheckFieldsResult = { + wrongTypeFields: Array<{ name: string; expectedTypes: FieldType[]; type: FieldType }>; + missingFields: string[]; +}; + +export function checkFields(data: DataFrame): CheckFieldsResult | undefined { + const fields: Array<[string, FieldType[]]> = [ + ['label', [FieldType.string, FieldType.enum]], + ['level', [FieldType.number]], + ['value', [FieldType.number]], + ['self', [FieldType.number]], + ]; + + const missingFields = []; + const wrongTypeFields = []; + + for (const field of fields) { + const [name, types] = field; + const frameField = data.fields.find((f) => f.name === name); + if (!frameField) { + missingFields.push(name); + continue; + } + if (!types.includes(frameField.type)) { + wrongTypeFields.push({ name, expectedTypes: types, type: frameField.type }); + } + } + + if (missingFields.length > 0 || wrongTypeFields.length > 0) { + return { + wrongTypeFields, + missingFields, + }; + } + return undefined; +} + export class FlameGraphDataContainer { data: DataFrame; labelField: Field; @@ -85,15 +144,17 @@ export class FlameGraphDataContainer { constructor(data: DataFrame, theme: GrafanaTheme2 = createTheme()) { this.data = data; + + const wrongFields = checkFields(data); + if (wrongFields) { + throw new Error(getMessageCheckFieldsResult(wrongFields)); + } + this.labelField = data.fields.find((f) => f.name === 'label')!; this.levelField = data.fields.find((f) => f.name === 'level')!; this.valueField = data.fields.find((f) => f.name === 'value')!; this.selfField = data.fields.find((f) => f.name === 'self')!; - if (!(this.labelField && this.levelField && this.valueField && this.selfField)) { - throw new Error('Malformed dataFrame: value, level and label and self fields are required.'); - } - const enumConfig = this.labelField?.config?.type?.enum; // 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 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 729dfad1d89..3d889c12364 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.test.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.test.ts @@ -1,4 +1,4 @@ -import { createDataFrame } from '@grafana/data'; +import { createDataFrame, FieldType } from '@grafana/data'; import { FlameGraphDataContainer, LevelItem } from './dataTransform'; import { getRectDimensionsForLevel } from './rendering'; @@ -8,6 +8,7 @@ function makeDataFrame(fields: Record>) { fields: Object.keys(fields).map((key) => ({ name: key, values: fields[key], + type: typeof fields[key][0] === 'string' ? FieldType.string : FieldType.number, })), }); } diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/testHelpers.ts b/public/app/plugins/panel/flamegraph/components/FlameGraph/testHelpers.ts index 510c3397064..fc42c5bf2ac 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/testHelpers.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/testHelpers.ts @@ -1,4 +1,4 @@ -import { arrayToDataFrame } from '@grafana/data'; +import { arrayToDataFrame, FieldType } from '@grafana/data'; import { FlameGraphDataContainer, LevelItem } from './dataTransform'; @@ -77,6 +77,8 @@ export function textToDataContainer(text: string) { } const df = arrayToDataFrame(dfSorted); + const labelField = df.fields.find((f) => f.name === 'label')!; + labelField.type = FieldType.string; return new FlameGraphDataContainer(df); } diff --git a/public/app/plugins/panel/flamegraph/suggestions.ts b/public/app/plugins/panel/flamegraph/suggestions.ts index b8fbf4af50d..3421e57c16c 100644 --- a/public/app/plugins/panel/flamegraph/suggestions.ts +++ b/public/app/plugins/panel/flamegraph/suggestions.ts @@ -1,7 +1,7 @@ import { VisualizationSuggestionsBuilder } from '@grafana/data'; import { SuggestionName } from 'app/types/suggestions'; -import { FlameGraphDataContainer as FlameGraphDataContainer } from './components/FlameGraph/dataTransform'; +import { checkFields } from './components/FlameGraph/dataTransform'; export class FlameGraphSuggestionsSupplier { getListWithDefaults(builder: VisualizationSuggestionsBuilder) { @@ -16,13 +16,12 @@ export class FlameGraphSuggestionsSupplier { return; } - // Try to instantiate FlameGraphDataContainer (depending on the version), since the instantiation can fail due - // to the format of the data - meaning that a Flame Graph cannot be used to visualize those data. - // Without this check, a suggestion containing an error is shown to the user. const dataFrame = builder.data.series[0]; - try { - new FlameGraphDataContainer(dataFrame); - } catch (err) { + if (!dataFrame) { + return; + } + const wrongFields = checkFields(dataFrame); + if (wrongFields) { return; }