Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Migrate Alerting Kibana pluign to the new Kibana Platform #209

Merged
merged 51 commits into from
Nov 21, 2020
Merged
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
5f28e97
Bump version to 1.11.0.2
Oct 24, 2020
6dd8dee
wip: migrating to new platform
Nov 8, 2020
d65ef1a
wip: migrating to new platform, with API 404 server error
Nov 11, 2020
6cb4717
Fix get alerts, get monitors and create monitor APIs
Nov 15, 2020
12bd463
Fix update monitor api
Nov 15, 2020
8fb49bd
Temporarily add target folder into .gitignore
Nov 15, 2020
6351a01
Fix api: execute monitor
Nov 15, 2020
37ca8d7
Fix toasts and execute monitor api, change some variable names
Nov 15, 2020
755b46e
Override headers when setting up the server-side plugin in new platform
Nov 15, 2020
3fde6e2
reformat the way of creating cluster client in server-side to be simi…
Nov 16, 2020
a34d7fb
Fix Destination APIs, and correct the data path of http responses
Nov 16, 2020
fd0a9b7
Fix Destination APIs: correct some validation of the routes in server…
Nov 16, 2020
71db812
Fix AnomalyDetector APIs
Nov 16, 2020
32e202f
Fix ack alert API
Nov 16, 2020
9acf695
Fix the unit tests for getting the data from http response in the rig…
Nov 16, 2020
c33e819
Remove the temporary build script
Nov 16, 2020
361394a
Fix unit tests in CreateMonitor and Breadcrumbs
Nov 16, 2020
2ab8bd4
Fix unit tests in CreateMonitor and MonitorIndex and validation of Cr…
Nov 16, 2020
0e81849
Fix unit tests: correct the data path of http response in MonitorIndex
Nov 16, 2020
045e51f
Fix unit tests and add mocks for core services
Nov 16, 2020
b54f75f
remove a console.log()
Nov 17, 2020
148f177
restore content of package.json
Nov 17, 2020
f135af1
clean up: remove comment out codes
Nov 17, 2020
618b447
update kibana-plugin-helpers to adapt with new platform
Nov 17, 2020
2012070
fix toast notifications escecially in email pages, and correct 2 apis…
Nov 17, 2020
71b5731
set breadcrumb for alerting plugin and import the css style for react…
Nov 17, 2020
d61d0e2
remove the comment out codes in app.js
Nov 17, 2020
ee4b086
remove 2 console.log() that used for testing
Nov 17, 2020
841b4b2
refactor the core context and related props
Nov 17, 2020
f3f5aaa
refactor the core context and related props
Nov 17, 2020
0f1b13b
cherry-pick PR#203 and clean comment out codes in server/services
Nov 17, 2020
05271e6
Pass in isEmailAllowed for Email modals on CreateDestination page (#204)
qreshi Oct 26, 2020
9c74a64
Stop calling alerting indices directly (#203)
lezzago Oct 25, 2020
46b9c91
cherry-pick PR#203
Nov 17, 2020
efca040
Merge remote-tracking branch 'upstream/master' into new-platform-7.9.1
Nov 17, 2020
a58666b
cherry-pick PR#203
Nov 17, 2020
e5956a9
correct propTypes check in Email.js
Nov 17, 2020
ace80f5
add the icon property back but comment it out in public/plugin.js
Nov 18, 2020
fa65396
Add copyright header for the newly created files
Nov 18, 2020
28d1288
Add a space in the comments
Nov 18, 2020
9ca0480
clean comment out codes
Nov 18, 2020
9cb2029
remove an unused import in AnomalyDetectorService
Nov 18, 2020
0f55cbd
remove unused import of 'coreMock' in unit tests
Nov 18, 2020
d54a032
replace remaining 'callWithRequest' to 'callAsCurrentUser'
Nov 18, 2020
80be167
restore a line in 'jest.config.js' deleted by mistake
Nov 18, 2020
6c063a1
Add a missing notifications props to CreateMonitor
Nov 18, 2020
952f07f
Correct the path of getting message in toast notifications
Nov 18, 2020
e4ecbd4
Stop putting 'public' folder into the build artifacts, as 'target' fo…
Nov 18, 2020
4cb2c97
Use props to pass values for 'notifications' and 'isDarkMode' to avoi…
Nov 19, 2020
7e72b0d
remove the comment out code for assgning the icon for the plugin
Nov 19, 2020
f0b42b9
add a props of notifications in Home.js
Nov 19, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ build
.kibana-plugin-helpers.dev.json
coverage
yarn-error.log
target
6 changes: 3 additions & 3 deletions .kibana-plugin-helpers.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
"package.json",
"yarn.lock",
".yarnrc",
"index.js",
"{lib,public,server,webpackShims,translations,utils}/**/*",
"kibana.json",
"{lib,public,server,webpackShims,translations,utils,target}/**/*",
"!test",
"!__tests__"
]
}
}
79 changes: 0 additions & 79 deletions index.js

