[APM] Blank page when selecting a future time range (#87298) (#88306)

cauemarcondes · kibanamachine · web-flow · commit 2ddc8cfad917 · 2021-01-14T12:15:37.000+01:00
* fixing blank page

* fixing blank page

* addressing PR comments

Co-authored-by: Kibana Machine &lt;42973632+kibanamachine@users.noreply.github.com&gt;

Co-authored-by: Kibana Machine &lt;42973632+kibanamachine@users.noreply.github.com&gt;
diff --git a/x-pack/plugins/apm/public/components/app/ServiceMap/Popover/ServiceStatsFetcher.tsx b/x-pack/plugins/apm/public/components/app/ServiceMap/Popover/ServiceStatsFetcher.tsx
@@ -56,8 +56,7 @@ export function ServiceStatsFetcher({
     }
   );
 
-  const isLoading =
-    status === FETCH_STATUS.PENDING || status === FETCH_STATUS.LOADING;
+  const isLoading = status === FETCH_STATUS.LOADING;
 
   if (isLoading) {
     return <LoadingSpinner />;
diff --git a/x-pack/plugins/apm/public/components/app/ServiceMap/index.test.tsx b/x-pack/plugins/apm/public/components/app/ServiceMap/index.test.tsx
@@ -11,7 +11,6 @@ import React, { ReactNode } from 'react';
 import { createKibanaReactContext } from 'src/plugins/kibana_react/public';
 import { License } from '../../../../../licensing/common/license';
 import { EuiThemeProvider } from '../../../../../observability/public';
-import { FETCH_STATUS } from '../../../../../observability/public/hooks/use_fetcher';
 import { MockApmPluginContextWrapper } from '../../../context/apm_plugin/mock_apm_plugin_context';
 import { LicenseContext } from '../../../context/license/license_context';
 import * as useFetcherModule from '../../../hooks/use_fetcher';
@@ -92,7 +91,7 @@ describe('ServiceMap', () => {
         jest.spyOn(useFetcherModule, 'useFetcher').mockReturnValueOnce({
           data: { elements: [] },
           refetch: () => {},
-          status: FETCH_STATUS.SUCCESS,
+          status: useFetcherModule.FETCH_STATUS.SUCCESS,
         });
 
         expect(
diff --git a/x-pack/plugins/apm/public/components/app/Settings/anomaly_detection/add_environments.tsx b/x-pack/plugins/apm/public/components/app/Settings/anomaly_detection/add_environments.tsx
@@ -71,8 +71,7 @@ export function AddEnvironments({
     );
   }
 
-  const isLoading =
-    status === FETCH_STATUS.PENDING || status === FETCH_STATUS.LOADING;
+  const isLoading = status === FETCH_STATUS.LOADING;
   return (
     <EuiPanel>
       <EuiTitle>
diff --git a/x-pack/plugins/apm/public/components/app/Settings/anomaly_detection/jobs_list.tsx b/x-pack/plugins/apm/public/components/app/Settings/anomaly_detection/jobs_list.tsx
@@ -124,8 +124,7 @@ export function JobsList({ data, status, onAddEnvironments }: Props) {
 
 function getNoItemsMessage({ status }: { status: FETCH_STATUS }) {
   // loading state
-  const isLoading =
-    status === FETCH_STATUS.PENDING || status === FETCH_STATUS.LOADING;
+  const isLoading = status === FETCH_STATUS.LOADING;
   if (isLoading) {
     return <LoadingStatePrompt />;
   }
diff --git a/x-pack/plugins/apm/public/components/app/service_details/service_icons/icon_popover.tsx b/x-pack/plugins/apm/public/components/app/service_details/service_icons/icon_popover.tsx
@@ -35,10 +35,7 @@ export function IconPopover({
   if (!icon) {
     return null;
   }
-  const isLoading =
-    detailsFetchStatus === FETCH_STATUS.LOADING ||
-    detailsFetchStatus === FETCH_STATUS.PENDING;
-
+  const isLoading = detailsFetchStatus === FETCH_STATUS.LOADING;
   return (
     <EuiPopover
       anchorPosition="downCenter"
diff --git a/x-pack/plugins/apm/public/components/app/service_details/service_icons/index.tsx b/x-pack/plugins/apm/public/components/app/service_details/service_icons/index.tsx
@@ -94,10 +94,7 @@ export function ServiceIcons({ serviceName }: Props) {
     [selectedIconPopover, serviceName, start, end, uiFilters]
   );
 
-  const isLoading =
-    !icons &&
-    (iconsFetchStatus === FETCH_STATUS.LOADING ||
-      iconsFetchStatus === FETCH_STATUS.PENDING);
+  const isLoading = !icons && iconsFetchStatus === FETCH_STATUS.LOADING;
 
   if (isLoading) {
     return <EuiLoadingSpinner data-test-subj="loading" />;
diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_instances_table/index.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_instances_table/index.tsx
@@ -230,8 +230,7 @@ export function ServiceOverviewInstancesTable({ serviceName }: Props) {
     memoryUsageValue: item.memoryUsage?.value ?? 0,
   }));
 
-  const isLoading =
-    status === FETCH_STATUS.LOADING || status === FETCH_STATUS.PENDING;
+  const isLoading = status === FETCH_STATUS.LOADING;
 
   return (
     <EuiFlexGroup direction="column" gutterSize="s">
diff --git a/x-pack/plugins/apm/public/components/app/transaction_overview/index.tsx b/x-pack/plugins/apm/public/components/app/transaction_overview/index.tsx
@@ -83,6 +83,7 @@ export function TransactionOverview({ serviceName }: TransactionOverviewProps) {
     typeof LocalUIFilters
   > = useMemo(
     () => ({
+      shouldFetch: !!transactionType,
       filterNames: [
         'transactionResult',
         'host',
@@ -101,7 +102,7 @@ export function TransactionOverview({ serviceName }: TransactionOverviewProps) {
 
   // TODO: improve urlParams typings.
   // `serviceName` or `transactionType` will never be undefined here, and this check should not be needed
-  if (!serviceName || !transactionType) {
+  if (!serviceName) {
     return null;
   }
 
diff --git a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/index.tsx b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/index.tsx
@@ -24,6 +24,7 @@ interface Props {
   params?: Record<string, string | number | boolean | undefined>;
   showCount?: boolean;
   children?: React.ReactNode;
+  shouldFetch?: boolean;
 }
 
 const ButtonWrapper = styled.div`
@@ -36,11 +37,13 @@ function LocalUIFilters({
   filterNames,
   children,
   showCount = true,
+  shouldFetch = true,
 }: Props) {
   const { filters, setFilterValue, clearValues } = useLocalUIFilters({
     filterNames,
     projection,
     params,
+    shouldFetch,
   });
 
   const hasValues = filters.some((filter) => filter.value.length > 0);
diff --git a/x-pack/plugins/apm/public/components/shared/charts/chart_container.test.tsx b/x-pack/plugins/apm/public/components/shared/charts/chart_container.test.tsx
@@ -11,7 +11,7 @@ import { ChartContainer } from './chart_container';
 describe('ChartContainer', () => {
   describe('loading indicator', () => {
     it('shows loading when status equals to Loading or Pending and has no data', () => {
-      [FETCH_STATUS.PENDING, FETCH_STATUS.LOADING].map((status) => {
+      [FETCH_STATUS.NOT_INITIATED, FETCH_STATUS.LOADING].map((status) => {
         const { queryAllByTestId } = render(
           <ChartContainer
             height={100}
@@ -26,7 +26,7 @@ describe('ChartContainer', () => {
       });
     });
     it('does not show loading when status equals to Loading or Pending and has data', () => {
-      [FETCH_STATUS.PENDING, FETCH_STATUS.LOADING].map((status) => {
+      [FETCH_STATUS.NOT_INITIATED, FETCH_STATUS.LOADING].map((status) => {
         const { queryAllByText } = render(
           <ChartContainer height={100} status={status} hasData={true}>
             <div>My amazing component</div>
diff --git a/x-pack/plugins/apm/public/components/shared/charts/chart_container.tsx b/x-pack/plugins/apm/public/components/shared/charts/chart_container.tsx
@@ -17,10 +17,7 @@ interface Props {
 }
 
 export function ChartContainer({ children, height, status, hasData }: Props) {
-  if (
-    !hasData &&
-    (status === FETCH_STATUS.LOADING || status === FETCH_STATUS.PENDING)
-  ) {
+  if (!hasData && status === FETCH_STATUS.LOADING) {
     return <LoadingChartPlaceholder height={height} />;
   }
 
diff --git a/x-pack/plugins/apm/public/components/shared/charts/helper/helper.test.ts b/x-pack/plugins/apm/public/components/shared/charts/helper/helper.test.ts
@@ -3,8 +3,9 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import { onBrushEnd } from './helper';
+import { onBrushEnd, isTimeseriesEmpty } from './helper';
 import { History } from 'history';
+import { TimeSeries } from '../../../../../typings/timeseries';
 
 describe('Chart helper', () => {
   describe('onBrushEnd', () => {
@@ -36,4 +37,60 @@ describe('Chart helper', () => {
       });
     });
   });
+
+  describe('isTimeseriesEmpty', () => {
+    it('returns true when timeseries is undefined', () => {
+      expect(isTimeseriesEmpty()).toBeTruthy();
+    });
+    it('returns true when timeseries data is empty', () => {
+      const timeseries = [
+        {
+          title: 'foo',
+          data: [],
+          type: 'line',
+          color: 'red',
+        },
+      ] as TimeSeries[];
+      expect(isTimeseriesEmpty(timeseries)).toBeTruthy();
+    });
+    it('returns true when y coordinate is null', () => {
+      const timeseries = [
+        {
+          title: 'foo',
+          data: [{ x: 1, y: null }],
+          type: 'line',
+          color: 'red',
+        },
+      ] as TimeSeries[];
+      expect(isTimeseriesEmpty(timeseries)).toBeTruthy();
+    });
+    it('returns true when y coordinate is undefined', () => {
+      const timeseries = [
+        {
+          title: 'foo',
+          data: [{ x: 1, y: undefined }],
+          type: 'line',
+          color: 'red',
+        },
+      ] as TimeSeries[];
+      expect(isTimeseriesEmpty(timeseries)).toBeTruthy();
+    });
+    it('returns false when at least one coordinate is filled', () => {
+      const timeseries = [
+        {
+          title: 'foo',
+          data: [{ x: 1, y: undefined }],
+          type: 'line',
+          color: 'red',
+        },
+        {
+          title: 'bar',
+          data: [{ x: 1, y: 1 }],
+          type: 'line',
+          color: 'green',
+        },
+      ] as TimeSeries[];
+      expect(isTimeseriesEmpty(timeseries)).toBeFalsy();
+    });
+  });
 });
diff --git a/x-pack/plugins/apm/public/components/shared/charts/helper/helper.ts b/x-pack/plugins/apm/public/components/shared/charts/helper/helper.ts
@@ -6,6 +6,7 @@
 
 import { XYBrushArea } from '@elastic/charts';
 import { History } from 'history';
+import { TimeSeries } from '../../../../../typings/timeseries';
 import { fromQuery, toQuery } from '../../Links/url_helpers';
 
 export const onBrushEnd = ({
@@ -33,3 +34,16 @@ export const onBrushEnd = ({
     });
   }
 };
+
+export function isTimeseriesEmpty(timeseries?: TimeSeries[]) {
+  return (
+    !timeseries ||
+    timeseries
+      .map((serie) => serie.data)
+      .flat()
+      .every(
+        ({ y }: { x?: number | null; y?: number | null }) =>
+          y === null || y === undefined
+      )
+  );
+}
diff --git a/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx b/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx
@@ -36,7 +36,7 @@ import { useAnnotationsContext } from '../../../context/annotations/use_annotati
 import { useChartPointerEventContext } from '../../../context/chart_pointer_event/use_chart_pointer_event_context';
 import { unit } from '../../../style/variables';
 import { ChartContainer } from './chart_container';
-import { onBrushEnd } from './helper/helper';
+import { onBrushEnd, isTimeseriesEmpty } from './helper/helper';
 import { getLatencyChartSelector } from '../../../selectors/latency_chart_selectors';
 
 interface Props {
@@ -86,13 +86,7 @@ export function TimeseriesChart({
 
   const xFormatter = niceTimeFormatter([min, max]);
 
-  const isEmpty = timeseries
-    .map((serie) => serie.data)
-    .flat()
-    .every(
-      ({ y }: { x?: number | null; y?: number | null }) =>
-        y === null || y === undefined
-    );
+  const isEmpty = isTimeseriesEmpty(timeseries);
 
   const annotationColor = theme.eui.euiColorSecondary;
 
diff --git a/x-pack/plugins/apm/public/components/shared/charts/transaction_breakdown_chart/transaction_breakdown_chart_contents.tsx b/x-pack/plugins/apm/public/components/shared/charts/transaction_breakdown_chart/transaction_breakdown_chart_contents.tsx
@@ -35,7 +35,7 @@ import { useAnnotationsContext } from '../../../../context/annotations/use_annot
 import { useChartPointerEventContext } from '../../../../context/chart_pointer_event/use_chart_pointer_event_context';
 import { unit } from '../../../../style/variables';
 import { ChartContainer } from '../../charts/chart_container';
-import { onBrushEnd } from '../../charts/helper/helper';
+import { isTimeseriesEmpty, onBrushEnd } from '../../charts/helper/helper';
 
 interface Props {
   fetchStatus: FETCH_STATUS;
@@ -66,8 +66,10 @@ export function TransactionBreakdownChartContents({
 
   const annotationColor = theme.eui.euiColorSecondary;
 
+  const isEmpty = isTimeseriesEmpty(timeseries);
+
   return (
-    <ChartContainer height={height} hasData={!!timeseries} status={fetchStatus}>
+    <ChartContainer height={height} hasData={!isEmpty} status={fetchStatus}>
       <Chart ref={chartRef}>
         <Settings
           onBrushEnd={({ x }) => onBrushEnd({ x, history })}
diff --git a/x-pack/plugins/apm/public/hooks/useLocalUIFilters.ts b/x-pack/plugins/apm/public/hooks/useLocalUIFilters.ts
@@ -34,10 +34,12 @@ export function useLocalUIFilters({
   projection,
   filterNames,
   params,
+  shouldFetch,
 }: {
   projection: Projection;
   filterNames: LocalUIFilterName[];
   params?: Record<string, string | number | boolean | undefined>;
+  shouldFetch: boolean;
 }) {
   const history = useHistory();
   const { uiFilters, urlParams } = useUrlParams();
@@ -68,17 +70,19 @@ export function useLocalUIFilters({
   };
 
   const { data = getInitialData(filterNames), status } = useFetcher(() => {
-    return callApi<LocalUIFiltersAPIResponse>({
-      method: 'GET',
-      pathname: `/api/apm/ui_filters/local_filters/${projection}`,
-      query: {
-        uiFilters: JSON.stringify(uiFilters),
-        start: urlParams.start,
-        end: urlParams.end,
-        filterNames: JSON.stringify(filterNames),
-        ...params,
-      },
-    });
+    if (shouldFetch) {
+      return callApi<LocalUIFiltersAPIResponse>({
+        method: 'GET',
+        pathname: `/api/apm/ui_filters/local_filters/${projection}`,
+        query: {
+          uiFilters: JSON.stringify(uiFilters),
+          start: urlParams.start,
+          end: urlParams.end,
+          filterNames: JSON.stringify(filterNames),
+          ...params,
+        },
+      });
+    }
   }, [
     callApi,
     projection,
@@ -87,6 +91,7 @@ export function useLocalUIFilters({
     urlParams.end,
     filterNames,
     params,
+    shouldFetch,
   ]);
 
   const filters = data.map((filter) => ({
diff --git a/x-pack/plugins/apm/public/hooks/use_fetcher.tsx b/x-pack/plugins/apm/public/hooks/use_fetcher.tsx
@@ -15,7 +15,7 @@ export enum FETCH_STATUS {
   LOADING = 'loading',
   SUCCESS = 'success',
   FAILURE = 'failure',
-  PENDING = 'pending',
+  NOT_INITIATED = 'not_initiated',
 }
 
 export interface FetcherResult<Data> {
@@ -46,7 +46,7 @@ export function useFetcher<TReturn>(
     FetcherResult<InferResponseType<TReturn>>
   >({
     data: undefined,
-    status: FETCH_STATUS.PENDING,
+    status: FETCH_STATUS.NOT_INITIATED,
   });
   const [counter, setCounter] = useState(0);
 
diff --git a/x-pack/plugins/apm/server/lib/services/get_throughput.ts b/x-pack/plugins/apm/server/lib/services/get_throughput.ts
@@ -28,7 +28,10 @@ interface Options {
 type ESResponse = PromiseReturnType<typeof fetcher>;
 
 function transform(response: ESResponse) {
-  const buckets = response.aggregations?.throughput?.buckets ?? [];
+  if (response.hits.total.value === 0) {
+    return [];
+  }
+  const buckets = response.aggregations?.throughput.buckets ?? [];
   return buckets.map(({ key: x, doc_count: y }) => ({ x, y }));
 }
 

Original file line number	Diff line number	Diff line change
`@@ -56,8 +56,7 @@ export function ServiceStatsFetcher({`
`56`	`56`	`}`
`57`	`57`	`);`
`58`	`58`
`59`		`- const isLoading =`
`60`		`- status === FETCH_STATUS.PENDING \|\| status === FETCH_STATUS.LOADING;`
	`59`	`+ const isLoading = status === FETCH_STATUS.LOADING;`
`61`	`60`
`62`	`61`	`if (isLoading) {`
`63`	`62`	`return <LoadingSpinner />;`
Original file line number	Diff line number	Diff line change
`@@ -71,8 +71,7 @@ export function AddEnvironments({`
`71`	`71`	`);`
`72`	`72`	`}`
`73`	`73`
`74`		`- const isLoading =`
`75`		`- status === FETCH_STATUS.PENDING \|\| status === FETCH_STATUS.LOADING;`
	`74`	`+ const isLoading = status === FETCH_STATUS.LOADING;`
`76`	`75`	`return (`
`77`	`76`	`<EuiPanel>`
`78`	`77`	`<EuiTitle>`
Original file line number	Diff line number	Diff line change
`@@ -124,8 +124,7 @@ export function JobsList({ data, status, onAddEnvironments }: Props) {`
`124`	`124`
`125`	`125`	`function getNoItemsMessage({ status }: { status: FETCH_STATUS }) {`
`126`	`126`	`// loading state`
`127`		`- const isLoading =`
`128`		`- status === FETCH_STATUS.PENDING \|\| status === FETCH_STATUS.LOADING;`
	`127`	`+ const isLoading = status === FETCH_STATUS.LOADING;`
`129`	`128`	`if (isLoading) {`
`130`	`129`	`return <LoadingStatePrompt />;`
`131`	`130`	`}`