Skip to content

Commit

Permalink
[APM] Blank page when selecting a future time range (#87298)
Browse files Browse the repository at this point in the history
* fixing blank page

* fixing blank page

* addressing PR comments

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
cauemarcondes and kibanamachine committed Jan 14, 2021
1 parent fbfc509 commit 75891b7
Show file tree
Hide file tree
Showing 18 changed files with 115 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -92,7 +91,7 @@ describe('ServiceMap', () => {
jest.spyOn(useFetcherModule, 'useFetcher').mockReturnValueOnce({
data: { elements: [] },
refetch: () => {},
status: FETCH_STATUS.SUCCESS,
status: useFetcherModule.FETCH_STATUS.SUCCESS,
});

expect(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 />;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export function TransactionOverview({ serviceName }: TransactionOverviewProps) {
typeof LocalUIFilters
> = useMemo(
() => ({
shouldFetch: !!transactionType,
filterNames: [
'transactionResult',
'host',
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ interface Props {
params?: Record<string, string | number | boolean | undefined>;
showCount?: boolean;
children?: React.ReactNode;
shouldFetch?: boolean;
}

const ButtonWrapper = styled.div`
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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} />;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ({
Expand Down Expand Up @@ -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
)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 })}
Expand Down
27 changes: 16 additions & 11 deletions x-pack/plugins/apm/public/hooks/useLocalUIFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand All @@ -87,6 +91,7 @@ export function useLocalUIFilters({
urlParams.end,
filterNames,
params,
shouldFetch,
]);

const filters = data.map((filter) => ({
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/apm/public/hooks/use_fetcher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export enum FETCH_STATUS {
LOADING = 'loading',
SUCCESS = 'success',
FAILURE = 'failure',
PENDING = 'pending',
NOT_INITIATED = 'not_initiated',
}

export interface FetcherResult<Data> {
Expand Down Expand Up @@ -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);

Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/apm/server/lib/services/get_throughput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));
}

Expand Down

0 comments on commit 75891b7

Please sign in to comment.