This file was deleted.

9 changes: 9 additions & 0 deletions kibana.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"id": "opendistroAlerting",
"version": "1.11.0.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we bump to 1.12.0.0 ?

Copy link
Author

Choose a reason for hiding this comment

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

I will do the version upgrade in another PR. Keeping the version number as 1.11 makes me possible to test the functionality in an ODFE 1.11 cluster after the migration. 😁

"kibanaVersion": "kibana",
Copy link
Contributor

Choose a reason for hiding this comment

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

Specify kibana version as "kibana" will auto resolve as 7.10?

Copy link
Author

Choose a reason for hiding this comment

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

Hi Yaliang, thanks a lot for reviewing my PR. In my mind, it will resolve as the version defined in package.json. Let me find some references.

Copy link
Author

Choose a reason for hiding this comment

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

It comes from "kbn-plugin-generator", in the template of the sample Kibana plugin, the value of "kibanaVersion" is set to "kibana". (https://github.com/elastic/kibana/blob/master/packages/kbn-plugin-generator/template/kibana.json.ejs).
As far as I see, it will get the version number from package.json.
Unfortunately, there is not much explanation for the manifest from Kibana. https://github.com/elastic/kibana/blob/master/docs/development/core/server/kibana-plugin-core-server.pluginmanifest.md

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, it's ok if the build and yarn start test pass

Copy link
Author

Choose a reason for hiding this comment

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

Thank you.👍 For the "kibanaVersion", probably it's not getting the version from "package.json" 🤔, because when I ran yarn build, the script prompts me to input the Kibana version manually.
I will investigate more about the "kibanaVersion", but it doesn't affect building the plugin.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, you may check @yizheliu-amazon 's build changes here - looks like you will want to change the serverSourcePattern in .kibana-plugin-helpers.json, and specify the plugin and kibana version in the Kibana manifest file. Not sure how the version verification and build naming has changed, with both the npm package.json and the Kibana manifest kibana.json specifying similar things now.

Copy link
Author

Choose a reason for hiding this comment

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

Hi Tyler, thank you for your reminder. I didn't notice there is the name change in .kibana-plugin-helpers.json. 👍

Copy link
Author

Choose a reason for hiding this comment

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

I will change the "kibanaVersion" to the exact version number to avoid inputting version number manually when building through plugin-hlepers in another PR for ODFE 1.12.
As it's written in the migration guide, seems package.json doesn't relate much to the build process.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool. Also, per the new naming convention, can you change opendistroAlerting to opendistroAlertingKibana?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for pointing out. Sure, I just changed it in kibana.json in another branch, and will be in the following PR.

"configPath": ["opendistro_alerting"],
"requiredPlugins": [],
"server": true,
"ui": true
}
59 changes: 21 additions & 38 deletions public/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,53 +14,36 @@
*/

import React from 'react';
import chrome from 'ui/chrome';
import { render, unmountComponentAtNode } from 'react-dom';
import { uiModules } from 'ui/modules';
import ReactDOM from 'react-dom';
import { HashRouter as Router, Route } from 'react-router-dom';

import 'react-vis/dist/style.css';
import './less/main.less';
// TODO: review the CSS style and migrate the necessary style to SASS, as Less is not supported in Kibana "new platform" anymore
// import './less/main.less';
import Main from './pages/Main';
import { AppContext } from './utils/AppContext';
import { CoreContext } from './utils/CoreContext';

