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.

Sign up for GitHub

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]: Replace error occurrence watchers with Kibana Alerting #46547

Closed
wants to merge 1 commit into from
Closed

[APM]: Replace error occurrence watchers with Kibana Alerting #46547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions x-pack/legacy/plugins/apm/common/alerting/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* 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 const ERROR_OCCURRENCE_ALERT_TYPE_ID = 'apm.error_occurrence';
export const NOTIFICATION_EMAIL_ACTION_ID = 'apm.notificationEmail';
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we need this right now. I was figuring out if we could create the email action we need when registering the alert type, but because I never got around to implementing email it's not being used.

9 changes: 8 additions & 1 deletion x-pack/legacy/plugins/apm/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,14 @@ import { plugin } from './server/new-platform/index';

export const apm: LegacyPluginInitializer = kibana => {
return new kibana.Plugin({
require: ['kibana', 'elasticsearch', 'xpack_main', 'apm_oss'],
require: [
'kibana',
'elasticsearch',
'xpack_main',
'apm_oss',
'alerting',
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if these should be required, or if they are optional. If it's the latter, not sure how we can get access to these plugins on startup.

Copy link
Member

Choose a reason for hiding this comment

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

Really good question for platform how to handle optional dependencies. I think it has come up before but can't remember the answer.

Copy link
Contributor

Choose a reason for hiding this comment

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

New platform allows for optional dependencies, but legacy platform does not. If you are depending on something in a legacy plugin that could be disabled, you need to make sure that you implement isEnabled and check that the dependency is enabled, otherwise Kibana will crash.

'actions'
],
id: 'apm',
configPrefix: 'xpack.apm',
publicDir: resolve(__dirname, 'public'),
Expand Down
145 changes: 29 additions & 116 deletions ...cy/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/WatcherFlyout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
EuiForm,
EuiFormRow,
EuiLink,
EuiRadio,
EuiSelect,
EuiSpacer,
EuiSwitch,
Expand All @@ -26,15 +25,17 @@ import {
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import { padLeft, range } from 'lodash';
import moment from 'moment-timezone';
import React, { Component } from 'react';
import styled from 'styled-components';
import { toastNotifications } from 'ui/notify';
import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n';
import { KibanaCoreContext } from '../../../../../../observability/public';
import { IUrlParams } from '../../../../context/UrlParamsContext/types';
import { KibanaLink } from '../../../shared/Links/KibanaLink';
import { createErrorGroupWatch, Schedule } from './createErrorGroupWatch';
import { Schedule } from './createErrorGroupWatch';
import { ElasticDocsLink } from '../../../shared/Links/ElasticDocsLink';
import { createErrorOccurrenceAlert } from './createErrorOccurrenceAlert';
import { APMClient } from '../../../../services/rest/createCallApmApi';

type ScheduleKey = keyof Schedule;

Expand All @@ -49,6 +50,7 @@ const SmallInput = styled.div`

interface WatcherFlyoutProps {
urlParams: IUrlParams;
callApmApi: APMClient;
onClose: () => void;
isOpen: boolean;
}
Expand Down Expand Up @@ -149,57 +151,32 @@ export class WatcherFlyout extends Component<
};

public createWatch = () => {
const core = this.context;
const { serviceName } = this.props.urlParams;

if (!serviceName) {
return;
}

const emails = this.state.actions.email
? this.state.emails
.split(',')
.map(email => email.trim())
.filter(email => !!email)
: [];
const email = this.state.actions.email ? this.state.emails : '';
Copy link
Member Author

Choose a reason for hiding this comment

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

I've simplified this because we can just pass a csv to the action, because it uses nodemailer under the hood. Never tested it though.


const slackUrl = this.state.actions.slack ? this.state.slackUrl : '';

const schedule =
this.state.schedule === 'interval'
? {
interval: `${this.state.interval.value}${this.state.interval.unit}`
}
: {
daily: { at: `${this.state.daily}` }
};
const timeRange = {
value: this.state.interval.value,
unit: this.state.interval.unit
};

const timeRange =
this.state.schedule === 'interval'
? {
value: this.state.interval.value,
unit: this.state.interval.unit
}
: {
value: 24,
unit: 'h'
};

const apmIndexPatternTitle = core.injectedMetadata.getInjectedVar(
'apmIndexPatternTitle'
) as string;

return createErrorGroupWatch({
emails,
schedule,
return createErrorOccurrenceAlert({
callApmApi: this.props.callApmApi,
email,
serviceName,
slackUrl,
threshold: this.state.threshold,
timeRange,
apmIndexPatternTitle
timeRange
})
.then((id: string) => {
.then(savedObject => {
this.props.onClose();
const id = 'id' in savedObject ? savedObject.id : NOT_AVAILABLE_LABEL;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I'm outmoded, but isn't the in operator generally discouraged since it's at risk for prototype hijacking?

this.addSuccessToast(id);
})
.catch(e => {
Expand All @@ -210,9 +187,7 @@ export class WatcherFlyout extends Component<
};

public addErrorToast = () => {
const core = this.context;

core.notifications.toasts.addWarning({
toastNotifications.addWarning({
title: i18n.translate(
'xpack.apm.serviceDetails.enableErrorReportsPanel.watchCreationFailedNotificationTitle',
{
Expand All @@ -234,9 +209,7 @@ export class WatcherFlyout extends Component<
};

public addSuccessToast = (id: string) => {
const core = this.context;

core.notifications.toasts.addSuccess({
toastNotifications.addSuccess({
title: i18n.translate(
'xpack.apm.serviceDetails.enableErrorReportsPanel.watchCreatedNotificationTitle',
{
Expand All @@ -255,18 +228,16 @@ export class WatcherFlyout extends Component<
}
}
)}{' '}
<KibanaCoreContext.Provider value={core}>
<KibanaLink
path={`/management/elasticsearch/watcher/watches/watch/${id}`}
>
{i18n.translate(
'xpack.apm.serviceDetails.enableErrorReportsPanel.watchCreatedNotificationText.viewWatchLinkText',
{
defaultMessage: 'View watch'
}
)}
</KibanaLink>
</KibanaCoreContext.Provider>
<KibanaLink
path={`/management/elasticsearch/watcher/watches/watch/${id}`}
>
{i18n.translate(
'xpack.apm.serviceDetails.enableErrorReportsPanel.watchCreatedNotificationText.viewWatchLinkText',
{
defaultMessage: 'View watch'
}
)}
</KibanaLink>
</p>
)
});
Expand All @@ -277,20 +248,6 @@ export class WatcherFlyout extends Component<
return null;
}

const dailyTime = this.state.daily;
const inputTime = `${dailyTime}Z`; // Add tz to make into UTC
const inputFormat = 'HH:mmZ'; // Parse as 24 hour w. tz
const dailyTimeFormatted = moment(inputTime, inputFormat).format('HH:mm'); // Format as 24h
const dailyTime12HourFormatted = moment(inputTime, inputFormat).format(
'hh:mm A (z)'
); // Format as 12h w. tz

// Generate UTC hours for Daily Report select field
const intervalHours = range(24).map(i => {
const hour = padLeft(i.toString(), 2, '0');
return { value: `${hour}:00`, text: `${hour}:00 UTC` };
});

const flyoutBody = (
<EuiText>
<p>
Expand Down Expand Up @@ -367,50 +324,6 @@ export class WatcherFlyout extends Component<
}
)}
</EuiText>
<EuiSpacer size="m" />
<EuiRadio
id="daily"
label={i18n.translate(
'xpack.apm.serviceDetails.enableErrorReportsPanel.dailyReportRadioButtonLabel',
{
defaultMessage: 'Daily report'
}
)}
onChange={() => this.onChangeSchedule('daily')}
checked={this.state.schedule === 'daily'}
/>
<EuiSpacer size="m" />
<EuiFormRow
helpText={i18n.translate(
'xpack.apm.serviceDetails.enableErrorReportsPanel.dailyReportHelpText',
{
defaultMessage:
'The daily report will be sent at {dailyTimeFormatted} / {dailyTime12HourFormatted}.',
values: { dailyTimeFormatted, dailyTime12HourFormatted }
}
)}
compressed
>
<EuiSelect
value={dailyTime}
onChange={this.onChangeDailyUnit}
options={intervalHours}
disabled={this.state.schedule !== 'daily'}
/>
</EuiFormRow>
<EuiSpacer size="m" />
<EuiRadio
id="interval"
label={i18n.translate(
'xpack.apm.serviceDetails.enableErrorReportsPanel.intervalRadioButtonLabel',
{
defaultMessage: 'Interval'
}
)}
onChange={() => this.onChangeSchedule('interval')}
checked={this.state.schedule === 'interval'}
/>
<EuiSpacer size="m" />
<EuiFlexGroup>
<EuiFlexItem grow={false}>
<SmallInput>
Expand Down
48 changes: 48 additions & 0 deletions ...pm/public/components/app/ServiceDetails/ServiceIntegrations/createErrorOccurrenceAlert.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* 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 { APMClient } from '../../../../services/rest/createCallApmApi';

export interface Schedule {
interval: string;
}

interface Arguments {
callApmApi: APMClient;
email: string;
serviceName: string;
slackUrl: string;
threshold: number;
timeRange: {
value: number;
unit: string;
};
}

export async function createErrorOccurrenceAlert({
callApmApi,
email,
serviceName,
slackUrl,
threshold,
timeRange
}: Arguments) {
return callApmApi({
pathname: '/api/apm/alerts/error_occurrence',
method: 'POST',
params: {
body: {
serviceName,
threshold,
interval: `${timeRange.value}${timeRange.unit}`,
actions: {
email,
slack: slackUrl
}
}
}
});
}
3 changes: 3 additions & 0 deletions x-pack/legacy/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ import { IUrlParams } from '../../../../context/UrlParamsContext/types';
import { LicenseContext } from '../../../../context/LicenseContext';
import { MachineLearningFlyout } from './MachineLearningFlyout';
import { WatcherFlyout } from './WatcherFlyout';
import { APMClient } from '../../../../services/rest/createCallApmApi';

interface Props {
urlParams: IUrlParams;
callApmApi: APMClient;
}
interface State {
isPopoverOpen: boolean;
Expand Down Expand Up @@ -162,6 +164,7 @@ export class ServiceIntegrations extends React.Component<Props, State> {
urlParams={this.props.urlParams}
/>
<WatcherFlyout
callApmApi={this.props.callApmApi}
isOpen={this.state.activeFlyout === 'Watcher'}
onClose={this.closeFlyouts}
urlParams={this.props.urlParams}
Expand Down
7 changes: 6 additions & 1 deletion x-pack/legacy/plugins/apm/public/components/app/ServiceDetails/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { ApmHeader } from '../../shared/ApmHeader';
import { ServiceDetailTabs } from './ServiceDetailTabs';
import { ServiceIntegrations } from './ServiceIntegrations';
import { useUrlParams } from '../../../hooks/useUrlParams';
import { useCallApmApi } from '../../../hooks/useCallApmApi';

interface Props {
tab: React.ComponentProps<typeof ServiceDetailTabs>['tab'];
Expand All @@ -18,6 +19,7 @@ interface Props {
export function ServiceDetails({ tab }: Props) {
const { urlParams } = useUrlParams();
const { serviceName } = urlParams;
const callApmApi = useCallApmApi();

return (
<div>
Expand All @@ -29,7 +31,10 @@ export function ServiceDetails({ tab }: Props) {
</EuiTitle>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<ServiceIntegrations urlParams={urlParams} />
<ServiceIntegrations
urlParams={urlParams}
callApmApi={callApmApi}
/>
</EuiFlexItem>
</EuiFlexGroup>
</ApmHeader>
Expand Down
Loading