Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Observability] Load hasData call asynchronously #80644

Merged
merged 21 commits into from
Nov 23, 2020
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
FetchDataParams,
HasDataParams,
UxFetchDataResponse,
UXHasDataResponse,
} from '../../../../../observability/public/';
import { callApmApi } from '../../../services/rest/createCallApmApi';

Expand Down Expand Up @@ -35,7 +36,9 @@ export const fetchUxOverviewDate = async ({
};
};

export async function hasRumData({ absoluteTime }: HasDataParams) {
export async function hasRumData({
absoluteTime,
}: HasDataParams): Promise<UXHasDataResponse> {
return await callApmApi({
endpoint: 'GET /api/apm/observability_overview/has_rum_data',
params: {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/apm/server/lib/rum_client/has_rum_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,6 @@ export async function hasRumData({ setup }: { setup: Setup & SetupTimeRange }) {
response.aggregations?.services?.mostTraffic?.buckets?.[0]?.key,
};
} catch (e) {
return false;
return { hasData: false, serviceName: undefined };
}
}
11 changes: 2 additions & 9 deletions x-pack/plugins/infra/public/utils/logs_overview_fetchers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,7 @@

import { encode } from 'rison-node';
import { SearchResponse } from 'elasticsearch';
import {
FetchData,
FetchDataParams,
HasData,
LogsFetchDataResponse,
} from '../../../observability/public';
import { FetchData, FetchDataParams, LogsFetchDataResponse } from '../../../observability/public';
import { DEFAULT_SOURCE_ID } from '../../common/constants';
import { callFetchLogSourceConfigurationAPI } from '../containers/logs/log_source/api/fetch_log_source_configuration';
import { callFetchLogSourceStatusAPI } from '../containers/logs/log_source/api/fetch_log_source_status';
Expand All @@ -38,9 +33,7 @@ interface LogParams {

type StatsAndSeries = Pick<LogsFetchDataResponse, 'stats' | 'series'>;

export function getLogsHasDataFetcher(
getStartServices: InfraClientCoreSetup['getStartServices']
): HasData {
export function getLogsHasDataFetcher(getStartServices: InfraClientCoreSetup['getStartServices']) {
return async () => {
const [core] = await getStartServices();
const sourceStatus = await callFetchLogSourceStatusAPI(DEFAULT_SOURCE_ID, core.http.fetch);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,25 @@ import { ObservabilityPluginSetupDeps } from '../plugin';
import { renderApp } from './';

describe('renderApp', () => {
const originalConsole = global.console;
beforeAll(() => {
// mocks console to avoid poluting the test output
global.console = ({ error: jest.fn() } as unknown) as typeof console;
});

afterAll(() => {
global.console = originalConsole;
});
it('renders', async () => {
const plugins = ({
usageCollection: { reportUiStats: () => {} },
data: {
query: {
timefilter: {
timefilter: { setTime: jest.fn(), getTime: jest.fn().mockImplementation(() => ({})) },
},
},
},
} as unknown) as ObservabilityPluginSetupDeps;
const core = ({
application: { currentAppId$: new Observable(), navigateToUrl: () => {} },
Expand Down
9 changes: 6 additions & 3 deletions x-pack/plugins/observability/public/application/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { PluginContext } from '../context/plugin_context';
import { usePluginContext } from '../hooks/use_plugin_context';
import { useRouteParams } from '../hooks/use_route_params';
import { ObservabilityPluginSetupDeps } from '../plugin';
import { HasDataContextProvider } from '../context/has_data_context';
import { Breadcrumbs, routes } from '../routes';

const observabilityLabelBreadcrumb = {
Expand Down Expand Up @@ -46,8 +47,8 @@ function App() {
core.chrome.docTitle.change(getTitleFromBreadCrumbs(breadcrumb));
}, [core, breadcrumb]);

const { query, path: pathParams } = useRouteParams(route.params);
return route.handler({ query, path: pathParams });
const params = useRouteParams(path);
return route.handler(params);
};
return <Route key={path} path={path} exact={true} component={Wrapper} />;
})}
Expand Down Expand Up @@ -79,7 +80,9 @@ export const renderApp = (
<EuiThemeProvider darkMode={isDarkMode}>
<i18nCore.Context>
<RedirectAppLinks application={core.application}>
<App />
<HasDataContextProvider>
<App />
</HasDataContextProvider>
</RedirectAppLinks>
</i18nCore.Context>
</EuiThemeProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import React from 'react';
import { ISection } from '../../../typings/section';
import { render } from '../../../utils/test_helper';
import { EmptySection } from './';
import { EmptySection } from './empty_section';

describe('EmptySection', () => {
it('renders without action button', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { EuiFlexGrid, EuiFlexItem, EuiSpacer } from '@elastic/eui';
import React, { useContext } from 'react';
import { ThemeContext } from 'styled-components';
import { Alert } from '../../../../../alerts/common';
import { FETCH_STATUS } from '../../../hooks/use_fetcher';
import { useHasData } from '../../../hooks/use_has_data';
import { usePluginContext } from '../../../hooks/use_plugin_context';
import { getEmptySections } from '../../../pages/overview/empty_section';
import { UXHasDataResponse } from '../../../typings';
import { EmptySection } from './empty_section';

export function EmptySections() {
const { core } = usePluginContext();
const theme = useContext(ThemeContext);
const { hasData } = useHasData();

const appEmptySections = getEmptySections({ core }).filter(({ id }) => {
if (id === 'alert') {
const { status, hasData: alerts } = hasData.alert || {};
return (
status === FETCH_STATUS.FAILURE ||
(status === FETCH_STATUS.SUCCESS && (alerts as Alert[]).length === 0)
);
} else {
const app = hasData[id];
if (app) {
const _hasData = id === 'ux' ? (app.hasData as UXHasDataResponse)?.hasData : app.hasData;
return app.status === FETCH_STATUS.FAILURE || !_hasData;
}
}
return false;
Comment on lines +23 to +36
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should get rid of the special logic for alert and ux here if possible

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove the alert logic based on my comment here. Ux will demand more changes with I'm not familiar to, maybe we could leave it like this for now and improve it in another PR.

});
return (
<EuiFlexItem>
<EuiSpacer size="s" />
<EuiFlexGrid
columns={
// when more than 2 empty sections are available show them on 2 columns, otherwise 1
appEmptySections.length > 2 ? 2 : 1
}
gutterSize="s"
>
{appEmptySections.map((app) => {
return (
<EuiFlexItem
key={app.id}
style={{
border: `1px dashed ${theme.eui.euiBorderColor}`,
borderRadius: '4px',
}}
>
<EmptySection section={app} />
</EuiFlexItem>
);
})}
</EuiFlexGrid>
</EuiFlexItem>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,59 @@ import * as fetcherHook from '../../../../hooks/use_fetcher';
import { render } from '../../../../utils/test_helper';
import { APMSection } from './';
import { response } from './mock_data/apm.mock';
import moment from 'moment';
import * as hasDataHook from '../../../../hooks/use_has_data';
import * as pluginContext from '../../../../hooks/use_plugin_context';
import { HasDataContextValue } from '../../../../context/has_data_context';
import { AppMountParameters, CoreStart } from 'kibana/public';
import { ObservabilityPluginSetupDeps } from '../../../../plugin';

jest.mock('react-router-dom', () => ({
useLocation: () => ({
pathname: '/observability/overview/',
search: '',
}),
useHistory: jest.fn(),
}));

describe('APMSection', () => {
beforeAll(() => {
jest.spyOn(hasDataHook, 'useHasData').mockReturnValue({
hasData: {
apm: {
status: fetcherHook.FETCH_STATUS.SUCCESS,
hasData: true,
},
},
} as HasDataContextValue);
jest.spyOn(pluginContext, 'usePluginContext').mockImplementation(() => ({
core: ({
uiSettings: { get: jest.fn() },
http: { basePath: { prepend: jest.fn() } },
} as unknown) as CoreStart,
appMountParameters: {} as AppMountParameters,
plugins: ({
data: {
query: {
timefilter: {
timefilter: {
getTime: jest.fn().mockImplementation(() => ({
from: '2020-10-08T06:00:00.000Z',
to: '2020-10-08T07:00:00.000Z',
})),
},
},
},
},
} as unknown) as ObservabilityPluginSetupDeps,
}));
});
it('renders with transaction series and stats', () => {
jest.spyOn(fetcherHook, 'useFetcher').mockReturnValue({
data: response,
status: fetcherHook.FETCH_STATUS.SUCCESS,
refetch: jest.fn(),
});
const { getByText, queryAllByTestId } = render(
<APMSection
absoluteTime={{
start: moment('2020-06-29T11:38:23.747Z').valueOf(),
end: moment('2020-06-29T12:08:23.748Z').valueOf(),
}}
relativeTime={{ start: 'now-15m', end: 'now' }}
bucketSize="60s"
/>
);
const { getByText, queryAllByTestId } = render(<APMSection bucketSize="60s" />);

expect(getByText('APM')).toBeInTheDocument();
expect(getByText('View in app')).toBeInTheDocument();
Expand All @@ -40,16 +74,7 @@ describe('APMSection', () => {
status: fetcherHook.FETCH_STATUS.LOADING,
refetch: jest.fn(),
});
const { getByText, queryAllByText, getByTestId } = render(
<APMSection
absoluteTime={{
start: moment('2020-06-29T11:38:23.747Z').valueOf(),
end: moment('2020-06-29T12:08:23.748Z').valueOf(),
}}
relativeTime={{ start: 'now-15m', end: 'now' }}
bucketSize="60s"
/>
);
const { getByText, queryAllByText, getByTestId } = render(<APMSection bucketSize="60s" />);

expect(getByText('APM')).toBeInTheDocument();
expect(getByTestId('loading')).toBeInTheDocument();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,43 +12,54 @@ import moment from 'moment';
import React, { useContext } from 'react';
import { useHistory } from 'react-router-dom';
import { ThemeContext } from 'styled-components';
import { useTimeRange } from '../../../../hooks/use_time_range';
import { SectionContainer } from '../';
import { getDataHandler } from '../../../../data_handler';
import { useChartTheme } from '../../../../hooks/use_chart_theme';
import { FETCH_STATUS, useFetcher } from '../../../../hooks/use_fetcher';
import { useHasData } from '../../../../hooks/use_has_data';
import { ChartContainer } from '../../chart_container';
import { StyledStat } from '../../styled_stat';
import { onBrushEnd } from '../helper';

interface Props {
absoluteTime: { start?: number; end?: number };
relativeTime: { start: string; end: string };
bucketSize?: string;
}

function formatTpm(value?: number) {
return numeral(value).format('0.00a');
}

export function APMSection({ absoluteTime, relativeTime, bucketSize }: Props) {
export function APMSection({ bucketSize }: Props) {
const theme = useContext(ThemeContext);
const chartTheme = useChartTheme();
const history = useHistory();
const { forceUpdate, hasData } = useHasData();
const { relativeStart, relativeEnd, absoluteStart, absoluteEnd } = useTimeRange();

const { start, end } = absoluteTime;
const { data, status } = useFetcher(() => {
if (start && end && bucketSize) {
return getDataHandler('apm')?.fetchData({
absoluteTime: { start, end },
relativeTime,
bucketSize,
});
}
}, [start, end, bucketSize, relativeTime]);
const { data, status } = useFetcher(
() => {
if (bucketSize) {
return getDataHandler('apm')?.fetchData({
absoluteTime: { start: absoluteStart, end: absoluteEnd },
relativeTime: { start: relativeStart, end: relativeEnd },
bucketSize,
});
}
},
// Absolute times shouldn't be used here, since it would refetch on every render
// eslint-disable-next-line react-hooks/exhaustive-deps
[bucketSize, relativeStart, relativeEnd, forceUpdate]
);

if (!hasData.apm?.hasData) {
return null;
}

const { appLink, stats, series } = data || {};

const min = moment.utc(absoluteTime.start).valueOf();
const max = moment.utc(absoluteTime.end).valueOf();
const min = moment.utc(absoluteStart).valueOf();
const max = moment.utc(absoluteEnd).valueOf();

const formatter = niceTimeFormatter([min, max]);

Expand Down Expand Up @@ -93,7 +104,7 @@ export function APMSection({ absoluteTime, relativeTime, bucketSize }: Props) {
<ChartContainer isInitialLoad={isLoading && !data}>
<Settings
onBrushEnd={({ x }) => onBrushEnd({ x, history })}
theme={useChartTheme()}
theme={chartTheme}
showLegend={false}
xDomain={{ min, max }}
/>
Expand Down
Loading