[ML] APM Correlations: Fix chart errors caused by inconsistent histog…

…ram range steps. (#138259) The `AreaSeries` for the latency correlations charts expect the provided timestamp keys for each series to be the same, otherwise there might be errors in how the chart renders the areas. In some cases this could happen because we fetch the data for the areas independently (for example overall latency and failed transactions latency). The fix provided by this PR adds optional `durationMin` and `durationMax` parameters to affected endpoints. This way the `durationMin/Max` returned by the first area request (for example "overall latency") can be passed on to the second request to enforce the same buckets/keys (for example for "failed transaction latency"). (cherry picked from commit 3aabd88)
elastic · Aug 11, 2022 · ecb3a01 · ecb3a01
1 parent 4afe80f
commit ecb3a01
Show file tree

Hide file tree

Showing 13 changed files with 265 additions and 121 deletions.
diff --git a/...gins/apm/public/components/app/correlations/use_failed_transactions_correlations.test.tsx b/...gins/apm/public/components/app/correlations/use_failed_transactions_correlations.test.tsx
@@ -168,18 +168,41 @@ describe('useFailedTransactionsCorrelations', () => {
       );
 
       try {
+        // Each simulated request takes 100ms. After an inital 50ms
+        // we track the internal requests the hook is running and
+        // check the expected progress after these requests.
         jest.advanceTimersByTime(50);
         await waitFor(() => expect(result.current.progress.loaded).toBe(0));
         jest.advanceTimersByTime(100);
-        await waitFor(() => expect(result.current.progress.loaded).toBe(0));
-        jest.advanceTimersByTime(100);
         await waitFor(() => expect(result.current.progress.loaded).toBe(0.05));
 
         expect(result.current.progress).toEqual({
           error: undefined,
           isRunning: true,
           loaded: 0.05,
         });
+        expect(result.current.response).toEqual({
+          ccsWarning: false,
+          fieldStats: undefined,
+          errorHistogram: undefined,
+          failedTransactionsCorrelations: undefined,
+          overallHistogram: [
+            {
+              doc_count: 1234,
+              key: 'the-key',
+            },
+          ],
+          percentileThresholdValue: 1.234,
+        });
+
+        jest.advanceTimersByTime(100);
+        await waitFor(() => expect(result.current.progress.loaded).toBe(0.1));
+
+        expect(result.current.progress).toEqual({
+          error: undefined,
+          isRunning: true,
+          loaded: 0.1,
+        });
         expect(result.current.response).toEqual({
           ccsWarning: false,
           fieldStats: undefined,
@@ -200,23 +223,23 @@ describe('useFailedTransactionsCorrelations', () => {
         });
 
         jest.advanceTimersByTime(100);
-        await waitFor(() => expect(result.current.progress.loaded).toBe(0.1));
+        await waitFor(() => expect(result.current.progress.loaded).toBe(0.15));
 
         // field candidates are an implementation detail and
-        // will not be exposed, it will just set loaded to 0.1.
+        // will not be exposed, it will just set loaded to 0.15.
         expect(result.current.progress).toEqual({
           error: undefined,
           isRunning: true,
-          loaded: 0.1,
+          loaded: 0.15,
         });
 
         jest.advanceTimersByTime(100);
-        await waitFor(() => expect(result.current.progress.loaded).toBe(1));
+        await waitFor(() => expect(result.current.progress.loaded).toBe(0.9));
 
         expect(result.current.progress).toEqual({
           error: undefined,
           isRunning: true,
-          loaded: 1,
+          loaded: 0.9,
         });
 
         expect(result.current.response).toEqual({
@@ -252,9 +275,7 @@ describe('useFailedTransactionsCorrelations', () => {
         });
 
         jest.advanceTimersByTime(100);
-        await waitFor(() =>
-          expect(result.current.response.fieldStats).toBeDefined()
-        );
+        await waitFor(() => expect(result.current.progress.loaded).toBe(1));
 
         expect(result.current.progress).toEqual({
           error: undefined,

diff --git a/...ck/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts b/...ck/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts
@@ -37,9 +37,10 @@ import { useFetchParams } from './use_fetch_params';
 
 // Overall progress is a float from 0 to 1.
 const LOADED_OVERALL_HISTOGRAM = 0.05;
-const LOADED_FIELD_CANDIDATES = LOADED_OVERALL_HISTOGRAM + 0.05;
+const LOADED_ERROR_HISTOGRAM = LOADED_OVERALL_HISTOGRAM + 0.05;
+const LOADED_FIELD_CANDIDATES = LOADED_ERROR_HISTOGRAM + 0.05;
 const LOADED_DONE = 1;
-const PROGRESS_STEP_P_VALUES = 0.9;
+const PROGRESS_STEP_P_VALUES = 0.9 - LOADED_FIELD_CANDIDATES;
 
 export function useFailedTransactionsCorrelations() {
   const fetchParams = useFetchParams();
@@ -85,61 +86,78 @@ export function useFailedTransactionsCorrelations() {
         fallbackResult: undefined,
       };
 
-      const [overallHistogramResponse, errorHistogramRespone] =
-        await Promise.all([
-          // Initial call to fetch the overall distribution for the log-log plot.
-          callApmApi(
-            'POST /internal/apm/latency/overall_distribution/transactions',
-            {
-              signal: abortCtrl.current.signal,
-              params: {
-                body: {
-                  ...fetchParams,
-                  percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD,
-                  chartType:
-                    LatencyDistributionChartType.failedTransactionsCorrelations,
-                },
-              },
-            }
-          ),
-          callApmApi(
-            'POST /internal/apm/latency/overall_distribution/transactions',
-            {
-              signal: abortCtrl.current.signal,
-              params: {
-                body: {
-                  ...fetchParams,
-                  percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD,
-                  termFilters: [
-                    {
-                      fieldName: EVENT_OUTCOME,
-                      fieldValue: EventOutcome.failure,
-                    },
-                  ],
-                  chartType:
-                    LatencyDistributionChartType.failedTransactionsCorrelations,
-                },
-              },
-            }
-          ),
-        ]);
+      // Initial call to fetch the overall distribution for the log-log plot.
+      const overallHistogramResponse = await callApmApi(
+        'POST /internal/apm/latency/overall_distribution/transactions',
+        {
+          signal: abortCtrl.current.signal,
+          params: {
+            body: {
+              ...fetchParams,
+              percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD,
+              chartType:
+                LatencyDistributionChartType.failedTransactionsCorrelations,
+            },
+          },
+        }
+      );
+
+      if (abortCtrl.current.signal.aborted) {
+        return;
+      }
 
-      const { overallHistogram, totalDocCount, percentileThresholdValue } =
-        overallHistogramResponse;
-      const { overallHistogram: errorHistogram } = errorHistogramRespone;
+      const {
+        overallHistogram,
+        totalDocCount,
+        percentileThresholdValue,
+        durationMin,
+        durationMax,
+      } = overallHistogramResponse;
 
-      responseUpdate.errorHistogram = errorHistogram;
       responseUpdate.overallHistogram = overallHistogram;
       responseUpdate.totalDocCount = totalDocCount;
       responseUpdate.percentileThresholdValue = percentileThresholdValue;
 
+      setResponse({
+        ...responseUpdate,
+        loaded: LOADED_OVERALL_HISTOGRAM,
+      });
+      setResponse.flush();
+
+      const errorHistogramResponse = await callApmApi(
+        'POST /internal/apm/latency/overall_distribution/transactions',
+        {
+          signal: abortCtrl.current.signal,
+          params: {
+            body: {
+              ...fetchParams,
+              percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD,
+              termFilters: [
+                {
+                  fieldName: EVENT_OUTCOME,
+                  fieldValue: EventOutcome.failure,
+                },
+              ],
+              durationMin,
+              durationMax,
+              chartType:
+                LatencyDistributionChartType.failedTransactionsCorrelations,
+            },
+          },
+        }
+      );
+
       if (abortCtrl.current.signal.aborted) {
         return;
       }
 
+      const { overallHistogram: errorHistogram } = errorHistogramResponse;
+
+      responseUpdate.errorHistogram = errorHistogram;
+
       setResponse({
         ...responseUpdate,
-        loaded: LOADED_OVERALL_HISTOGRAM,
+        loaded: LOADED_ERROR_HISTOGRAM,
       });
       setResponse.flush();
 
@@ -179,7 +197,12 @@ export function useFailedTransactionsCorrelations() {
           {
             signal: abortCtrl.current.signal,
             params: {
-              body: { ...fetchParams, fieldCandidates: fieldCandidatesChunk },
+              body: {
+                ...fetchParams,
+                fieldCandidates: fieldCandidatesChunk,
+                durationMin,
+                durationMax,
+              },
             },
           }
         );
@@ -284,7 +307,7 @@ export function useFailedTransactionsCorrelations() {
   const progress = useMemo(
     () => ({
       error,
-      loaded,
+      loaded: Math.round(loaded * 100) / 100,
       isRunning,
     }),
     [error, loaded, isRunning]

diff --git a/x-pack/plugins/apm/public/components/app/correlations/use_latency_correlations.ts b/x-pack/plugins/apm/public/components/app/correlations/use_latency_correlations.ts
@@ -86,20 +86,25 @@ export function useLatencyCorrelations() {
       };
 
       // Initial call to fetch the overall distribution for the log-log plot.
-      const { overallHistogram, totalDocCount, percentileThresholdValue } =
-        await callApmApi(
-          'POST /internal/apm/latency/overall_distribution/transactions',
-          {
-            signal: abortCtrl.current.signal,
-            params: {
-              body: {
-                ...fetchParams,
-                percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD,
-                chartType: LatencyDistributionChartType.latencyCorrelations,
-              },
+      const {
+        overallHistogram,
+        totalDocCount,
+        percentileThresholdValue,
+        durationMin,
+        durationMax,
+      } = await callApmApi(
+        'POST /internal/apm/latency/overall_distribution/transactions',
+        {
+          signal: abortCtrl.current.signal,
+          params: {
+            body: {
+              ...fetchParams,
+              percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD,
+              chartType: LatencyDistributionChartType.latencyCorrelations,
             },
-          }
-        );
+          },
+        }
+      );
       responseUpdate.overallHistogram = overallHistogram;
       responseUpdate.totalDocCount = totalDocCount;
       responseUpdate.percentileThresholdValue = percentileThresholdValue;
@@ -192,7 +197,12 @@ export function useLatencyCorrelations() {
           {
             signal: abortCtrl.current.signal,
             params: {
-              body: { ...fetchParams, fieldValuePairs: fieldValuePairChunk },
+              body: {
+                ...fetchParams,
+                durationMin,
+                durationMax,
+                fieldValuePairs: fieldValuePairChunk,
+              },
             },
           }
         );

diff --git a/...omponents/app/transaction_details/distribution/use_transaction_distribution_chart_data.ts b/...omponents/app/transaction_details/distribution/use_transaction_distribution_chart_data.ts
@@ -86,14 +86,18 @@ export const useTransactionDistributionChartData = () => {
           params.serviceName &&
           params.environment &&
           params.start &&
-          params.end
+          params.end &&
+          overallLatencyData.durationMin &&
+          overallLatencyData.durationMax
         ) {
           return callApmApi(
             'POST /internal/apm/latency/overall_distribution/transactions',
             {
               params: {
                 body: {
                   ...params,
+                  durationMin: overallLatencyData.durationMin,
+                  durationMax: overallLatencyData.durationMax,
                   percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD,
                   termFilters: [
                     {
@@ -108,7 +112,7 @@ export const useTransactionDistributionChartData = () => {
           );
         }
       },
-      [params]
+      [params, overallLatencyData.durationMin, overallLatencyData.durationMax]
     );
 
   useEffect(() => {

diff --git a/...ck/plugins/apm/public/components/shared/charts/duration_distribution_chart/index.test.tsx b/...ck/plugins/apm/public/components/shared/charts/duration_distribution_chart/index.test.tsx
@@ -7,11 +7,11 @@
 
 import type { HistogramItem } from '../../../../../common/correlations/types';
 
-import { replaceHistogramDotsWithBars } from '.';
+import { replaceHistogramZerosWithMinimumDomainValue } from '.';
 
 describe('TransactionDistributionChart', () => {
-  describe('replaceHistogramDotsWithBars', () => {
-    it('does the thing', () => {
+  describe('replaceHistogramZerosWithMinimumDomainValue', () => {
+    it('replaces zeroes', () => {
       const mockHistogram = [
         { doc_count: 10 },
         { doc_count: 10 },
@@ -25,7 +25,9 @@ describe('TransactionDistributionChart', () => {
         { doc_count: 10 },
       ] as HistogramItem[];
 
-      expect(replaceHistogramDotsWithBars(mockHistogram)).toEqual([
+      expect(
+        replaceHistogramZerosWithMinimumDomainValue(mockHistogram)
+      ).toEqual([
         { doc_count: 10 },
         { doc_count: 10 },
         { doc_count: 0.0001 },