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

[Monitoring] Support for unlinked deployments #28278

Merged
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
9efe952
Unlinked deployment working for beats
chrisronline Jan 7, 2019
3534720
Use better constant
chrisronline Jan 7, 2019
f5b3086
Merge remote-tracking branch 'elastic/master' into monitoring/unlinke…
chrisronline Jan 8, 2019
fa77948
Show N/A for license
chrisronline Jan 8, 2019
5fda7e1
Rename to Unlinked Cluster
chrisronline Jan 9, 2019
02108f2
Use callout to mention unlinked cluster
chrisronline Jan 9, 2019
5fb7138
PR feedback
chrisronline Jan 9, 2019
4f3a64d
Use fragment
chrisronline Jan 9, 2019
7abe26c
Speed up the query by using terminate_after
chrisronline Jan 9, 2019
61dff35
Handle failures more defensively
chrisronline Jan 9, 2019
d5e2263
Merge remote-tracking branch 'elastic/master' into monitoring/unlinke…
chrisronline Jan 9, 2019
3590d8a
Remove unnecessary msearch
chrisronline Jan 9, 2019
dbc878b
PR feedback
chrisronline Jan 10, 2019
220bfe3
PR feedback and a bit of light refactor
chrisronline Jan 11, 2019
694921c
Updated text
chrisronline Jan 11, 2019
136d23a
Merge remote-tracking branch 'elastic/master' into monitoring/unlinke…
chrisronline Jan 11, 2019
4b168ed
Merge remote-tracking branch 'elastic/master' into monitoring/unlinke…
chrisronline Jan 15, 2019
83ac0bf
Merge remote-tracking branch 'elastic/master' into monitoring/unlinke…
chrisronline Jan 15, 2019
d5fa37f
Add api integration tests
chrisronline Jan 15, 2019
f2ee66c
Localize call out
chrisronline Jan 15, 2019
cc99fe8
Update loc pattern
chrisronline Jan 15, 2019
0e2e760
Fix improper i18n.translate usage
chrisronline Jan 16, 2019
2d6cf9e
Merge remote-tracking branch 'elastic/master' into monitoring/unlinke…
chrisronline Jan 16, 2019
f35eed3
Revert "Fix improper i18n.translate usage"
chrisronline Jan 16, 2019
0849281
Revert "Update loc pattern"
chrisronline Jan 16, 2019
9a87135
Ensure the unlinked deployment cluster counts as a valid cluster
chrisronline Jan 16, 2019
cbe32b7
Sometimes, you miss the smallest things
chrisronline Jan 16, 2019
c3f1dfc
Ensure the unlinked cluster is supported, in that users can click the…
chrisronline Jan 17, 2019
8aba3cd
Update tests
chrisronline Jan 17, 2019
753d9d8
PR feedback. Simplifying the flag supported code and adding more tests
chrisronline Jan 24, 2019
5c426cd
Update naming
chrisronline Jan 24, 2019
12d7337
Rename to Standalone Cluster
chrisronline Jan 24, 2019
1e1d1ca
Remove unnecessary file
chrisronline Jan 24, 2019
d9cc7df
Move logic for setting isSupported to exclusively in flag supported c…
chrisronline Jan 24, 2019
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
2 changes: 2 additions & 0 deletions x-pack/plugins/monitoring/common/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,5 @@ export const DEBOUNCE_FAST_MS = 10; // roughly how long it takes to render a fra
* Configuration key for setting the email address used for cluster alert notifications.
*/
export const CLUSTER_ALERTS_ADDRESS_CONFIG_KEY = 'cluster_alerts.email_notifications.email_address';

export const UNLINKED_DEPLOYMENT_CLUSTER_UUID = '__unlinked_deployment__';
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* 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.
*/

