[Synthetics] Improve loading state for metric items (#192930)

## Summary There are some cases where the latest addition to the overview grid causes metric items not to load for very small numbers of monitors. This PR aims to fix that issue, and to add a more robust loading state to the individual items to make it easier to track when there is a request in-flight. Additionally, it adds a type guard to the server code and an io-ts codec for verifying the safety of the code we use to generate our server responses. ## Testing Ensure that you see metric items load their duration graphs for small numbers (3 or less) of monitors, and that scrolling etc., continues working for larger ones. --------- Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 7a7888b)
elastic · Sep 17, 2024 · b47b3dc · b47b3dc
1 parent c9c3e89
commit b47b3dc
Show file tree

Hide file tree

Showing 11 changed files with 315 additions and 100 deletions.
diff --git a/x-pack/plugins/observability_solution/synthetics/common/types/overview.ts b/x-pack/plugins/observability_solution/synthetics/common/types/overview.ts
@@ -22,11 +22,16 @@ export interface OverviewTrend {
   locationId: string;
   data: TrendDatum[];
   count: number;
-  min: number;
-  max: number;
-  avg: number;
-  sum: number;
-  median: number;
+  min: number | null;
+  max: number | null;
+  avg: number | null;
+  sum: number | null;
+  median: number | null;
 }
 
-export type TrendTable = Record<string, OverviewTrend | null>;
+export type TrendTable = Record<string, OverviewTrend | null | 'loading'>;
+
+export interface GetTrendPayload {
+  trendStats: TrendTable;
+  batch: TrendRequest[];
+}
diff --git a/...thetics/public/apps/synthetics/components/monitors_page/overview/overview/metric_item.tsx b/...thetics/public/apps/synthetics/components/monitors_page/overview/overview/metric_item.tsx
@@ -141,26 +141,27 @@ export const MetricItem = ({
                 {
                   title: monitor.name,
                   subtitle: locationName,
-                  value: trendData?.median ?? 0,
+                  value: trendData !== 'loading' ? trendData?.median ?? 0 : 0,
                   trendShape: MetricTrendShape.Area,
-                  trend: trendData?.data ?? [],
-                  extra: trendData ? (
-                    <MetricItemExtra
-                      stats={{
-                        medianDuration: trendData.median,
-                        minDuration: trendData.min,
-                        maxDuration: trendData.max,
-                        avgDuration: trendData.avg,
-                      }}
-                    />
-                  ) : (
-                    <div>
-                      <FormattedMessage
-                        defaultMessage="Loading metrics"
-                        id="xpack.synthetics.overview.metricItem.loadingMessage"
+                  trend: trendData !== 'loading' && !!trendData?.data ? trendData.data : [],
+                  extra:
+                    trendData !== 'loading' && !!trendData ? (
+                      <MetricItemExtra
+                        stats={{
+                          medianDuration: trendData.median,
+                          minDuration: trendData.min,
+                          maxDuration: trendData.max,
+                          avgDuration: trendData.avg,
+                        }}
                       />
-                    </div>
-                  ),
+                    ) : trendData === 'loading' ? (
+                      <div>
+                        <FormattedMessage
+                          defaultMessage="Loading metrics"
+                          id="xpack.synthetics.overview.metricItem.loadingMessage"
+                        />
+                      </div>
+                    ) : undefined,
                   valueFormatter: (d: number) => formatDuration(d),
                   color: getColor(theme, monitor.isEnabled, status),
                   body: <MetricItemBody monitor={monitor} />,

diff --git a/...thetics/components/monitors_page/overview/overview/metric_item/metric_item_extra.test.tsx b/...thetics/components/monitors_page/overview/overview/metric_item/metric_item_extra.test.tsx
@@ -0,0 +1,40 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import React from 'react';
+import { render } from '../../../../../utils/testing/rtl_helpers';
+import { MetricItemExtra } from './metric_item_extra';
+import { fireEvent, waitFor } from '@testing-library/dom';
+
+describe('<MetricItemExtra />', () => {
+  it('renders the tooltip when there is content', async () => {
+    const { getByText } = render(
+      <MetricItemExtra
+        stats={{ medianDuration: 10, avgDuration: 10, minDuration: 5, maxDuration: 15 }}
+      />
+    );
+    expect(getByText('Duration')).toBeInTheDocument();
+    fireEvent.mouseOver(getByText('Info'));
+    await waitFor(() => expect(getByText('Median duration of last 50 checks')).toBeInTheDocument());
+  });
+
+  it('renders the empty tooltip when there is no content', async () => {
+    const { getByText } = render(
+      <MetricItemExtra
+        stats={{
+          medianDuration: null,
+          avgDuration: null,
+          minDuration: null,
+          maxDuration: null,
+        }}
+      />
+    );
+    expect(getByText('Duration')).toBeInTheDocument();
+    fireEvent.mouseOver(getByText('Info'));
+    await waitFor(() => expect(getByText('Metric data is not available')).toBeInTheDocument());
+  });
+});
diff --git a/...s/synthetics/components/monitors_page/overview/overview/metric_item/metric_item_extra.tsx b/...s/synthetics/components/monitors_page/overview/overview/metric_item/metric_item_extra.tsx
@@ -14,10 +14,10 @@ export const MetricItemExtra = ({
   stats,
 }: {
   stats: {
-    medianDuration: number;
-    avgDuration: number;
-    minDuration: number;
-    maxDuration: number;
+    medianDuration: number | null;
+    avgDuration: number | null;
+    minDuration: number | null;
+    maxDuration: number | null;
   };
 }) => {
   const { avgDuration, minDuration, maxDuration } = stats;
@@ -36,20 +36,31 @@ export const MetricItemExtra = ({
         })}
       </EuiFlexItem>
       <EuiFlexItem grow={false} component="span">
-        <EuiIconTip
-          title={i18n.translate('xpack.synthetics.overview.duration.description', {
-            defaultMessage: 'Median duration of last 50 checks',
-          })}
-          content={i18n.translate('xpack.synthetics.overview.duration.description.values', {
-            defaultMessage: 'Avg: {avg}, Min: {min}, Max: {max}',
-            values: {
-              avg: formatDuration(avgDuration, { noSpace: true }),
-              min: formatDuration(minDuration, { noSpace: true }),
-              max: formatDuration(maxDuration, { noSpace: true }),
-            },
-          })}
-          position="top"
-        />
+        {avgDuration && minDuration && maxDuration ? (
+          <EuiIconTip
+            title={i18n.translate('xpack.synthetics.overview.duration.description', {
+              defaultMessage: 'Median duration of last 50 checks',
+            })}
+            content={i18n.translate('xpack.synthetics.overview.duration.description.values', {
+              defaultMessage: 'Avg: {avg}, Min: {min}, Max: {max}',
+              values: {
+                avg: formatDuration(avgDuration, { noSpace: true }),
+                min: formatDuration(minDuration, { noSpace: true }),
+                max: formatDuration(maxDuration, { noSpace: true }),
+              },
+            })}
+            position="top"
+          />
+        ) : (
+          <EuiIconTip
+            title={i18n.translate('xpack.synthetics.overview.metricsTooltip.noMetrics.title', {
+              defaultMessage: 'Metric data is not available',
+            })}
+            content={i18n.translate('xpack.synthetics.overview.metricsTooltip.noMetrics.content', {
+              defaultMessage: 'No metric data available for this monitor',
+            })}
+          />
+        )}
       </EuiFlexItem>
     </EuiFlexGroup>
   );

diff --git a/...etics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx b/...etics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx
@@ -93,7 +93,7 @@ export const OverviewGrid = memo(() => {
   );
 
   useEffect(() => {
-    if (monitorsSortedByStatus.length && maxItem) {
+    if (monitorsSortedByStatus.length) {
       const batch: TrendRequest[] = [];
       const chunk = monitorsSortedByStatus.slice(0, (maxItem + 1) * ROW_COUNT);
       for (const item of chunk) {

diff --git a/...lugins/observability_solution/synthetics/public/apps/synthetics/state/overview/actions.ts b/...lugins/observability_solution/synthetics/public/apps/synthetics/state/overview/actions.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 import { createAction } from '@reduxjs/toolkit';
-import { TrendRequest, TrendTable } from '../../../../../common/types';
+import { GetTrendPayload, TrendRequest, TrendTable } from '../../../../../common/types';
 import { createAsyncAction } from '../utils/actions';
 
 import type {
@@ -39,6 +39,6 @@ export const refreshOverviewTrends = createAsyncAction<void, TrendTable, any>(
   'refreshOverviewTrendStats'
 );
 
-export const trendStatsBatch = createAsyncAction<TrendRequest[], TrendTable, any>(
+export const trendStatsBatch = createAsyncAction<TrendRequest[], GetTrendPayload, TrendRequest[]>(
   'batchTrendStats'
 );
diff --git a/...s/observability_solution/synthetics/public/apps/synthetics/state/overview/effects.test.ts b/...s/observability_solution/synthetics/public/apps/synthetics/state/overview/effects.test.ts
@@ -7,7 +7,7 @@
 
 import sagaHelper from 'redux-saga-testing';
 import { call, put, select } from 'redux-saga/effects';
-import { TrendKey, TrendRequest, TrendTable } from '../../../../../common/types';
+import { GetTrendPayload, TrendKey, TrendRequest, TrendTable } from '../../../../../common/types';
 import { TRENDS_CHUNK_SIZE, fetchTrendEffect, refreshTrends } from './effects';
 import { trendStatsBatch } from './actions';
 import { fetchOverviewTrendStats as trendsApi } from './api';
@@ -48,7 +48,9 @@ describe('overview effects', () => {
     });
 
     it('sends trends stats success action', (putResult) => {
-      expect(putResult).toEqual(put(trendStatsBatch.success(firstChunkResponse)));
+      expect(putResult).toEqual(
+        put(trendStatsBatch.success({ trendStats: firstChunkResponse, batch: firstChunk }))
+      );
     });
 
     it('calls the api for the second chunk', (callResult) => {
@@ -57,7 +59,9 @@ describe('overview effects', () => {
     });
 
     it('sends trends stats success action', (putResult) => {
-      expect(putResult).toEqual(put(trendStatsBatch.success(secondChunkResponse)));
+      expect(putResult).toEqual(
+        put(trendStatsBatch.success({ trendStats: secondChunkResponse, batch: secondChunk }))
+      );
     });
 
     it('terminates', (result) => {
@@ -111,6 +115,10 @@ describe('overview effects', () => {
       },
     };
 
+    const batch = [
+      { configId: 'monitor1', locationId: 'location', schedule: '3' },
+      { configId: 'monitor3', locationId: 'location', schedule: '3' },
+    ];
     const apiResponse: TrendTable = {
       monitor1: {
         configId: 'monitor1',
@@ -142,6 +150,11 @@ describe('overview effects', () => {
       },
     };
 
+    const successPayload: GetTrendPayload = {
+      trendStats: apiResponse,
+      batch,
+    };
+
     it('selects the trends in the table', (selectResult) => {
       expect(selectResult).toEqual(select(selectOverviewTrends));
 
@@ -161,18 +174,13 @@ describe('overview effects', () => {
     });
 
     it('calls the api for the first chunk', (callResult) => {
-      expect(callResult).toEqual(
-        call(trendsApi, [
-          { configId: 'monitor1', locationId: 'location', schedule: '3' },
-          { configId: 'monitor3', locationId: 'location', schedule: '3' },
-        ])
-      );
+      expect(callResult).toEqual(call(trendsApi, batch));
 
       return apiResponse;
     });
 
     it('sends trends stats success action', (putResult) => {
-      expect(putResult).toEqual(put(trendStatsBatch.success(apiResponse)));
+      expect(putResult).toEqual(put(trendStatsBatch.success(successPayload)));
     });
   });
 });
diff --git a/...lugins/observability_solution/synthetics/public/apps/synthetics/state/overview/effects.ts b/...lugins/observability_solution/synthetics/public/apps/synthetics/state/overview/effects.ts
@@ -6,7 +6,7 @@
  */
 
 import { debounce, call, takeLeading, takeEvery, put, select } from 'redux-saga/effects';
-import type { TrendTable } from '../../../../../common/types';
+import type { OverviewTrend, TrendTable } from '../../../../../common/types';
 import { fetchEffectFactory } from '../utils/fetch_effect';
 import { selectOverviewState, selectOverviewTrends } from './selectors';
 import {
@@ -41,11 +41,11 @@ export function* fetchTrendEffect(
       const chunk = action.payload.slice(Math.max(i - TRENDS_CHUNK_SIZE, 0), i);
       if (chunk.length > 0) {
         const trendStats = yield call(trendsApi, chunk);
-        yield put(trendStatsBatch.success(trendStats));
+        yield put(trendStatsBatch.success({ trendStats, batch: chunk }));
       }
     }
   } catch (e: any) {
-    yield put(trendStatsBatch.fail(e));
+    yield put(trendStatsBatch.fail(action.payload));
   }
 }
 
@@ -57,33 +57,32 @@ export function* refreshTrends(): Generator<unknown, void, any> {
   const existingTrends: TrendTable = yield select(selectOverviewTrends);
   const overviewState: MonitorOverviewState = yield select(selectOverviewState);
 
-  let acc = {};
   const keys = Object.keys(existingTrends);
   while (keys.length) {
     const chunk = keys
       .splice(0, keys.length < 10 ? keys.length : 40)
       .filter(
         (key: string) =>
           existingTrends[key] !== null &&
+          existingTrends[key] !== 'loading' &&
           overviewState.data.monitors.some(
-            ({ configId }) => configId === existingTrends[key]!.configId
+            ({ configId }) => configId === (existingTrends[key] as OverviewTrend)!.configId
           )
       )
-      .map((key: string) => ({
-        configId: existingTrends[key]!.configId,
-        locationId: existingTrends[key]!.locationId,
-        schedule: overviewState.data.monitors.find(
-          ({ configId }) => configId === existingTrends[key]!.configId
-        )!.schedule,
-      }));
+      .map((key: string) => {
+        const trend = existingTrends[key] as OverviewTrend;
+        return {
+          configId: trend.configId,
+          locationId: trend.locationId,
+          schedule: overviewState.data.monitors.find(({ configId }) => configId === trend.configId)!
+            .schedule,
+        };
+      });
     if (chunk.length) {
-      const res = yield call(trendsApi, chunk);
-      acc = { ...acc, ...res };
+      const trendStats = yield call(trendsApi, chunk);
+      yield put(trendStatsBatch.success({ trendStats, batch: chunk }));
     }
   }
-  if (Object.keys(acc).length) {
-    yield put(trendStatsBatch.success(acc));
-  }
 }
 
 export function* refreshOverviewTrendStats() {

diff --git a/.../plugins/observability_solution/synthetics/public/apps/synthetics/state/overview/index.ts b/.../plugins/observability_solution/synthetics/public/apps/synthetics/state/overview/index.ts
@@ -99,13 +99,25 @@ export const monitorOverviewReducer = createReducer(initialState, (builder) => {
     .addCase(trendStatsBatch.get, (state, action) => {
       for (const { configId, locationId } of action.payload) {
         if (!state.trendStats[configId + locationId]) {
+          state.trendStats[configId + locationId] = 'loading';
+        }
+      }
+    })
+    .addCase(trendStatsBatch.fail, (state, action) => {
+      for (const { configId, locationId } of action.payload) {
+        if (state.trendStats[configId + locationId] === 'loading') {
           state.trendStats[configId + locationId] = null;
         }
       }
     })
     .addCase(trendStatsBatch.success, (state, action) => {
-      for (const key of Object.keys(action.payload)) {
-        state.trendStats[key] = action.payload[key];
+      for (const key of Object.keys(action.payload.trendStats)) {
+        state.trendStats[key] = action.payload.trendStats[key];
+      }
+      for (const { configId, locationId } of action.payload.batch) {
+        if (!action.payload.trendStats[configId + locationId]) {
+          state.trendStats[configId + locationId] = null;
+        }
       }
     });
 });