From e94b7f45a1b5e550bfa3587b33d6193ef8d2db63 Mon Sep 17 00:00:00 2001
From: Connor Lindsey <cblindsey3@gmail.com>
Date: Wed, 9 Feb 2022 07:25:39 -0700
Subject: [PATCH] Tempo: Fix visual service graph bug by setting upper bound
 for failed arc (#45009)

* Fix visual service graph bug by setting upper bound for failed arc calculation
---
 .../datasource/tempo/graphTransform.test.ts   | 38 +++++++++++++++++++
 .../datasource/tempo/graphTransform.ts        |  4 +-
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/public/app/plugins/datasource/tempo/graphTransform.test.ts b/public/app/plugins/datasource/tempo/graphTransform.test.ts
index 990088eacae..a6dd5b417cf 100644
--- a/public/app/plugins/datasource/tempo/graphTransform.test.ts
+++ b/public/app/plugins/datasource/tempo/graphTransform.test.ts
@@ -88,6 +88,31 @@ describe('mapPromMetricsToServiceMap', () => {
       { name: 'secondaryStat', values: new ArrayVector([1000, 2000]) },
     ]);
   });
+
+  it('handles invalid failed count', () => {
+    // If node.failed > node.total, the stat circle will render in the wrong position
+    // Fixed this by limiting the failed value to the total value
+    const range = {
+      from: dateTime('2000-01-01T00:00:00'),
+      to: dateTime('2000-01-01T00:01:00'),
+    };
+    const { nodes } = mapPromMetricsToServiceMap(
+      [{ data: [totalsPromMetric, secondsPromMetric, invalidFailedPromMetric] }],
+      {
+        ...range,
+        raw: range,
+      }
+    );
+
+    expect(nodes.fields).toMatchObject([
+      { name: 'id', values: new ArrayVector(['db', 'app', 'lb']) },
+      { name: 'title', values: new ArrayVector(['db', 'app', 'lb']) },
+      { name: 'mainStat', values: new ArrayVector([1000, 2000, NaN]) },
+      { name: 'secondaryStat', values: new ArrayVector([0.17, 0.33, NaN]) },
+      { name: 'arc__success', values: new ArrayVector([0, 0, 1]) },
+      { name: 'arc__failed', values: new ArrayVector([1, 1, 0]) },
+    ]);
+  });
 });
 
 const singleSpanResponse = new MutableDataFrame({
@@ -152,3 +177,16 @@ const failedPromMetric = new MutableDataFrame({
     { name: 'Value #traces_service_graph_request_failed_total', values: [2, 15] },
   ],
 });
+
+const invalidFailedPromMetric = new MutableDataFrame({
+  refId: 'traces_service_graph_request_failed_total',
+  fields: [
+    { name: 'Time', values: [1628169788000, 1628169788000] },
+    { name: 'client', values: ['app', 'lb'] },
+    { name: 'instance', values: ['127.0.0.1:12345', '127.0.0.1:12345'] },
+    { name: 'job', values: ['local_scrape', 'local_scrape'] },
+    { name: 'server', values: ['db', 'app'] },
+    { name: 'tempo_config', values: ['default', 'default'] },
+    { name: 'Value #traces_service_graph_request_failed_total', values: [20, 40] },
+  ],
+});
diff --git a/public/app/plugins/datasource/tempo/graphTransform.ts b/public/app/plugins/datasource/tempo/graphTransform.ts
index 9f385f3f443..212fe45fcac 100644
--- a/public/app/plugins/datasource/tempo/graphTransform.ts
+++ b/public/app/plugins/datasource/tempo/graphTransform.ts
@@ -303,8 +303,8 @@ function convertToDataFrames(
       // any requests itself.
       [Fields.mainStat]: node.total ? (node.seconds! / node.total) * 1000 : Number.NaN, // Average response time
       [Fields.secondaryStat]: node.total ? Math.round((node.total / (rangeMs / 1000)) * 100) / 100 : Number.NaN, // Request per second (to 2 decimals)
-      [Fields.arc + 'success']: node.total ? (node.total - (node.failed || 0)) / node.total : 1,
-      [Fields.arc + 'failed']: node.total ? (node.failed || 0) / node.total : 0,
+      [Fields.arc + 'success']: node.total ? (node.total - Math.min(node.failed || 0, node.total)) / node.total : 1,
+      [Fields.arc + 'failed']: node.total ? Math.min(node.failed || 0, node.total) / node.total : 0,
     });
   }
   for (const edgeId of Object.keys(edgesMap)) {