export { Listing } from './listing';
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,35 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { Fragment } from 'react';
import { render } from 'react-dom';
import { capitalize, partial } from 'lodash';
import React, { Fragment, Component } from 'react';
import chrome from 'ui/chrome';
import moment from 'moment';
import numeral from '@elastic/numeral';
import { uiModules } from 'ui/modules';
import chrome from 'ui/chrome';
import { capitalize, partial } from 'lodash';
import {
EuiHealth,
EuiLink,
EuiPage,
EuiPageBody,
EuiPageContent,
EuiCallOut,
EuiSpacer,
EuiIcon
} from '@elastic/eui';
import { toastNotifications } from 'ui/notify';
import { EuiMonitoringTable } from 'plugins/monitoring/components/table';
import { Tooltip } from 'plugins/monitoring/components/tooltip';
import { AlertsIndicator } from 'plugins/monitoring/components/cluster/listing/alerts_indicator';
import { I18nProvider, FormattedMessage } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';
import { UNLINKED_DEPLOYMENT_CLUSTER_UUID } from '../../../../common/constants';

const IsClusterSupported = ({ isSupported, children }) => {
return isSupported ? children : '-';
};

const UNLINKED_DEPLOYMENT_STORAGE_KEY = 'viewedUnlinkedDeployments';

