From 0eda368d32dbd7f57635cabd6b7d6870ca7b58ee Mon Sep 17 00:00:00 2001 From: Oscar Kilhed Date: Thu, 2 Nov 2023 16:47:42 +0100 Subject: [PATCH] Transformations: Deduplicate names when using `extract fields` transformation. (#77569) * dedupe field names * add test to make sure we don't break normal case --- .../feature-toggles/index.md | 1 + packages/grafana-data/src/field/fieldState.ts | 2 +- packages/grafana-data/src/field/index.ts | 2 +- .../src/types/featureToggles.gen.ts | 1 + pkg/services/featuremgmt/registry.go | 7 +++++ pkg/services/featuremgmt/toggles_gen.csv | 1 + pkg/services/featuremgmt/toggles_gen.go | 4 +++ .../extractFields/extractFields.ts | 10 +++++-- .../extractFields/fieldExtractor.test.ts | 26 +++++++++++++++++++ 9 files changed, 50 insertions(+), 4 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index e4ca209463f..bd049762d54 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -161,6 +161,7 @@ Experimental features might be changed or removed without prior notice. | `alertmanagerRemotePrimary` | Enable Grafana to have a remote Alertmanager instance as the primary Alertmanager. | | `alertmanagerRemoteOnly` | Disable the internal Alertmanager and only use the external one defined. | | `annotationPermissionUpdate` | Separate annotation permissions from dashboard permissions to allow for more granular control. | +| `extractFieldsNameDeduplication` | Make sure extracted field names are unique in the dataframe | ## Development feature toggles diff --git a/packages/grafana-data/src/field/fieldState.ts b/packages/grafana-data/src/field/fieldState.ts index 6f816ca5cf5..55754aa9179 100644 --- a/packages/grafana-data/src/field/fieldState.ts +++ b/packages/grafana-data/src/field/fieldState.ts @@ -159,7 +159,7 @@ export function calculateFieldDisplayName(field: Field, frame?: DataFrame, allFr return displayName; } -function getUniqueFieldName(field: Field, frame?: DataFrame) { +export function getUniqueFieldName(field: Field, frame?: DataFrame) { let dupeCount = 0; let foundSelf = false; diff --git a/packages/grafana-data/src/field/index.ts b/packages/grafana-data/src/field/index.ts index b457d674034..e7b483bdd88 100644 --- a/packages/grafana-data/src/field/index.ts +++ b/packages/grafana-data/src/field/index.ts @@ -14,5 +14,5 @@ export { FieldConfigOptionsRegistry } from './FieldConfigOptionsRegistry'; export { sortThresholds, getActiveThreshold } from './thresholds'; export { applyFieldOverrides, validateFieldConfig, applyRawFieldOverrides, useFieldOverrides } from './fieldOverrides'; export { getFieldDisplayValuesProxy } from './getFieldDisplayValuesProxy'; -export { getFieldDisplayName, getFrameDisplayName, cacheFieldDisplayNames } from './fieldState'; +export { getFieldDisplayName, getFrameDisplayName, cacheFieldDisplayNames, getUniqueFieldName } from './fieldState'; export { getScaleCalculator, getFieldConfigWithMinMax, getMinMaxAndDelta } from './scale'; diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 256983e6fda..09eb6c3f322 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -155,4 +155,5 @@ export interface FeatureToggles { alertmanagerRemotePrimary?: boolean; alertmanagerRemoteOnly?: boolean; annotationPermissionUpdate?: boolean; + extractFieldsNameDeduplication?: boolean; } diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 45267daa23f..157911d1972 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -960,5 +960,12 @@ var ( RequiresDevMode: false, Owner: grafanaAuthnzSquad, }, + { + Name: "extractFieldsNameDeduplication", + Description: "Make sure extracted field names are unique in the dataframe", + Stage: FeatureStageExperimental, + FrontendOnly: true, + Owner: grafanaBiSquad, + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 5b76e116dba..f0b8125952d 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -136,3 +136,4 @@ alertmanagerRemoteSecondary,experimental,@grafana/alerting-squad,false,false,fal alertmanagerRemotePrimary,experimental,@grafana/alerting-squad,false,false,false,false alertmanagerRemoteOnly,experimental,@grafana/alerting-squad,false,false,false,false annotationPermissionUpdate,experimental,@grafana/grafana-authnz-team,false,false,false,false +extractFieldsNameDeduplication,experimental,@grafana/grafana-bi-squad,false,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 10325675080..28f9e832dc8 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -554,4 +554,8 @@ const ( // FlagAnnotationPermissionUpdate // Separate annotation permissions from dashboard permissions to allow for more granular control. FlagAnnotationPermissionUpdate = "annotationPermissionUpdate" + + // FlagExtractFieldsNameDeduplication + // Make sure extracted field names are unique in the dataframe + FlagExtractFieldsNameDeduplication = "extractFieldsNameDeduplication" ) diff --git a/public/app/features/transformers/extractFields/extractFields.ts b/public/app/features/transformers/extractFields/extractFields.ts index ac9d163a9cb..b2f39319960 100644 --- a/public/app/features/transformers/extractFields/extractFields.ts +++ b/public/app/features/transformers/extractFields/extractFields.ts @@ -7,8 +7,10 @@ import { Field, FieldType, getFieldTypeFromValue, + getUniqueFieldName, SynchronousDataTransformerInfo, } from '@grafana/data'; +import { config } from '@grafana/runtime'; import { findField } from 'app/features/dimensions'; import { fieldExtractors } from './fieldExtractors'; @@ -30,7 +32,7 @@ export const extractFieldsTransformer: SynchronousDataTransformerInfo { const buffer = values.get(name); - return { + const field = { name, values: buffer, type: buffer ? getFieldTypeFromValue(buffer.find((v) => v != null)) : FieldType.other, config: {}, } as Field; + if (config.featureToggles.extractFieldsNameDeduplication) { + field.name = getUniqueFieldName(field, frame); + } + return field; }); if (options.keepTime) { diff --git a/public/app/features/transformers/extractFields/fieldExtractor.test.ts b/public/app/features/transformers/extractFields/fieldExtractor.test.ts index 8cb58383877..6757acc3d90 100644 --- a/public/app/features/transformers/extractFields/fieldExtractor.test.ts +++ b/public/app/features/transformers/extractFields/fieldExtractor.test.ts @@ -1,3 +1,7 @@ +import { FieldType, toDataFrame } from '@grafana/data'; +import { config } from '@grafana/runtime'; + +import { addExtractedFields } from './extractFields'; import { fieldExtractors } from './fieldExtractors'; import { FieldExtractorID } from './types'; @@ -111,4 +115,26 @@ describe('Extract fields from text', () => { } `); }); + + it('deduplicates names', async () => { + const frame = toDataFrame({ + fields: [{ name: 'foo', type: FieldType.string, values: ['{"foo":"extracedValue1"}'] }], + }); + config.featureToggles.extractFieldsNameDeduplication = true; + const newFrame = addExtractedFields(frame, { format: FieldExtractorID.JSON, source: 'foo' }); + config.featureToggles.extractFieldsNameDeduplication = false; + expect(newFrame.fields.length).toBe(2); + expect(newFrame.fields[1].name).toBe('foo 1'); + }); + + it('keeps correct names when deduplication is active', async () => { + const frame = toDataFrame({ + fields: [{ name: 'foo', type: FieldType.string, values: ['{"bar":"extracedValue1"}'] }], + }); + config.featureToggles.extractFieldsNameDeduplication = true; + const newFrame = addExtractedFields(frame, { format: FieldExtractorID.JSON, source: 'foo' }); + config.featureToggles.extractFieldsNameDeduplication = false; + expect(newFrame.fields.length).toBe(2); + expect(newFrame.fields[1].name).toBe('bar'); + }); });