const app = uiModules.get('apps/alerting');
const darkMode = chrome.getUiSettingsClient().get('theme:darkMode') || false;
export function renderApp(coreStart, params) {
const isDarkMode = coreStart.uiSettings.get('theme:darkMode') || false;
coreStart.chrome.setBreadcrumbs([{ text: 'Alerting' }]); // Set Breadcrumbs for the plugin

//Load Chart's dark mode CSS
if (darkMode) {
require('@elastic/charts/dist/theme_only_dark.css');
} else {
require('@elastic/charts/dist/theme_only_light.css');
}

app.config(($locationProvider) => {
$locationProvider.html5Mode({
enabled: false,
requireBase: false,
rewriteLinks: false,
});
});

app.config((stateManagementConfigProvider) => stateManagementConfigProvider.disable());

function RootController($scope, $element, $http) {
const domNode = $element[0];
// Load Chart's dark mode CSS
if (isDarkMode) {
require('@elastic/charts/dist/theme_only_dark.css');
} else {
require('@elastic/charts/dist/theme_only_light.css');
}

// render react to DOM
render(
ReactDOM.render(
<Router>
<AppContext.Provider value={{ httpClient: $http, darkMode }}>
<Route render={(props) => <Main title="Alerting" httpClient={$http} {...props} />} />
</AppContext.Provider>
<CoreContext.Provider
value={{ http: coreStart.http, isDarkMode, notifications: coreStart.notifications }}
>
<Route render={(props) => <Main title="Alerting" {...props} />} />
</CoreContext.Provider>
</Router>,
domNode
params.element
);

// unmount react on controller destroy
$scope.$on('$destroy', () => {
unmountComponentAtNode(domNode);
});
return () => ReactDOM.unmountComponentAtNode(params.element);
}

chrome.setRootController('alerting', RootController);
18 changes: 10 additions & 8 deletions public/components/Breadcrumbs/Breadcrumbs.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {

const propTypes = {
history: PropTypes.object.isRequired,
httpClient: PropTypes.func.isRequired,
httpClient: PropTypes.object.isRequired,
location: PropTypes.object.isRequired,
title: PropTypes.string.isRequired,
};
Expand Down Expand Up @@ -64,7 +64,9 @@ export default class Breadcrumbs extends Component {
location: { state: routeState },
} = this.props;
const rawBreadcrumbs = await getBreadcrumbs(window.location.hash, routeState, httpClient);
const breadcrumbs = rawBreadcrumbs.map(breadcrumb => createEuiBreadcrumb(breadcrumb, history));
const breadcrumbs = rawBreadcrumbs.map((breadcrumb) =>
createEuiBreadcrumb(breadcrumb, history)
);
this.setState({ breadcrumbs });
}

Expand All @@ -88,7 +90,7 @@ export function createEuiBreadcrumb(breadcrumb, history) {
return {
text,
href: `#${href}`,
onClick: e => {
onClick: (e) => {
e.preventDefault();
history.push(href);
},
Expand All @@ -98,14 +100,14 @@ export function createEuiBreadcrumb(breadcrumb, history) {
export async function getBreadcrumbs(hash, routeState, httpClient) {
const routes = parseLocationHash(hash);
const asyncBreadcrumbs = await Promise.all(
routes.map(route => getBreadcrumb(route, routeState, httpClient))
routes.map((route) => getBreadcrumb(route, routeState, httpClient))
);
const breadcrumbs = _.flatten(asyncBreadcrumbs).filter(breadcrumb => !!breadcrumb);
const breadcrumbs = _.flatten(asyncBreadcrumbs).filter((breadcrumb) => !!breadcrumb);
return breadcrumbs;
}

export function parseLocationHash(hash) {
return hash.split('/').filter(route => !!route);
return hash.split('/').filter((route) => !!route);
}

export async function getBreadcrumb(route, routeState, httpClient) {
Expand All @@ -128,8 +130,8 @@ export async function getBreadcrumb(route, routeState, httpClient) {
let monitorName = base;
try {
const response = await httpClient.get(`../api/alerting/monitors/${base}`);
if (response.data.ok) {
monitorName = response.data.resp.name;
if (response.ok) {
monitorName = response.resp.name;
}
} catch (err) {
console.error(err);
Expand Down
14 changes: 6 additions & 8 deletions public/components/Breadcrumbs/Breadcrumbs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ beforeEach(() => {

describe('Breadcrumbs', () => {
const title = 'Alerting';
httpClientMock.get = jest
.fn()
.mockResolvedValue({ data: { ok: true, resp: { name: 'random monitor' } } });
httpClientMock.get = jest.fn().mockResolvedValue({ ok: true, resp: { name: 'random monitor' } });
delete global.window.location;
global.window.location = { hash: '' };

Expand Down Expand Up @@ -115,29 +113,29 @@ describe('getBreadcrumb', () => {

describe('when matching document IDs', () => {
test('calls get monitor route', async () => {
httpClientMock.get.mockResolvedValue({ data: { ok: true, resp: { name: 'random_name' } } });
httpClientMock.get.mockResolvedValue({ ok: true, resp: { name: 'random_name' } });
await getBreadcrumb(monitorId, {}, httpClientMock);
expect(httpClientMock.get).toHaveBeenCalled();
expect(httpClientMock.get).toHaveBeenCalledWith(`../api/alerting/monitors/${monitorId}`);
});

test('returns monitor name', async () => {
httpClientMock.get.mockResolvedValue({ data: { ok: true, resp: { name: 'random_name' } } });
httpClientMock.get.mockResolvedValue({ ok: true, resp: { name: 'random_name' } });
expect(await getBreadcrumb(monitorId, {}, httpClientMock)).toMatchSnapshot();
});

test('uses monitor id as name if request fails', async () => {
httpClientMock.get.mockRejectedValue({ data: { ok: true, resp: { name: 'random_name' } } });
httpClientMock.get.mockRejectedValue({ ok: true, resp: { name: 'random_name' } });
expect(await getBreadcrumb(monitorId, {}, httpClientMock)).toMatchSnapshot();
});

test('uses monitor id as name if ok=false', async () => {
httpClientMock.get.mockResolvedValue({ data: { ok: false, resp: { name: 'random_name' } } });
httpClientMock.get.mockResolvedValue({ ok: false, resp: { name: 'random_name' } });
expect(await getBreadcrumb(monitorId, {}, httpClientMock)).toMatchSnapshot();
});

test('adds appropriate action breadcrumb', async () => {
httpClientMock.get.mockResolvedValue({ data: { ok: true, resp: { name: 'random_name' } } });
httpClientMock.get.mockResolvedValue({ ok: true, resp: { name: 'random_name' } });
expect(
await getBreadcrumb(
`${monitorId}?action=${MONITOR_ACTIONS.UPDATE_MONITOR}`,
Expand Down
8 changes: 7 additions & 1 deletion public/hack.js → public/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
* Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
Expand All @@ -12,3 +12,9 @@
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

import { AlertingPlugin } from './plugin';

export function plugin(initializerContext) {
return new AlertingPlugin(initializerContext);
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@ import React from 'react';
import { render } from 'enzyme';
import { EmptyFeaturesMessage } from './EmptyFeaturesMessage';

jest.mock('ui/chrome', () => ({
getBasePath: () => {
return 'http://localhost/app';
},
}));

describe('EmptyFeaturesMessage', () => {
test('renders ', () => {
const component = <EmptyFeaturesMessage detectorId="tempId" />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
import React from 'react';
import PropTypes from 'prop-types';
import moment from 'moment';
import { AppContext } from '../../../../utils/AppContext';
import { CoreContext } from '../../../../utils/CoreContext';
import { AD_PREVIEW_DAYS } from '../../../../utils/constants';

class AnomalyDetectorData extends React.Component {
static contextType = AppContext;
static contextType = CoreContext;
constructor(props) {
super(props);
this.state = {
Expand All @@ -43,25 +43,23 @@ class AnomalyDetectorData extends React.Component {

async getPreviewData() {
const { detectorId, startTime, endTime } = this.props;
const { httpClient } = this.context;
const { http: httpClient } = this.context;
this.setState({
isLoading: true,
});
if (!detectorId) return;
const requestParams = {
startTime: moment()
.subtract(AD_PREVIEW_DAYS, 'd')
.valueOf(),
startTime: moment().subtract(AD_PREVIEW_DAYS, 'd').valueOf(),
startTime: startTime,
endTime: endTime,
preview: this.props.preview,
};
try {
const response = await httpClient.get(`../api/alerting/detectors/${detectorId}/results`, {
params: requestParams,
query: requestParams,
});
if (response.data.ok) {
const { anomalyResult, detector } = response.data.response;
if (response.ok) {
const { anomalyResult, detector } = response.response;
this.setState({
...this.state,
anomalyResult,
Expand Down Expand Up @@ -94,9 +92,7 @@ AnomalyDetectorData.propTypes = {
};
AnomalyDetectorData.defaultProps = {
preview: true,
startTime: moment()
.subtract(5, 'd')
.valueOf(),
startTime: moment().subtract(5, 'd').valueOf(),
endTime: moment().valueOf(),
};

Expand Down