From a86d7a5a901ad700ae8429b6cd71fb6357ed2f65 Mon Sep 17 00:00:00 2001
From: Cat Perry <000.perry@gmail.com>
Date: Tue, 1 Feb 2022 17:39:09 -0500
Subject: [PATCH] Tempo: Show error if transformTrace() is passed bad data to
 parse to JSON (#44684)

* Surface errors if transformTrace() is passed bad data to parse
---
 .../tempo/resultTransformer.test.ts           | 121 +++++++++++++++---
 .../datasource/tempo/resultTransformer.ts     |   7 +-
 .../plugins/datasource/tempo/testResponse.ts  |  29 +++++
 3 files changed, 138 insertions(+), 19 deletions(-)

diff --git a/public/app/plugins/datasource/tempo/resultTransformer.test.ts b/public/app/plugins/datasource/tempo/resultTransformer.test.ts
index 175efdcfabb..c5361458c89 100644
--- a/public/app/plugins/datasource/tempo/resultTransformer.test.ts
+++ b/public/app/plugins/datasource/tempo/resultTransformer.test.ts
@@ -1,14 +1,45 @@
-import { FieldType, MutableDataFrame, PluginType, DataSourceInstanceSettings, dateTime } from '@grafana/data';
+import {
+  ArrayVector,
+  FieldType,
+  MutableDataFrame,
+  PluginType,
+  DataSourceInstanceSettings,
+  dateTime,
+} from '@grafana/data';
 import {
   SearchResponse,
   createTableFrame,
   transformToOTLP,
   transformFromOTLP,
+  transformTrace,
   createTableFrameFromSearch,
 } from './resultTransformer';
-import { otlpDataFrameToResponse, otlpDataFrameFromResponse, otlpResponse, tempoSearchResponse } from './testResponse';
+import {
+  badOTLPResponse,
+  otlpDataFrameToResponse,
+  otlpDataFrameFromResponse,
+  otlpResponse,
+  tempoSearchResponse,
+} from './testResponse';
 import { collectorTypes } from '@opentelemetry/exporter-collector';
 
+const defaultSettings: DataSourceInstanceSettings = {
+  id: 0,
+  uid: '0',
+  type: 'tracing',
+  name: 'tempo',
+  access: 'proxy',
+  meta: {
+    id: 'tempo',
+    name: 'tempo',
+    type: PluginType.datasource,
+    info: {} as any,
+    module: '',
+    baseUrl: '',
+  },
+  jsonData: {},
+};
+
 describe('transformTraceList()', () => {
   const lokiDataFrame = new MutableDataFrame({
     fields: [
@@ -84,19 +115,73 @@ describe('createTableFrameFromSearch()', () => {
   });
 });
 
-const defaultSettings: DataSourceInstanceSettings = {
-  id: 0,
-  uid: '0',
-  type: 'tracing',
-  name: 'tempo',
-  access: 'proxy',
-  meta: {
-    id: 'tempo',
-    name: 'tempo',
-    type: PluginType.datasource,
-    info: {} as any,
-    module: '',
-    baseUrl: '',
-  },
-  jsonData: {},
-};
+describe('transformFromOTLP()', () => {
+  // Mock the console error so that running the test suite doesnt throw the error
+  const origError = console.error;
+  const consoleErrorMock = jest.fn();
+  afterEach(() => (console.error = origError));
+  beforeEach(() => (console.error = consoleErrorMock));
+
+  test('if passed bad data, will surface an error', () => {
+    const res = transformFromOTLP(
+      (badOTLPResponse.batches as unknown) as collectorTypes.opentelemetryProto.trace.v1.ResourceSpans[],
+      false
+    );
+
+    expect(res.data[0]).toBeFalsy();
+    expect(res.error?.message).toBeTruthy();
+    // if it does have resources, no error will be thrown
+    expect({
+      ...res.data[0],
+      resources: {
+        attributes: [
+          { key: 'service.name', value: { stringValue: 'db' } },
+          { key: 'job', value: { stringValue: 'tns/db' } },
+          { key: 'opencensus.exporterversion', value: { stringValue: 'Jaeger-Go-2.22.1' } },
+          { key: 'host.name', value: { stringValue: '63d16772b4a2' } },
+          { key: 'ip', value: { stringValue: '0.0.0.0' } },
+          { key: 'client-uuid', value: { stringValue: '39fb01637a579639' } },
+        ],
+      },
+    }).not.toBeFalsy();
+  });
+});
+
+describe('transformTrace()', () => {
+  // Mock the console error so that running the test suite doesnt throw the error
+  const origError = console.error;
+  const consoleErrorMock = jest.fn();
+  afterEach(() => (console.error = origError));
+  beforeEach(() => (console.error = consoleErrorMock));
+
+  const badFrame = new MutableDataFrame({
+    fields: [
+      {
+        name: 'serviceTags',
+        values: new ArrayVector([undefined]),
+      },
+    ],
+  });
+
+  const goodFrame = new MutableDataFrame({
+    fields: [
+      {
+        name: 'serviceTags',
+        values: new ArrayVector(),
+      },
+    ],
+  });
+
+  test('if passed bad data, will surface an error', () => {
+    const response = transformTrace({ data: [badFrame] }, false);
+    expect(response.data[0]).toBeFalsy();
+    expect(response.error?.message).toBeTruthy();
+  });
+
+  test('if passed good data, will parse successfully', () => {
+    const response2 = transformTrace({ data: [goodFrame] }, false);
+    expect(response2.data[0]).toBeTruthy();
+    expect(response2.data[0]).toMatchObject(goodFrame);
+    expect(response2.error).toBeFalsy();
+  });
+});
diff --git a/public/app/plugins/datasource/tempo/resultTransformer.ts b/public/app/plugins/datasource/tempo/resultTransformer.ts
index cba5d12db45..ee53d36199c 100644
--- a/public/app/plugins/datasource/tempo/resultTransformer.ts
+++ b/public/app/plugins/datasource/tempo/resultTransformer.ts
@@ -491,7 +491,12 @@ export function transformTrace(response: DataQueryResponse, nodeGraph = false):
     return emptyDataQueryResponse;
   }
 
-  parseJsonFields(frame);
+  try {
+    parseJsonFields(frame);
+  } catch (error) {
+    console.error(error);
+    return { error: { message: 'Unable to parse trace response: ' + error }, data: [] };
+  }
 
   let data = [...response.data];
   if (nodeGraph) {
diff --git a/public/app/plugins/datasource/tempo/testResponse.ts b/public/app/plugins/datasource/tempo/testResponse.ts
index 97f7242b53b..22e91bac225 100644
--- a/public/app/plugins/datasource/tempo/testResponse.ts
+++ b/public/app/plugins/datasource/tempo/testResponse.ts
@@ -2223,3 +2223,32 @@ export const tempoSearchResponse = {
     inspectedBytes: '83720',
   },
 };
+
+export const badOTLPResponse = {
+  batches: [
+    {
+      resource: {},
+      instrumentationLibrarySpans: [
+        {
+          spans: [
+            {
+              traceId: 'AAAAAAAAAABguiq7RPE+rg==',
+              spanId: 'cmteMBAvwNA=',
+              parentSpanId: 'OY8PIaPbma4=',
+              name: 'HTTP GET - root',
+              kind: 'SPAN_KIND_CLIENT',
+              startTimeUnixNano: 1627471657255809000,
+              endTimeUnixNano: 1627471657256268000,
+              attributes: [
+                { key: 'http.status_code', value: { intValue: 200 } },
+                { key: 'http.method', value: { stringValue: 'GET' } },
+                { key: 'http.url', value: { stringValue: '/' } },
+                { key: 'component', value: { stringValue: 'net/http' } },
+              ],
+            },
+          ],
+        },
+      ],
+    },
+  ],
+};