Transformer: labels to fields should not also merge frames (#38671)

2021-09-01 20:18:50 +02:00
parent 205c672417
commit ae702caec6
4 changed files with 173 additions and 80 deletions
@@ -1,6 +1,6 @@
 import { mockTransformationsRegistry } from '../../utils/tests/mockTransformationsRegistry';
 import { LabelsToFieldsOptions, labelsToFieldsTransformer } from './labelsToFields';
-import { DataTransformerConfig, FieldDTO, FieldType } from '../../types';
+import { DataFrame, DataTransformerConfig, FieldDTO, FieldType } from '../../types';
 import { DataTransformerID } from './ids';
 import { toDataFrame, toDataFrameDTO } from '../../dataframe';
 import { transformDataFrame } from '../transformDataFrame';
@@ -26,21 +26,26 @@ describe('Labels as Columns', () => {

    await expect(transformDataFrame([cfg], [source])).toEmitValuesWith((received) => {
      const data = received[0];
-      const result = toDataFrameDTO(data[0]);
-
-      const expected: FieldDTO[] = [
-        { name: 'time', type: FieldType.time, values: [1000, 2000], config: {} },
-        {
-          name: 'location',
-          type: FieldType.string,
-          values: ['inside', 'inside'],
-          config: {},
-        },
-        { name: 'feelsLike', type: FieldType.string, values: ['ok', 'ok'], config: {} },
-        { name: 'Value', type: FieldType.number, values: [1, 2], config: {} },
-      ];
-
-      expect(result.fields).toEqual(expected);
+      expect(toSimpleObject(data[0])).toMatchInlineSnapshot(`
+        Object {
+          "Value": Array [
+            1,
+            2,
+          ],
+          "feelsLike": Array [
+            "ok",
+            "ok",
+          ],
+          "location": Array [
+            "inside",
+            "inside",
+          ],
+          "time": Array [
+            1000,
+            2000,
+          ],
+        }
+      `);
    });
  });

@@ -73,13 +78,13 @@ describe('Labels as Columns', () => {

      const expected: FieldDTO[] = [
        { name: 'time', type: FieldType.time, values: [1000, 2000], config: {} },
+        { name: 'Request', type: FieldType.number, values: [1, 2], config: {} },
        {
          name: 'location',
          type: FieldType.string,
          values: ['inside', 'inside'],
          config: {},
        },
-        { name: 'Request', type: FieldType.number, values: [1, 2], config: {} },
      ];

      expect(result.fields).toEqual(expected);
@@ -110,15 +115,42 @@ describe('Labels as Columns', () => {

    await expect(transformDataFrame([cfg], [oneValueOneLabelA, oneValueOneLabelB])).toEmitValuesWith((received) => {
      const data = received[0];
-      const result = toDataFrameDTO(data[0]);
+      expect(data.length).toEqual(2);

-      const expected: FieldDTO[] = [
-        { name: 'time', type: FieldType.time, values: [1000, 2000], config: {} },
-        { name: 'location', type: FieldType.string, values: ['inside', 'outside'], config: {} },
-        { name: 'temp', type: FieldType.number, values: [1, -1], config: {} },
-      ];
-
-      expect(result.fields).toEqual(expected);
+      expect(toSimpleObject(data[0])).toMatchInlineSnapshot(`
+        Object {
+          "location": Array [
+            "inside",
+          ],
+          "temp": Array [
+            1,
+          ],
+          "time": Array [
+            1000,
+          ],
+        }
+      `);
+      expect(toSimpleObject(data[1])).toMatchInlineSnapshot(`
+        Object {
+          "location": Array [
+            "outside",
+          ],
+          "temp": Array [
+            -1,
+          ],
+          "time": Array [
+            2000,
+          ],
+        }
+      `);
    });
  });
 });
+
+function toSimpleObject(frame: DataFrame) {
+  const obj: any = {};
+  for (const field of frame.fields) {
+    obj[field.name] = field.values.toArray();
+  }
+  return obj;
+}
@@ -1,9 +1,8 @@
 import { map } from 'rxjs/operators';

-import { DataFrame, DataTransformerInfo, Field, FieldType } from '../../types';
+import { DataFrame, Field, FieldType, SynchronousDataTransformerInfo } from '../../types';
 import { DataTransformerID } from './ids';
 import { ArrayVector } from '../../vector';
-import { mergeTransformer } from './merge';

 export interface LabelsToFieldsOptions {
  /*
@@ -12,65 +11,61 @@ export interface LabelsToFieldsOptions {
  valueLabel?: string;
 }

-export const labelsToFieldsTransformer: DataTransformerInfo<LabelsToFieldsOptions> = {
+export const labelsToFieldsTransformer: SynchronousDataTransformerInfo<LabelsToFieldsOptions> = {
  id: DataTransformerID.labelsToFields,
  name: 'Labels to fields',
  description: 'Extract time series labels to fields (columns)',
  defaultOptions: {},
-  operator: (options) => (source) =>
-    source.pipe(
-      map((data) => {
-        const result: DataFrame[] = [];

-        for (const frame of data) {
-          const newFields: Field[] = [];
+  operator: (options) => (source) => source.pipe(map((data) => labelsToFieldsTransformer.transformer(options)(data))),

-          for (const field of frame.fields) {
-            if (!field.labels) {
-              newFields.push(field);
-              continue;
-            }
+  transformer: (options: LabelsToFieldsOptions) => (data: DataFrame[]) => {
+    const result: DataFrame[] = [];

-            let name = field.name;
+    for (const frame of data) {
+      const newFields: Field[] = [];

-            for (const labelName of Object.keys(field.labels)) {
-              // if we should use this label as the value field name store it and skip adding this as a separate field
-              if (options.valueLabel === labelName) {
-                name = field.labels[labelName];
-                continue;
-              }
-
-              const values = new Array(frame.length).fill(field.labels[labelName]);
-              newFields.push({
-                name: labelName,
-                type: FieldType.string,
-                values: new ArrayVector(values),
-                config: {},
-              });
-            }
-
-            // add the value field but clear out any labels or displayName
-            newFields.push({
-              ...field,
-              name,
-              config: {
-                ...field.config,
-                // we need to clear thes for this transform as these can contain label names that we no longer want
-                displayName: undefined,
-                displayNameFromDS: undefined,
-              },
-              labels: undefined,
-            });
-          }
-
-          result.push({
-            fields: newFields,
-            length: frame.length,
-          });
+      for (const field of frame.fields) {
+        if (!field.labels) {
+          newFields.push(field);
+          continue;
        }

-        return result;
-      }),
-      mergeTransformer.operator({})
-    ),
+        const sansLabels = {
+          ...field,
+          config: {
+            ...field.config,
+            // we need to clear thes for this transform as these can contain label names that we no longer want
+            displayName: undefined,
+            displayNameFromDS: undefined,
+          },
+          labels: undefined,
+        };
+        newFields.push(sansLabels);
+
+        for (const labelName of Object.keys(field.labels)) {
+          // if we should use this label as the value field name store it and skip adding this as a separate field
+          if (options.valueLabel === labelName) {
+            sansLabels.name = field.labels[labelName];
+            continue;
+          }
+
+          const values = new Array(frame.length).fill(field.labels[labelName]);
+          newFields.push({
+            name: labelName,
+            type: FieldType.string,
+            values: new ArrayVector(values),
+            config: {},
+          });
+        }
+      }
+
+      result.push({
+        fields: newFields,
+        length: frame.length,
+      });
+    }
+
+    return result;
+  },
 };
@@ -162,7 +162,7 @@ describe('DashboardModel', () => {
    });

    it('dashboard schema version should be set to latest', () => {
-      expect(model.schemaVersion).toBe(30);
+      expect(model.schemaVersion).toBe(31);
    });

    it('graph thresholds should be migrated', () => {
@@ -1420,6 +1420,38 @@ describe('DashboardModel', () => {
      expect(model.panels[0].panels[0].fieldConfig.defaults).toEqual(undefined);
    });
  });
+
+  describe('labelsToFields should be split into two transformers', () => {
+    let model: DashboardModel;
+
+    beforeEach(() => {
+      model = new DashboardModel({
+        schemaVersion: 29,
+        panels: [
+          {
+            id: 1,
+            type: 'timeseries',
+            transformations: [{ id: 'labelsToFields' }],
+          },
+        ],
+      });
+    });
+
+    it('should create two transormatoins', () => {
+      const xforms = model.panels[0].transformations;
+      expect(xforms).toMatchInlineSnapshot(`
+        Array [
+          Object {
+            "id": "labelsToFields",
+          },
+          Object {
+            "id": "merge",
+            "options": Object {},
+          },
+        ]
+      `);
+    });
+  });
 });

 function createRow(options: any, panelDescriptions: any[]) {
@@ -19,6 +19,7 @@ import {
  ValueMap,
  ValueMapping,
  getActiveThreshold,
+  DataTransformerConfig,
 } from '@grafana/data';
 // Constants
 import {
@@ -36,6 +37,8 @@ import { config } from 'app/core/config';
 import { plugin as statPanelPlugin } from 'app/plugins/panel/stat/module';
 import { plugin as gaugePanelPlugin } from 'app/plugins/panel/gauge/module';
 import { getStandardFieldConfigs, getStandardOptionEditors } from '@grafana/ui';
+import { labelsToFieldsTransformer } from '../../../../../packages/grafana-data/src/transformations/transformers/labelsToFields';
+import { mergeTransformer } from '../../../../../packages/grafana-data/src/transformations/transformers/merge';

 standardEditorsRegistry.setInit(getStandardOptionEditors);
 standardFieldConfigEditorRegistry.setInit(getStandardFieldConfigs);
@@ -52,7 +55,7 @@ export class DashboardMigrator {
    let i, j, k, n;
    const oldVersion = this.dashboard.schemaVersion;
    const panelUpgrades: PanelSchemeUpgradeHandler[] = [];
-    this.dashboard.schemaVersion = 30;
+    this.dashboard.schemaVersion = 31;

    if (oldVersion === this.dashboard.schemaVersion) {
      return;
@@ -660,6 +663,22 @@ export class DashboardMigrator {
      panelUpgrades.push(migrateTooltipOptions);
    }

+    if (oldVersion < 31) {
+      panelUpgrades.push((panel: PanelModel) => {
+        if (panel.transformations) {
+          for (const t of panel.transformations) {
+            if (t.id === labelsToFieldsTransformer.id) {
+              return appedTransformerAfter(panel, labelsToFieldsTransformer.id, {
+                id: mergeTransformer.id,
+                options: {},
+              });
+            }
+          }
+        }
+        return panel;
+      });
+    }
+
    if (panelUpgrades.length === 0) {
      return;
    }
@@ -949,6 +968,21 @@ function migrateSinglestat(panel: PanelModel) {
  return panel;
 }

+// mutates transformations appending a new transformer after the existing one
+function appedTransformerAfter(panel: PanelModel, id: string, cfg: DataTransformerConfig) {
+  if (panel.transformations) {
+    const transformations: DataTransformerConfig[] = [];
+    for (const t of panel.transformations) {
+      transformations.push(t);
+      if (t.id === id) {
+        transformations.push({ ...cfg });
+      }
+    }
+    panel.transformations = transformations;
+  }
+  return panel;
+}
+
 function upgradeValueMappingsForPanel(panel: PanelModel) {
  const fieldConfig = panel.fieldConfig;
  if (!fieldConfig) {