From e714c9303e73024dbf4689aa6f44d89654dad248 Mon Sep 17 00:00:00 2001
From: Kyle Cunningham <codeincarnate@users.noreply.github.com>
Date: Thu, 2 Nov 2023 10:25:48 -0500
Subject: [PATCH] Timeseries to table transformation: Update Output Changes
 (#77415)

* Break out labels into separate fields

* More Updates

* Minor test changes

* Use 'A' for transformed refId

* Make sure tests pass

* Add additional test

* Prettier

* Remove dead comment

* Update time field selection options

* remove console.log

---------

Co-authored-by: Victor Marin <victor.marin@grafana.com>
---
 .../TimeSeriesTableTransformEditor.tsx        |  68 ++++----
 .../timeSeriesTableTransformer.test.ts        |  85 ++++++----
 .../timeSeriesTableTransformer.ts             | 148 ++++++++++--------
 3 files changed, 171 insertions(+), 130 deletions(-)

diff --git a/public/app/features/transformers/timeSeriesTable/TimeSeriesTableTransformEditor.tsx b/public/app/features/transformers/timeSeriesTable/TimeSeriesTableTransformEditor.tsx
index 13eefd29ef4..6e996eed2a9 100644
--- a/public/app/features/transformers/timeSeriesTable/TimeSeriesTableTransformEditor.tsx
+++ b/public/app/features/transformers/timeSeriesTable/TimeSeriesTableTransformEditor.tsx
@@ -7,9 +7,10 @@ import {
   ReducerID,
   isReducerID,
   SelectableValue,
-  getFieldDisplayName,
+  Field,
+  FieldType,
 } from '@grafana/data';
-import { InlineFieldRow, InlineField, StatsPicker, InlineSwitch, Select } from '@grafana/ui';
+import { InlineFieldRow, InlineField, StatsPicker, Select, InlineLabel } from '@grafana/ui';
 
 import {
   timeSeriesTableTransformer,
@@ -22,19 +23,8 @@ export function TimeSeriesTableTransformEditor({
   options,
   onChange,
 }: TransformerUIProps<TimeSeriesTableTransformerOptions>) {
-  const timeFields: Array<SelectableValue<string>> = [];
   const refIdMap = getRefData(input);
 
-  // Retrieve time fields
-  for (const frame of input) {
-    for (const field of frame.fields) {
-      if (field.type === 'time') {
-        const name = getFieldDisplayName(field, frame, input);
-        timeFields.push({ label: name, value: name });
-      }
-    }
-  }
-
   const onSelectTimefield = useCallback(
     (refId: string, value: SelectableValue<string>) => {
       const val = value?.value !== undefined ? value.value : '';
@@ -65,32 +55,45 @@ export function TimeSeriesTableTransformEditor({
     [onChange, options]
   );
 
-  const onMergeSeriesToggle = useCallback(
-    (refId: string) => {
-      const mergeSeries = options[refId]?.mergeSeries !== undefined ? !options[refId].mergeSeries : false;
-      onChange({
-        ...options,
-        [refId]: {
-          ...options[refId],
-          mergeSeries,
-        },
-      });
-    },
-    [onChange, options]
-  );
-
   let configRows = [];
   for (const refId of Object.keys(refIdMap)) {
+    // Get time fields for the current refId
+    const timeFields: Record<string, Field<FieldType.time>> = {};
+    const timeValues: Array<SelectableValue<string>> = [];
+
+    // Get a map of time fields, we map
+    // by field name and assume that time fields
+    // in the same query with the same name
+    // are the same
+    for (const frame of input) {
+      if (frame.refId === refId) {
+        for (const field of frame.fields) {
+          if (field.type === 'time') {
+            timeFields[field.name] = field;
+          }
+        }
+      }
+    }
+
+    for (const timeField of Object.values(timeFields)) {
+      const { name } = timeField;
+      timeValues.push({ label: name, value: name });
+    }
+
     configRows.push(
       <InlineFieldRow key={refId}>
+        <InlineField>
+          <InlineLabel>{`Trend #${refId}`}</InlineLabel>
+        </InlineField>
         <InlineField
           label="Time field"
           tooltip="The time field that will be used for the time series. If not selected the first found will be used."
         >
           <Select
             onChange={onSelectTimefield.bind(null, refId)}
-            options={timeFields}
+            options={timeValues}
             value={options[refId]?.timeField}
+            isClearable={true}
           />
         </InlineField>
         <InlineField label="Stat" tooltip="The statistic that should be calculated for this time series.">
@@ -100,15 +103,6 @@ export function TimeSeriesTableTransformEditor({
             filterOptions={(ext) => ext.id !== ReducerID.allValues && ext.id !== ReducerID.uniqueValues}
           />
         </InlineField>
-        <InlineField
-          label="Merge series"
-          tooltip="If selected, multiple series from a single datasource will be merged into one series."
-        >
-          <InlineSwitch
-            value={options[refId]?.mergeSeries !== undefined ? options[refId]?.mergeSeries : true}
-            onChange={onMergeSeriesToggle.bind(null, refId)}
-          />
-        </InlineField>
       </InlineFieldRow>
     );
   }
diff --git a/public/app/features/transformers/timeSeriesTable/timeSeriesTableTransformer.test.ts b/public/app/features/transformers/timeSeriesTable/timeSeriesTableTransformer.test.ts
index f379785c767..e7c66a676c9 100644
--- a/public/app/features/transformers/timeSeriesTable/timeSeriesTableTransformer.test.ts
+++ b/public/app/features/transformers/timeSeriesTable/timeSeriesTableTransformer.test.ts
@@ -15,12 +15,9 @@ describe('timeSeriesTableTransformer', () => {
     const result = results[0];
     expect(result.refId).toBe('A');
     expect(result.fields).toHaveLength(3);
-    expect(result.fields[0].values).toEqual([
-      'Value : instance=A : pod=B',
-      'Value : instance=A : pod=C',
-      'Value : instance=A : pod=D',
-    ]);
-    assertDataFrameField(result.fields[1], series);
+    expect(result.fields[0].values).toEqual(['A', 'A', 'A']);
+    expect(result.fields[1].values).toEqual(['B', 'C', 'D']);
+    assertDataFrameField(result.fields[2], series);
   });
 
   it('Will pass through non time series frames', () => {
@@ -34,9 +31,11 @@ describe('timeSeriesTableTransformer', () => {
     const results = timeSeriesToTableTransform({}, series);
     expect(results).toHaveLength(3);
     expect(results[0]).toEqual(series[0]);
-    expect(results[1].refId).toBe('A');
-    expect(results[1].fields).toHaveLength(3);
-    expect(results[2]).toEqual(series[3]);
+    expect(results[2].refId).toBe('A');
+    expect(results[2].fields).toHaveLength(3);
+    expect(results[2].fields[0].values).toEqual(['A', 'A']);
+    expect(results[2].fields[1].values).toEqual(['B', 'C']);
+    expect(results[1]).toEqual(series[3]);
   });
 
   it('Will group by refId', () => {
@@ -49,26 +48,19 @@ describe('timeSeriesTableTransformer', () => {
     ];
 
     const results = timeSeriesToTableTransform({}, series);
+
     expect(results).toHaveLength(2);
     expect(results[0].refId).toBe('A');
     expect(results[0].fields).toHaveLength(3);
-    expect(results[0].fields[0].values).toEqual([
-      'Value : instance=A : pod=B',
-      'Value : instance=A : pod=C',
-      'Value : instance=A : pod=D',
-    ]);
-    assertDataFrameField(results[0].fields[1], series.slice(0, 3));
+    expect(results[0].fields[0].values).toEqual(['A', 'A', 'A']);
+    expect(results[0].fields[1].values).toEqual(['B', 'C', 'D']);
+    assertDataFrameField(results[0].fields[2], series.slice(0, 3));
     expect(results[1].refId).toBe('B');
-    expect(results[1].fields).toHaveLength(3);
-    expect(results[1].fields[0].values).toEqual([
-      'Value : instance=B : pod=F : cluster=A',
-      'Value : instance=B : pod=G : cluster=B',
-    ]);
-    expect(results[1].fields[0].values).toEqual([
-      'Value : instance=B : pod=F : cluster=A',
-      'Value : instance=B : pod=G : cluster=B',
-    ]);
-    assertDataFrameField(results[1].fields[1], series.slice(3, 5));
+    expect(results[1].fields).toHaveLength(4);
+    expect(results[1].fields[0].values).toEqual(['B', 'B']);
+    expect(results[1].fields[1].values).toEqual(['F', 'G']);
+    expect(results[1].fields[2].values).toEqual(['A', 'B']);
+    assertDataFrameField(results[1].fields[3], series.slice(3, 5));
   });
 
   it('Will include last value by deault', () => {
@@ -78,8 +70,8 @@ describe('timeSeriesTableTransformer', () => {
     ];
 
     const results = timeSeriesToTableTransform({}, series);
-    expect(results[0].fields[1].values[0].fields[1].values[2]).toEqual(3);
-    expect(results[0].fields[1].values[1].fields[1].values[2]).toEqual(5);
+    expect(results[0].fields[2].values[0].value).toEqual(3);
+    expect(results[0].fields[2].values[1].value).toEqual(5);
   });
 
   it('Will calculate average value if configured', () => {
@@ -88,12 +80,45 @@ describe('timeSeriesTableTransformer', () => {
       getTimeSeries('B', { instance: 'A', pod: 'C' }, [3, 4, 5]),
     ];
 
-    const results = timeSeriesToTableTransform({ B: { stat: ReducerID.mean } }, series);
-    expect(results[0].fields[2].values[0]).toEqual(3);
-    expect(results[1].fields[2].values[0]).toEqual(4);
+    const results = timeSeriesToTableTransform(
+      {
+        B: {
+          stat: ReducerID.mean,
+        },
+      },
+      series
+    );
+
+    expect(results[0].fields[2].values[0].value).toEqual(3);
+    expect(results[1].fields[2].values[0].value).toEqual(4);
   });
 });
 
+it('Will transform multiple data series with the same label', () => {
+  const series = [
+    getTimeSeries('A', { instance: 'A', pod: 'B' }, [4, 2, 3]),
+    getTimeSeries('B', { instance: 'A', pod: 'B' }, [3, 4, 5]),
+    getTimeSeries('C', { instance: 'A', pod: 'B' }, [3, 4, 5]),
+  ];
+
+  const results = timeSeriesToTableTransform({}, series);
+
+  // Check series A
+  expect(results[0].fields).toHaveLength(3);
+  expect(results[0].fields[0].values[0]).toBe('A');
+  expect(results[0].fields[1].values[0]).toBe('B');
+
+  // Check series B
+  expect(results[1].fields).toHaveLength(3);
+  expect(results[1].fields[0].values[0]).toBe('A');
+  expect(results[1].fields[1].values[0]).toBe('B');
+
+  // Check series C
+  expect(results[2].fields).toHaveLength(3);
+  expect(results[2].fields[0].values[0]).toBe('A');
+  expect(results[2].fields[1].values[0]).toBe('B');
+});
+
 function assertFieldsEqual(field1: Field, field2: Field) {
   expect(field1.type).toEqual(field2.type);
   expect(field1.name).toEqual(field2.name);
diff --git a/public/app/features/transformers/timeSeriesTable/timeSeriesTableTransformer.ts b/public/app/features/transformers/timeSeriesTable/timeSeriesTableTransformer.ts
index 1e4bc51aa6f..dfe99397e30 100644
--- a/public/app/features/transformers/timeSeriesTable/timeSeriesTableTransformer.ts
+++ b/public/app/features/transformers/timeSeriesTable/timeSeriesTableTransformer.ts
@@ -4,6 +4,7 @@ import {
   DataFrame,
   DataTransformerID,
   DataTransformerInfo,
+  DataFrameWithValue,
   Field,
   FieldType,
   MutableDataFrame,
@@ -13,8 +14,6 @@ import {
   TransformationApplicabilityLevels,
 } from '@grafana/data';
 
-const MERGE_DEFAULT = true;
-
 /**
  * Maps a refId to a Field which can contain
  * different types of data. In our case we
@@ -24,6 +23,27 @@ interface RefFieldMap<T> {
   [index: string]: Field<T>;
 }
 
+/**
+ * A map of RefIds to labels where each
+ * label maps to a field of the given
+ * type. It's technically possible
+ * to use the above type to achieve
+ * this in combination with another mapping
+ * but the RefIds are on the outer map
+ * in this case, so we use a different type
+ * to avoid future issues.
+ *
+ *  RefId: {
+ *     label1: Field<T>
+ *     label2: Field<T>
+ *  }
+ */
+interface RefLabelFieldMap<T> {
+  [index: string]: {
+    [index: string]: Field<T>;
+  };
+}
+
 /**
  * For options we have a set of options
  * for each refId. So we map the refId
@@ -56,8 +76,8 @@ interface RefCount {
  */
 export interface RefIdTransformerOptions {
   stat?: ReducerID;
-  mergeSeries?: boolean;
   timeField?: string;
+  inlineStat?: boolean;
 }
 
 export const timeSeriesTableTransformer: DataTransformerInfo<TimeSeriesTableTransformerOptions> = {
@@ -101,9 +121,8 @@ export const timeSeriesTableTransformer: DataTransformerInfo<TimeSeriesTableTran
  */
 export function timeSeriesToTableTransform(options: TimeSeriesTableTransformerOptions, data: DataFrame[]): DataFrame[] {
   // Initialize maps for labels, sparklines, and reduced values
-  const refId2LabelField: RefFieldMap<string> = {};
-  const refId2FrameField: RefFieldMap<DataFrame> = {};
-  const refId2ValueField: RefFieldMap<number> = {};
+  const refId2trends: RefLabelFieldMap<DataFrameWithValue> = {};
+  const refId2labelz: RefLabelFieldMap<string> = {};
 
   // Accumulator for our final value
   // which we'll return
@@ -117,11 +136,12 @@ export function timeSeriesToTableTransform(options: TimeSeriesTableTransformerOp
   // series we initialize fields here
   // so we end up with one
   for (const refId of Object.keys(refIdMap)) {
-    const merge = options[refId]?.mergeSeries !== undefined ? options[refId].mergeSeries : MERGE_DEFAULT;
-
     // Get the frames with the current refId
     const framesForRef = data.filter((frame) => frame.refId === refId);
 
+    // Intialize object for this refId
+    refId2trends[refId] = {};
+
     for (let i = 0; i < framesForRef.length; i++) {
       const frame = framesForRef[i];
 
@@ -132,12 +152,6 @@ export function timeSeriesToTableTransform(options: TimeSeriesTableTransformerOp
         continue;
       }
 
-      // If we're not dealing with a frame
-      // of the current refId skip it
-      if (frame.refId !== refId) {
-        continue;
-      }
-
       // Retrieve the time field that's been configured
       // If one isn't configured then use the first found
       let timeField = null;
@@ -147,14 +161,6 @@ export function timeSeriesToTableTransform(options: TimeSeriesTableTransformerOp
         timeField = frame.fields.find((field) => field.type === FieldType.time);
       }
 
-      // Initialize fields for this frame
-      // if we're not merging them
-      if ((merge && i === 0) || !merge) {
-        refId2LabelField[refId] = newField('Label', FieldType.string);
-        refId2FrameField[refId] = newField('Trend', FieldType.frame);
-        refId2ValueField[refId] = newField('Trend Value', FieldType.number);
-      }
-
       for (const field of frame.fields) {
         // Skip non-number based fields
         // i.e. we skip time, strings, etc.
@@ -162,35 +168,11 @@ export function timeSeriesToTableTransform(options: TimeSeriesTableTransformerOp
           continue;
         }
 
-        // Create the value for the label field
-        let labelParts: string[] = [];
-
-        // Add the refId to the label if we have
-        // more than one
-        if (refIdMap.length > 1) {
-          labelParts.push(refId);
-        }
-
-        // Add the name of the field
-        labelParts.push(field.name);
-
-        // If there is any labeled data add it here
-        if (field.labels !== undefined) {
-          for (const [labelKey, labelValue] of Object.entries(field.labels)) {
-            labelParts.push(`${labelKey}=${labelValue}`);
-          }
-        }
-
-        // Add the label parts to the label field
-        const label = labelParts.join(' : ');
-        refId2LabelField[refId].values.push(label);
-
         // Calculate the reduction of the current field
         // and push the frame with reduction
         // into the the appropriate field
         const reducerId = options[refId]?.stat ?? ReducerID.lastNotNull;
         const value = reduceField({ field, reducers: [reducerId] })[reducerId] || null;
-        refId2ValueField[refId].values.push(value);
 
         // Push the appropriate time and value frame
         // to the trend frame for the sparkline
@@ -198,29 +180,69 @@ export function timeSeriesToTableTransform(options: TimeSeriesTableTransformerOp
         if (timeField !== undefined) {
           sparklineFrame.addField(timeField);
           sparklineFrame.addField(field);
+
+          if (refId2trends[refId][`Trend #${refId}`] === undefined) {
+            refId2trends[refId][`Trend #${refId}`] = newField(`Trend #${refId}`, FieldType.frame);
+          }
+
+          refId2trends[refId][`Trend #${refId}`].values.push({
+            ...sparklineFrame,
+            value,
+            length: field.values.length,
+          });
         }
-        refId2FrameField[refId].values.push(sparklineFrame);
-      }
 
-      // If we're merging then we only add at the very
-      // end that is when i has reached the end of the data
-      if (merge && framesForRef.length - 1 !== i) {
-        continue;
+        // If there are labels add them to the appropriate fields
+        // Because we iterate each frame
+        if (field.labels !== undefined) {
+          for (const [labelKey, labelValue] of Object.entries(field.labels)) {
+            if (refId2labelz[refId] === undefined) {
+              refId2labelz[refId] = {};
+            }
+
+            if (refId2labelz[refId][labelKey] === undefined) {
+              refId2labelz[refId][labelKey] = newField(labelKey, FieldType.string);
+            }
+
+            refId2labelz[refId][labelKey].values.push(labelValue);
+          }
+        }
       }
+    }
+  }
 
-      // Finally, allocate the new frame
-      const table = new MutableDataFrame();
+  for (const refId of Object.keys(refIdMap)) {
+    const label2fields: RefFieldMap<string> = {};
+
+    // Allocate a new frame
+    const table = new MutableDataFrame();
+    table.refId = refId;
+
+    // Rather than having a label fields for each refId
+    // we combine them into a single set of labels
+    // taking the first value available
+    const labels = refId2labelz[refId];
+    if (labels !== undefined) {
+      for (const [labelName, labelField] of Object.entries(labels)) {
+        if (label2fields[labelName] === undefined) {
+          label2fields[labelName] = labelField;
+        }
+      }
+    }
 
-      // Set the refId
-      table.refId = refId;
+    // Add label fields to the the resulting frame
+    for (const label of Object.values(label2fields)) {
+      table.addField(label);
+    }
 
-      // Add the label, sparkline, and value fields
-      // into the new frame
-      table.addField(refId2LabelField[refId]);
-      table.addField(refId2FrameField[refId]);
-      table.addField(refId2ValueField[refId]);
+    // Add trend fields to frame
+    const refTrends = refId2trends[refId];
+    for (const trend of Object.values(refTrends)) {
+      table.addField(trend);
+    }
 
-      // Finaly push to the result
+    // Finaly push to the result
+    if (table.fields.length > 0) {
       result.push(table);
     }
   }
@@ -250,7 +272,7 @@ function newField(label: string, type: FieldType) {
 /**
  * Get the refIds contained in an array of Data frames.
  * @param data
- * @returns
+ * @returns A RefCount object
  */
 export function getRefData(data: DataFrame[]) {
   let refMap: RefCount = {};