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

Migrate status page app to core #72017

Merged
merged 26 commits into from
Jul 23, 2020
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ba9e213
move http.anonymousPaths.register('/status'); logic into core, remove…
pgayvallet Jul 16, 2020
5964ffc
Merge remote-tracking branch 'upstream/master' into kbn-67979-status-…
pgayvallet Jul 16, 2020
f63d8d2
move status_page to core & migrate lib
pgayvallet Jul 16, 2020
dcc1f0a
migrate the status_app components to TS/KP APIs
pgayvallet Jul 16, 2020
6b42c45
update rendering snapshots
pgayvallet Jul 16, 2020
854211e
use import type syntax
pgayvallet Jul 16, 2020
c4dbd19
moves `/status` server-side route to core
pgayvallet Jul 16, 2020
7f927f7
fix route registration
pgayvallet Jul 16, 2020
c857b74
Merge remote-tracking branch 'upstream/master' into kbn-67979-status-…
pgayvallet Jul 17, 2020
8a2f98e
update generated file
pgayvallet Jul 17, 2020
0a7b243
change statusPage i18n prefix
pgayvallet Jul 20, 2020
d65c125
Merge remote-tracking branch 'upstream/master' into kbn-67979-status-…
pgayvallet Jul 20, 2020
7bdc3c2
(temporary) try to restore legacy plugin to check behavior
pgayvallet Jul 20, 2020
e91d5b7
Merge remote-tracking branch 'upstream/master' into kbn-67979-status-…
pgayvallet Jul 21, 2020
b7bc230
add some FTR tests
pgayvallet Jul 21, 2020
4beb5d3
do not import whole lodash
pgayvallet Jul 21, 2020
66337af
update snapshots
pgayvallet Jul 21, 2020
46c5adf
Merge remote-tracking branch 'upstream/master' into kbn-67979-status-…
pgayvallet Jul 22, 2020
716dee7
review comments
pgayvallet Jul 22, 2020
92924b7
improve / clean component unit tests
pgayvallet Jul 22, 2020
7352ad9
change url for legacy status app
pgayvallet Jul 22, 2020
3a3e2e9
set status app as chromeless
pgayvallet Jul 22, 2020
5b8719e
fix FTR test due to chromeless app
pgayvallet Jul 22, 2020
1a0c2ae
fix typings
pgayvallet Jul 22, 2020
920f52b
add unit test for CoreApp /status registration
pgayvallet Jul 22, 2020
71f4273
Merge remote-tracking branch 'upstream/master' into kbn-67979-status-…
pgayvallet Jul 23, 2020
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
5 changes: 0 additions & 5 deletions docs/developer/architecture/code-exploration.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,6 @@ WARNING: Missing README.
Replaces the legacy ui/share module for registering share context menus.


- {kib-repo}blob/{branch}/src/plugins/status_page[statusPage]

WARNING: Missing README.


- {kib-repo}blob/{branch}/src/plugins/telemetry/README.md[telemetry]

Telemetry allows Kibana features to have usage tracked in the wild. The general term "telemetry" refers to multiple things:
Expand Down
31 changes: 24 additions & 7 deletions src/core/public/core_app/core_app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,19 @@ import {
AppNavLinkStatus,
AppMountParameters,
} from '../application';
import { HttpSetup, HttpStart } from '../http';
import { CoreContext } from '../core_system';
import { renderApp, setupUrlOverflowDetection } from './errors';
import { NotificationsStart } from '../notifications';
import { IUiSettingsClient } from '../ui_settings';
import type { HttpSetup, HttpStart } from '../http';
import type { CoreContext } from '../core_system';
import type { NotificationsSetup, NotificationsStart } from '../notifications';
import type { IUiSettingsClient } from '../ui_settings';
import type { InjectedMetadataSetup } from '../injected_metadata';
import { renderApp as renderErrorApp, setupUrlOverflowDetection } from './errors';
import { renderApp as renderStatusApp } from './status';

interface SetupDeps {
application: InternalApplicationSetup;
http: HttpSetup;
injectedMetadata: InjectedMetadataSetup;
notifications: NotificationsSetup;
}

