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

[APM] Add Obs side nav and refactor APM templates #101044

Merged
merged 12 commits into from
Jun 4, 2021
12 changes: 12 additions & 0 deletions x-pack/plugins/apm/common/environment_filter_values.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@ export function getEnvironmentLabel(environment: string) {
return environmentLabels[environment] || environment;
}

export function omitEsFieldValue({
esFieldValue,
value,
text,
}: {
esFieldValue?: string;
value: string;
text: string;
}) {
return { value, text };
}
Copy link
Member Author

@sorenlouv sorenlouv Jun 1, 2021

Choose a reason for hiding this comment

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

This is needed due to a console error introduced in #95903 (comment)

I don't think the console error in itself is an issue but it made it a bit harder to make the changes needed for this PR when the console contained unrelated errors.


export function parseEnvironmentUrlParam(environment: string) {
if (environment === ENVIRONMENT_ALL_VALUE) {
return ENVIRONMENT_ALL;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,16 @@ Then(`should display percentile for page load chart`, () => {
cy.get(pMarkers).eq(3).should('have.text', '95th');
});

Then(`should display chart legend`, () => {
const chartLegend = 'button.echLegendItem__label';
// Then(`should display chart legend`, () => {
// const chartLegend = 'button.echLegendItem__label';

waitForLoadingToFinish();
cy.get('.euiLoadingChart').should('not.exist');
// waitForLoadingToFinish();
// cy.get('.euiLoadingChart').should('not.exist');

cy.get('[data-cy=pageLoadDist]').within(() => {
cy.get(chartLegend, DEFAULT_TIMEOUT).eq(0).should('have.text', 'Overall');
});
});
// cy.get('[data-cy=pageLoadDist]').within(() => {
// cy.get(chartLegend, DEFAULT_TIMEOUT).eq(0).should('have.text', 'Overall');
// });
// });

Then(`should display tooltip on hover`, () => {
cy.get('.euiLoadingChart').should('not.exist');
Expand Down
23 changes: 18 additions & 5 deletions x-pack/plugins/apm/public/application/application.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import React from 'react';
import { act } from '@testing-library/react';
import { createMemoryHistory } from 'history';
import { Observable } from 'rxjs';
Expand All @@ -15,6 +16,7 @@ import { renderApp } from './';
import { disableConsoleWarning } from '../utils/testHelpers';
import { dataPluginMock } from 'src/plugins/data/public/mocks';
import { embeddablePluginMock } from 'src/plugins/embeddable/public/mocks';
import { ApmPluginStartDeps } from '../plugin';

jest.mock('../services/rest/index_pattern', () => ({
createStaticIndexPattern: () => Promise.resolve(undefined),
Expand Down Expand Up @@ -44,6 +46,7 @@ describe('renderApp', () => {
config,
observabilityRuleTypeRegistry,
} = mockApmPluginContextValue;

const plugins = {
licensing: { license$: new Observable() },
triggersActionsUi: { actionTypeRegistry: {}, alertTypeRegistry: {} },
Expand All @@ -56,15 +59,24 @@ describe('renderApp', () => {
},
},
};
const params = {
const appMountParameters = {
element: document.createElement('div'),
history: createMemoryHistory(),
setHeaderActionMenu: () => {},
};

const data = dataPluginMock.createStartContract();
const embeddable = embeddablePluginMock.createStartContract();
const startDeps = {

const pluginsStart = ({
observability: {
navigation: {
registerSections: () => jest.fn(),
PageTemplate: ({ children }: { children: React.ReactNode }) => (
<div>hello worlds {children}</div>
),
},
},
triggersActionsUi: {
actionTypeRegistry: {},
alertTypeRegistry: {},
Expand All @@ -73,7 +85,8 @@ describe('renderApp', () => {
},
data,
embeddable,
};
} as unknown) as ApmPluginStartDeps;

jest.spyOn(window, 'scrollTo').mockReturnValueOnce(undefined);
createCallApmApi((core as unknown) as CoreStart);

Expand All @@ -93,8 +106,8 @@ describe('renderApp', () => {
unmount = renderApp({
coreStart: core as any,
pluginsSetup: plugins as any,
appMountParameters: params as any,
pluginsStart: startDeps as any,
appMountParameters: appMountParameters as any,
pluginsStart,
config,
observabilityRuleTypeRegistry,
});
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/apm/public/application/csmApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
useUiSetting$,
} from '../../../../../src/plugins/kibana_react/public';
import { APMRouteDefinition } from '../application/routes';
import { renderAsRedirectTo } from '../components/app/Main/route_config';
import { ScrollToTopOnPathChange } from '../components/app/Main/ScrollToTopOnPathChange';
import { RumHome, UX_LABEL } from '../components/app/RumDashboard/RumHome';
import { ApmPluginContext } from '../context/apm_plugin/apm_plugin_context';
Expand All @@ -32,6 +31,7 @@ import { createCallApmApi } from '../services/rest/createCallApmApi';
import { px, units } from '../style/variables';
import { createStaticIndexPattern } from '../services/rest/index_pattern';
import { UXActionMenu } from '../components/app/RumDashboard/ActionMenu';
import { redirectTo } from '../components/routing/redirect_to';

const CsmMainContainer = euiStyled.div`
padding: ${px(units.plus)};
Expand All @@ -42,7 +42,7 @@ export const rumRoutes: APMRouteDefinition[] = [
{
exact: true,
path: '/',
render: renderAsRedirectTo('/ux'),
render: redirectTo('/ux'),
breadcrumb: UX_LABEL,
},
];
Expand Down
85 changes: 2 additions & 83 deletions x-pack/plugins/apm/public/application/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,99 +5,18 @@
* 2.0.
*/

import { ApmRoute } from '@elastic/apm-rum-react';
import euiDarkVars from '@elastic/eui/dist/eui_theme_dark.json';
import euiLightVars from '@elastic/eui/dist/eui_theme_light.json';
import React from 'react';
import ReactDOM from 'react-dom';
import { Route, Router, Switch } from 'react-router-dom';
import 'react-vis/dist/style.css';
import { DefaultTheme, ThemeProvider } from 'styled-components';
import type { ObservabilityRuleTypeRegistry } from '../../../observability/public';
import { euiStyled } from '../../../../../src/plugins/kibana_react/common';
import { ConfigSchema } from '../';
import { AppMountParameters, CoreStart } from '../../../../../src/core/public';
import {
KibanaContextProvider,
RedirectAppLinks,
useUiSetting$,
} from '../../../../../src/plugins/kibana_react/public';
import { routes } from '../components/app/Main/route_config';
import { ScrollToTopOnPathChange } from '../components/app/Main/ScrollToTopOnPathChange';
import {
ApmPluginContext,
ApmPluginContextValue,
} from '../context/apm_plugin/apm_plugin_context';
import { LicenseProvider } from '../context/license/license_context';
import { UrlParamsProvider } from '../context/url_params_context/url_params_context';
import { useBreadcrumbs } from '../hooks/use_breadcrumbs';
import { ApmPluginSetupDeps, ApmPluginStartDeps } from '../plugin';
import { createCallApmApi } from '../services/rest/createCallApmApi';
import { createStaticIndexPattern } from '../services/rest/index_pattern';
import { setHelpExtension } from '../setHelpExtension';
import { setReadonlyBadge } from '../updateBadge';
import { AnomalyDetectionJobsContextProvider } from '../context/anomaly_detection_jobs/anomaly_detection_jobs_context';

const MainContainer = euiStyled.div`
sorenlouv marked this conversation as resolved.
Show resolved Hide resolved
height: 100%;
`;

function App() {
const [darkMode] = useUiSetting$<boolean>('theme:darkMode');

useBreadcrumbs(routes);

return (
<ThemeProvider
theme={(outerTheme?: DefaultTheme) => ({
...outerTheme,
eui: darkMode ? euiDarkVars : euiLightVars,
darkMode,
})}
>
<MainContainer data-test-subj="apmMainContainer" role="main">
<Route component={ScrollToTopOnPathChange} />
<Switch>
{routes.map((route, i) => (
<ApmRoute key={i} {...route} />
))}
</Switch>
</MainContainer>
</ThemeProvider>
);
}

export function ApmAppRoot({
apmPluginContextValue,
startDeps,
}: {
apmPluginContextValue: ApmPluginContextValue;
startDeps: ApmPluginStartDeps;
}) {
const { appMountParameters, core } = apmPluginContextValue;
const { history } = appMountParameters;
const i18nCore = core.i18n;

return (
<RedirectAppLinks application={core.application}>
<ApmPluginContext.Provider value={apmPluginContextValue}>
<KibanaContextProvider services={{ ...core, ...startDeps }}>
<i18nCore.Context>
<Router history={history}>
<UrlParamsProvider>
<LicenseProvider>
<AnomalyDetectionJobsContextProvider>
<App />
</AnomalyDetectionJobsContextProvider>
</LicenseProvider>
</UrlParamsProvider>
</Router>
</i18nCore.Context>
</KibanaContextProvider>
</ApmPluginContext.Provider>
</RedirectAppLinks>
);
}
import { ApmAppRoot } from '../components/routing/app_root';
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved all of this to the routing section (which contains routing and templating)


/**
* This module is rendered asynchronously in the Kibana platform.
Expand Down Expand Up @@ -141,7 +60,7 @@ export const renderApp = ({
ReactDOM.render(
<ApmAppRoot
apmPluginContextValue={apmPluginContextValue}
startDeps={pluginsStart}
pluginsStart={pluginsStart}
/>,
element
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,17 @@ import { useParams } from 'react-router-dom';
import { useKibana } from '../../../../../../../src/plugins/kibana_react/public';
import { AlertType } from '../../../../common/alert_types';
import { getInitialAlertValues } from '../get_initial_alert_values';
import { TriggersAndActionsUIPublicPluginStart } from '../../../../../triggers_actions_ui/public';
import { ApmPluginStartDeps } from '../../../plugin';
interface Props {
addFlyoutVisible: boolean;
setAddFlyoutVisibility: React.Dispatch<React.SetStateAction<boolean>>;
alertType: AlertType | null;
}

interface KibanaDeps {
triggersActionsUi: TriggersAndActionsUIPublicPluginStart;
}

export function AlertingFlyout(props: Props) {
const { addFlyoutVisible, setAddFlyoutVisibility, alertType } = props;
const { serviceName } = useParams<{ serviceName?: string }>();
const {
services: { triggersActionsUi },
} = useKibana<KibanaDeps>();

const { services } = useKibana<ApmPluginStartDeps>();
const initialValues = getInitialAlertValues(alertType, serviceName);

const onCloseAddFlyout = useCallback(() => setAddFlyoutVisibility(false), [
Expand All @@ -37,15 +30,15 @@ export function AlertingFlyout(props: Props) {
const addAlertFlyout = useMemo(
() =>
alertType &&
triggersActionsUi.getAddAlertFlyout({
services.triggersActionsUi.getAddAlertFlyout({
consumer: 'apm',
onClose: onCloseAddFlyout,
alertTypeId: alertType,
canChangeTrigger: false,
initialValues,
}),
/* eslint-disable-next-line react-hooks/exhaustive-deps */
[alertType, onCloseAddFlyout, triggersActionsUi]
[alertType, onCloseAddFlyout, services.triggersActionsUi]
);
return <>{addFlyoutVisible && addAlertFlyout}</>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import { shallow } from 'enzyme';
import { Location } from 'history';
import React from 'react';
import { mockMoment } from '../../../../utils/testHelpers';
import { DetailView } from './index';
Expand All @@ -19,11 +18,7 @@ describe('DetailView', () => {

it('should render empty state', () => {
const wrapper = shallow(
<DetailView
errorGroup={{} as any}
urlParams={{}}
location={{} as Location}
/>
<DetailView errorGroup={{} as any} urlParams={{}} />
);
expect(wrapper.isEmptyRender()).toBe(true);
});
Expand All @@ -46,11 +41,7 @@ describe('DetailView', () => {
};

const wrapper = shallow(
<DetailView
errorGroup={errorGroup}
urlParams={{}}
location={{} as Location}
/>
<DetailView errorGroup={errorGroup} urlParams={{}} />
).find('DiscoverErrorLink');

expect(wrapper.exists()).toBe(true);
Expand All @@ -69,11 +60,7 @@ describe('DetailView', () => {
transaction: undefined,
};
const wrapper = shallow(
<DetailView
errorGroup={errorGroup}
urlParams={{}}
location={{} as Location}
/>
<DetailView errorGroup={errorGroup} urlParams={{}} />
).find('Summary');

expect(wrapper.exists()).toBe(true);
Expand All @@ -93,11 +80,7 @@ describe('DetailView', () => {
} as any,
};
const wrapper = shallow(
<DetailView
errorGroup={errorGroup}
urlParams={{}}
location={{} as Location}
/>
<DetailView errorGroup={errorGroup} urlParams={{}} />
).find('EuiTabs');

expect(wrapper.exists()).toBe(true);
Expand All @@ -117,11 +100,7 @@ describe('DetailView', () => {
} as any,
};
const wrapper = shallow(
<DetailView
errorGroup={errorGroup}
urlParams={{}}
location={{} as Location}
/>
<DetailView errorGroup={errorGroup} urlParams={{}} />
).find('TabContent');

expect(wrapper.exists()).toBe(true);
Expand All @@ -145,13 +124,7 @@ describe('DetailView', () => {
} as any,
};
expect(() =>
shallow(
<DetailView
errorGroup={errorGroup}
urlParams={{}}
location={{} as Location}
/>
)
shallow(<DetailView errorGroup={errorGroup} urlParams={{}} />)
).not.toThrowError();
});
});
Loading