/*
* This checks if alerts feature is supported via monitoring cluster
* license. If the alerts feature is not supported because the prod cluster
Expand Down Expand Up @@ -195,6 +198,17 @@ const getColumns = (
sortable: true,
render: (licenseType, cluster) => {
const license = cluster.license;

if (!licenseType) {
return (
<div>
<div className="monTableCell__clusterCellLiscense">
N/A
</div>
</div>
);
}

if (license) {
const licenseExpiry = () => {
if (license.expiry_date_in_millis < moment().valueOf()) {
Expand Down Expand Up @@ -342,72 +356,112 @@ const handleClickInvalidLicense = (scope, clusterName) => {
});
};

const uiModule = uiModules.get('monitoring/directives', []);
uiModule.directive('monitoringClusterListing', ($injector) => {
return {
restrict: 'E',
scope: {
clusters: '=',
sorting: '=',
filterText: '=',
paginationSettings: '=pagination',
onTableChange: '=',
},
link(scope, $el) {
const globalState = $injector.get('globalState');
const kbnUrl = $injector.get('kbnUrl');
const showLicenseExpiration = $injector.get('showLicenseExpiration');
export class Listing extends Component {
constructor(props) {
super(props);
this.state = {
[UNLINKED_DEPLOYMENT_STORAGE_KEY]: false,
};
}

const _changeCluster = partial(changeCluster, scope, globalState, kbnUrl);
const _handleClickIncompatibleLicense = partial(handleClickIncompatibleLicense, scope);
const _handleClickInvalidLicense = partial(handleClickInvalidLicense, scope);
renderUnlinkedDeployment(changeCluster, storage) {
if (storage.get(UNLINKED_DEPLOYMENT_STORAGE_KEY)) {
return null;
}

const { sorting, pagination, onTableChange } = scope;
return (
<div>
<EuiCallOut
color="warning"
title={i18n.translate('xpack.monitoring.cluster.listing.unlinkedDeploymentCallOutTitle', {
defaultMessage: 'It looks like you have instances that aren\'t connected to an Elasticsearch cluster.'
})}
iconType="link"
>
<p>
<EuiLink
onClick={() => changeCluster(UNLINKED_DEPLOYMENT_CLUSTER_UUID)}
data-test-subj="unlinkedDeploymentLink"
>
<FormattedMessage
id="xpack.monitoring.cluster.listing.unlinkedDeploymentCallOutLink"
defaultMessage="View these instances."
/>
</EuiLink>
&nbsp;
<FormattedMessage
id="xpack.monitoring.cluster.listing.unlinkedDeploymentCallOutText"
defaultMessage="Or, click Unlinked Cluster in the table below"
/>
</p>
<p>
<EuiLink onClick={() => {
storage.set(UNLINKED_DEPLOYMENT_STORAGE_KEY, true);
this.setState({ [UNLINKED_DEPLOYMENT_STORAGE_KEY]: true });
}}
>
<EuiIcon type="cross"/>
&nbsp;
<FormattedMessage
id="xpack.monitoring.cluster.listing.unlinkedDeploymentCallOutDismiss"
defaultMessage="Dismiss"
/>
</EuiLink>
</p>
</EuiCallOut>
<EuiSpacer/>
</div>
);
}

scope.$watch('clusters', (clusters = []) => {
const clusterTable = (
<I18nProvider>
<EuiPage>
<EuiPageBody>
<EuiPageContent>
<EuiMonitoringTable
className="clusterTable"
rows={clusters}
columns={getColumns(
showLicenseExpiration,
_changeCluster,
_handleClickIncompatibleLicense,
_handleClickInvalidLicense
)}
rowProps={item => {
return {
'data-test-subj': `clusterRow_${item.cluster_uuid}`
};
}}
sorting={{
...sorting,
sort: {
...sorting.sort,
field: 'cluster_name'
}
}}
pagination={pagination}
search={{
box: {
incremental: true,
placeholder: scope.filterText
},
}}
onTableChange={onTableChange}
/>
</EuiPageContent>
</EuiPageBody>
</EuiPage>
</I18nProvider>
);
render(clusterTable, $el[0]);
});
render() {
const { angular, clusters, sorting, pagination, onTableChange } = this.props;

}
};
});
const _changeCluster = partial(changeCluster, angular.scope, angular.globalState, angular.kbnUrl);
const _handleClickIncompatibleLicense = partial(handleClickIncompatibleLicense, angular.scope);
const _handleClickInvalidLicense = partial(handleClickInvalidLicense, angular.scope);
const hasUnlinkedDeployment = !!clusters.find(cluster => cluster.cluster_uuid === UNLINKED_DEPLOYMENT_CLUSTER_UUID);

return (
<I18nProvider>
<EuiPage>
<EuiPageBody>
<EuiPageContent>
{hasUnlinkedDeployment ? this.renderUnlinkedDeployment(_changeCluster, angular.storage) : null}
<EuiMonitoringTable
className="clusterTable"
rows={clusters}
columns={getColumns(
angular.showLicenseExpiration,
_changeCluster,
_handleClickIncompatibleLicense,
_handleClickInvalidLicense
)}
rowProps={item => {
return {
'data-test-subj': `clusterRow_${item.cluster_uuid}`
};
}}
sorting={{
...sorting,
sort: {
...sorting.sort,
field: 'cluster_name'
}
}}
pagination={pagination}
search={{
box: {
incremental: true,
placeholder: angular.scope.filterText
},
}}
onTableChange={onTableChange}
/>
</EuiPageContent>
</EuiPageBody>
</EuiPage>
</I18nProvider>
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React from 'react';
import React, { Fragment } from 'react';
import { ElasticsearchPanel } from './elasticsearch_panel';
import { KibanaPanel } from './kibana_panel';
import { LogstashPanel } from './logstash_panel';
Expand All @@ -13,23 +13,32 @@ import { BeatsPanel } from './beats_panel';

import { EuiPage, EuiPageBody } from '@elastic/eui';
import { ApmPanel } from './apm_panel';
import { UNLINKED_DEPLOYMENT_CLUSTER_UUID } from '../../../../common/constants';

export function Overview(props) {
const isFromUnlinkedDeployment = props.cluster.cluster_uuid === UNLINKED_DEPLOYMENT_CLUSTER_UUID;
Copy link
Member

Choose a reason for hiding this comment

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

This logic appears to be appropriately reused in a bunch of places. Is the thought here that used a predefined constant is better than undefined because it ends up in the global scope, and we don't want to add logic to the global scope handling?

(I'm asking to better understand your reasoning rather than "this is not right!")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup basically. I didn't want to use an empty string for cluster_uuid and I felt something like undefined or null misrepresented what's going on.

Copy link
Member

Choose a reason for hiding this comment

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

I felt something like undefined or null misrepresented what's going on.

But looking at the name, cluster_uuid, I feel like that's exactly what it means though?

I do think one problem with using undefined/null in the short term is that we redirect to the cluster listing page if multiple clusters are detected and it's not supplied / found. I wouldn't want you to change that behavior in this PR, but I do think we should stop redirecting automatically from the cluster listing page, which would then open up using it for that purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have any concerns with using this constant as the identifier for this? While I see your side of things, I feel like null or undefined, while technically true for this, make it seem like some sort of error scenario/state where it's not really for the user. I feel pretty good about this name, though we should change it to Unlinked Cluster

Copy link
Member

Choose a reason for hiding this comment

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

I don't see any problem with it. cluster_uuid is a fixed length identifier in ES, so you shouldn't have any unlikely collision opportunity. The constant's name is great, although I agree with the rename until "Deployment" is more widely applied.


return (
<EuiPage>
<EuiPageBody>
<AlertsPanel alerts={props.cluster.alerts} changeUrl={props.changeUrl} />

<ElasticsearchPanel
{...props.cluster.elasticsearch}
version={props.cluster.version}
ml={props.cluster.ml}
changeUrl={props.changeUrl}
license={props.cluster.license}
showLicenseExpiration={props.showLicenseExpiration}
/>

<KibanaPanel {...props.cluster.kibana} changeUrl={props.changeUrl} />
{ !isFromUnlinkedDeployment ?
(
<Fragment>
<ElasticsearchPanel
{...props.cluster.elasticsearch}
version={props.cluster.version}
ml={props.cluster.ml}
changeUrl={props.changeUrl}
license={props.cluster.license}
showLicenseExpiration={props.showLicenseExpiration}
/>
<KibanaPanel {...props.cluster.kibana} changeUrl={props.changeUrl} />
</Fragment>
)
: null
}

<LogstashPanel {...props.cluster.logstash} changeUrl={props.changeUrl} />

Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/monitoring/public/directives/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import './main';
import './chart';
import './sparkline';
import './cluster/listing';
import './elasticsearch/cluster_status';
import './elasticsearch/index_summary';
import './elasticsearch/node_summary';
Expand Down
16 changes: 14 additions & 2 deletions x-pack/plugins/monitoring/public/services/clusters.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,18 @@
import { uiModules } from 'ui/modules';
import { ajaxErrorHandlersProvider } from 'plugins/monitoring/lib/ajax_error_handler';
import { timefilter } from 'ui/timefilter';
import { UNLINKED_DEPLOYMENT_CLUSTER_UUID } from '../../common/constants';

function formatClusters(clusters) {
return clusters.map(formatCluster);
}

function formatCluster(cluster) {
if (cluster.cluster_uuid === UNLINKED_DEPLOYMENT_CLUSTER_UUID) {
cluster.cluster_name = 'Unlinked Cluster';
}
return cluster;
}

const uiModule = uiModules.get('monitoring/clusters');
uiModule.service('monitoringClusters', ($injector) => {
Expand All @@ -30,9 +42,9 @@ uiModule.service('monitoringClusters', ($injector) => {
.then(response => response.data)
.then(data => {
if (clusterUuid) {
return data[0]; // return single cluster
return formatCluster(data[0]); // return single cluster
}
return data; // return set of clusters
return formatClusters(data); // return set of clusters
})
.catch(err => {
const Private = $injector.get('Private');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
<monitoring-main name="listing">
<monitoring-cluster-listing
pagination-settings="clusters.pagination"
sorting="clusters.sorting"
on-table-change="clusters.onTableChange"
clusters="clusters.data"
></monitoring-cluster-listing>
<div id="monitoringClusterListingApp"></div>
</monitoring-main>
Loading