interface StartDeps {
Expand All @@ -47,7 +51,7 @@ export class CoreApp {

constructor(private readonly coreContext: CoreContext) {}

public setup({ http, application }: SetupDeps) {
public setup({ http, application, injectedMetadata, notifications }: SetupDeps) {
application.register(this.coreContext.coreId, {
id: 'error',
title: 'App Error',
Expand All @@ -56,7 +60,20 @@ export class CoreApp {
// Do not use an async import here in order to ensure that network failures
// cannot prevent the error UI from displaying. This UI is tiny so an async
// import here is probably not useful anyways.
return renderApp(params, { basePath: http.basePath });
return renderErrorApp(params, { basePath: http.basePath });
},
});

if (injectedMetadata.getAnonymousStatusPage()) {
http.anonymousPaths.register('/status');
Copy link
Contributor

Choose a reason for hiding this comment

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

You cannot navigate to the other Kibana pages from other anonymous pages without reloading. It creates a bug when status.allowAnonymous: true and you logged-in user lands on the /status page and navigates to other pages. result: SecurityNavControl is not rendered.
2020-07-22_10-50-55
As a workaround, we could make status page chromeless to enforce page reloading.
Is it possible to land on the /status page from a link in Kibana UI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You cannot navigate to the other Kibana pages from other anonymous pages without reloading. It creates a bug when status.allowAnonymous: true and you logged-in user lands on the /status page and navigates to other pages. result: SecurityNavControl is not rendered.

Hum. This indeed is an issue. We really need to have a way to know about anonymous vs logged-in apps from core, to be able to automatically performs that kind of reload when using navigateToApp between anon/logged or logged/anon apps. Do you know if we have an issue for that?

As a workaround, we could make status page chromeless to enforce page reloading.

chromeless apps do not really force page reloading, but I guess hiding the chrome navigation will remove any way for the user to navigate away from the status page using the UI. do you think it would be sufficient?

Is it possible to land on the /status page from a link in Kibana UI?

Not that I'm aware of. The app is hidden and there is no link to it anywhere in chrome.

Copy link
Contributor

Choose a reason for hiding this comment

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

but I guess hiding the chrome navigation will remove any way for the user to navigate away from the status page using the UI.

that was my reasoning as well. I think it's okay since the user navigates to the /status page manually.

Hum. This indeed is an issue. We really need to have a way to know about anonymous vs logged-in apps from core, to be able to automatically performs that kind of reload when using navigateToApp between anon/logged or logged/anon apps. Do you know if we have an issue for that?

I'm not aware of any.

}
application.register(this.coreContext.coreId, {
id: 'status',
title: 'Server Status',
appRoute: '/status',
navLinkStatus: AppNavLinkStatus.hidden,
mount(params: AppMountParameters) {
return renderStatusApp(params, { http, notifications });
Comment on lines +76 to +77
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 choose to not use async import for the error app, so I think it makes sense to do the same for the status page.

},
});
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
* under the License.
*/

import { PluginInitializer } from 'kibana/public';
import { StatusPagePlugin, StatusPagePluginSetup, StatusPagePluginStart } from './plugin';

export const plugin: PluginInitializer<StatusPagePluginSetup, StatusPagePluginStart> = () =>
new StatusPagePlugin();
export { MetricTile, MetricTiles } from './metric_tiles';
export { ServerStatus } from './server_status';
export { StatusTable } from './status_table';
Original file line number Diff line number Diff line change
Expand Up @@ -17,53 +17,43 @@
* under the License.
*/

import formatNumber from '../lib/format_number';
import React, { Component } from 'react';
import { Metric as MetricPropType } from '../lib/prop_types';
import PropTypes from 'prop-types';
import React, { FunctionComponent } from 'react';
import { EuiFlexGrid, EuiFlexItem, EuiCard } from '@elastic/eui';
import { formatNumber, Metric } from '../lib';

/*
Displays a metric with the correct format.
*/
export class MetricTile extends Component {
static propTypes = {
metric: MetricPropType.isRequired,
};

formattedMetric() {
const { value, type } = this.props.metric;

const metrics = [].concat(value);
return metrics
.map(function (metric) {
return formatNumber(metric, type);
})
.join(', ');
}

render() {
const { name } = this.props.metric;

return <EuiCard layout="horizontal" title={this.formattedMetric()} description={name} />;
}
}
* Displays a metric with the correct format.
*/
export const MetricTile: FunctionComponent<{ metric: Metric }> = ({ metric }) => {
const { name } = metric;
return (
<EuiCard
data-test-subj={`serverMetric-${formatMetricId(metric)}`}
layout="horizontal"
title={formatMetric(metric)}
description={name}
/>
);
};

/*
Wrapper component that simply maps each metric to MetricTile inside a FlexGroup
*/
const MetricTiles = ({ metrics }) => (
* Wrapper component that simply maps each metric to MetricTile inside a FlexGroup
*/
export const MetricTiles: FunctionComponent<{ metrics: Metric[] }> = ({ metrics }) => (
<EuiFlexGrid columns={3}>
{metrics.map((metric) => (
<EuiFlexItem key={metric.name}>
<EuiFlexItem key={metric.name} data-test-subj="serverMetric">
<MetricTile metric={metric} />
</EuiFlexItem>
))}
</EuiFlexGrid>
);

MetricTiles.propTypes = {
metrics: PropTypes.arrayOf(MetricPropType).isRequired,
const formatMetric = ({ value, type }: Metric) => {
const metrics = Array.isArray(value) ? value : [value];
return metrics.map((metric) => formatNumber(metric, type)).join(', ');
};

export default MetricTiles;
const formatMetricId = ({ name }: Metric) => {
return name.toLowerCase().replace(' ', '');
pgayvallet marked this conversation as resolved.
Show resolved Hide resolved
};
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,17 @@

import React from 'react';
import { shallow } from 'enzyme';
import ServerStatus from './server_status';
import { ServerStatus } from './server_status';

const STATE = {
id: 'green',
title: 'Green',
uiColor: 'secondary',
state: 'green',
message: '',
};

test('render', () => {
const component = shallow(<ServerStatus serverState={STATE} name="My Computer" />);
expect(component).toMatchSnapshot(); // eslint-disable-line
expect(component).toMatchSnapshot();
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,34 @@
* under the License.
*/

import React from 'react';
import PropTypes from 'prop-types';
import { State as StatePropType } from '../lib/prop_types';
import React, { FunctionComponent } from 'react';
import { EuiText, EuiFlexGroup, EuiFlexItem, EuiTitle, EuiBadge } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import type { FormattedStatus } from '../lib';

const ServerState = ({ name, serverState }) => (
interface ServerStateProps {
name: string;
serverState: FormattedStatus['state'];
}

export const ServerStatus: FunctionComponent<ServerStateProps> = ({ name, serverState }) => (
<EuiFlexGroup alignItems="center" justifyContent="spaceBetween" style={{ flexGrow: 0 }}>
<EuiFlexItem grow={false}>
<EuiTitle>
<h2>
<h2 data-test-subj="serverStatusTitle">
<FormattedMessage
id="statusPage.serverStatus.statusTitle"
id="core.statusPage.serverStatus.statusTitle"
defaultMessage="Kibana status is {kibanaStatus}"
values={{
kibanaStatus: <EuiBadge color={serverState.uiColor}>{serverState.title}</EuiBadge>,
kibanaStatus: (
<EuiBadge
data-test-subj="serverStatusTitleBadge"
color={serverState.uiColor}
aria-label={serverState.title}
>
{serverState.title}
</EuiBadge>
),
}}
/>
</h2>
Expand All @@ -45,10 +57,3 @@ const ServerState = ({ name, serverState }) => (
</EuiFlexItem>
</EuiFlexGroup>
);

ServerState.propTypes = {
name: PropTypes.string.isRequired,
serverState: StatePropType.isRequired,
};

export default ServerState;
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@

import React from 'react';
import { shallow } from 'enzyme';
import StatusTable from './status_table';
import { StatusTable } from './status_table';

const STATE = {
id: 'green',
uiColor: 'secondary',
message: 'Ready',
title: 'green',
};

test('render', () => {
Expand Down
67 changes: 67 additions & 0 deletions src/core/public/core_app/status/components/status_table.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React, { FunctionComponent } from 'react';
import { EuiBasicTable, EuiIcon } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import type { FormattedStatus } from '../lib';

interface StatusTableProps {
statuses?: FormattedStatus[];
}

const tableColumns = [
{
field: 'state',
name: '',
render: (state: FormattedStatus['state']) => (
<EuiIcon type="dot" aria-hidden color={state.uiColor} />
),
width: '32px',
},
{
field: 'id',
name: i18n.translate('core.statusPage.statusTable.columns.idHeader', {
defaultMessage: 'ID',
}),
},
{
field: 'state',
name: i18n.translate('core.statusPage.statusTable.columns.statusHeader', {
defaultMessage: 'Status',
}),
render: (state: FormattedStatus['state']) => <span>{state.message}</span>,
},
];

export const StatusTable: FunctionComponent<StatusTableProps> = ({ statuses }) => {
if (!statuses) {
return null;
}
return (
<EuiBasicTable<FormattedStatus>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL you can specify generics of react components in tsx.

columns={tableColumns}
items={statuses}
rowProps={({ state }) => ({
className: `status-table-row-${state.uiColor}`,
})}
data-test-subj="statusBreakdown"
/>
);
};
20 changes: 20 additions & 0 deletions src/core/public/core_app/status/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export { renderApp } from './render_app';
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import formatNumber from './format_number';
import { formatNumber } from './format_number';

describe('format byte', () => {
test('zero', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import numeral from '@elastic/numeral';

export default function formatNumber(num, which) {
export function formatNumber(num: number, which?: string) {
pgayvallet marked this conversation as resolved.
Show resolved Hide resolved
let format = '0.00';
let postfix = '';
switch (which) {
Expand Down
21 changes: 21 additions & 0 deletions src/core/public/core_app/status/lib/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export { formatNumber } from './format_number';
export { loadStatus, Metric, FormattedStatus, ProcessedServerResponse } from './load_status';
Loading