Skip to content

Commit

Permalink
Allow editing of APM rules (#106598)
Browse files Browse the repository at this point in the history
By pulling out most of the things that depend on the URL into where we open the flyout and passing them in as metadata props, we can make it so editing rules while in Stack Management.

You cannot edit a rule's service name, transaction type, or environment once it has been created (#106786 has been created to allow editing of these other values), but all other values can be edited.

In order for useFetcher to work outside of the APM plugin, it has been changed to use useKibana instead of useApmContext for toast notifications. The notifications API from useKibana is slightly different and allows passing a react element instead of a mount point as the body.

Fixes #76316.
  • Loading branch information
smith authored Jul 27, 2021
1 parent 76989b5 commit a6211f8
Show file tree
Hide file tree
Showing 13 changed files with 203 additions and 221 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import {
import { getInitialAlertValues } from '../get_initial_alert_values';
import { ApmPluginStartDeps } from '../../../plugin';
import { useServiceName } from '../../../hooks/use_service_name';
import { useApmParams } from '../../../hooks/use_apm_params';
import { AlertMetadata } from '../helper';
import { useUrlParams } from '../../../context/url_params_context/use_url_params';
import { ENVIRONMENT_ALL } from '../../../../common/environment_filter_values';

interface Props {
addFlyoutVisible: boolean;
Expand All @@ -23,7 +27,17 @@ interface Props {

export function AlertingFlyout(props: Props) {
const { addFlyoutVisible, setAddFlyoutVisibility, alertType } = props;

const serviceName = useServiceName();
const { query } = useApmParams('/*');
const {
urlParams: { start, end },
} = useUrlParams();
const environment =
'environment' in query ? query.environment : ENVIRONMENT_ALL.value;
const transactionType =
'transactionType' in query ? query.transactionType : undefined;

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

Expand All @@ -40,9 +54,26 @@ export function AlertingFlyout(props: Props) {
alertTypeId: alertType,
canChangeTrigger: false,
initialValues,
metadata: {
environment,
serviceName,
transactionType,
start,
end,
} as AlertMetadata,
}),
/* eslint-disable-next-line react-hooks/exhaustive-deps */
[alertType, onCloseAddFlyout, services.triggersActionsUi]
[
alertType,
environment,
onCloseAddFlyout,
services.triggersActionsUi,
serviceName,
transactionType,
environment,
start,
end,
]
);
return <>{addFlyoutVisible && addAlertFlyout}</>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,44 +5,47 @@
* 2.0.
*/

import React from 'react';
import { MemoryRouter } from 'react-router-dom';
import { ErrorCountAlertTrigger } from '.';
import { ApmPluginContextValue } from '../../../context/apm_plugin/apm_plugin_context';
import {
mockApmPluginContextValue,
MockApmPluginContextWrapper,
} from '../../../context/apm_plugin/mock_apm_plugin_context';
import React, { useState } from 'react';
import { AlertParams, ErrorCountAlertTrigger } from '.';
import { CoreStart } from '../../../../../../../src/core/public';
import { createKibanaReactContext } from '../../../../../../../src/plugins/kibana_react/public';

const KibanaReactContext = createKibanaReactContext(({
notifications: { toasts: { add: () => {} } },
} as unknown) as Partial<CoreStart>);

export default {
title: 'app/ErrorCountAlertTrigger',
title: 'alerting/ErrorCountAlertTrigger',
component: ErrorCountAlertTrigger,
decorators: [
(Story: React.ComponentClass) => (
<MockApmPluginContextWrapper
value={(mockApmPluginContextValue as unknown) as ApmPluginContextValue}
>
<MemoryRouter>
<div style={{ width: 400 }}>
<Story />
</div>
</MemoryRouter>
</MockApmPluginContextWrapper>
<KibanaReactContext.Provider>
<div style={{ width: 400 }}>
<Story />
</div>
</KibanaReactContext.Provider>
),
],
};

export function Example() {
const params = {
const [params, setParams] = useState<AlertParams>({
serviceName: 'testServiceName',
environment: 'testEnvironment',
threshold: 2,
window: '5m',
};
windowSize: 5,
windowUnit: 'm',
});

function setAlertParams(property: string, value: any) {
setParams({ ...params, [property]: value });
}

return (
<ErrorCountAlertTrigger
alertParams={params as any}
setAlertParams={() => undefined}
setAlertProperty={() => undefined}
alertParams={params}
setAlertParams={setAlertParams}
setAlertProperty={() => {}}
/>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,16 @@
*/

import { i18n } from '@kbn/i18n';
import { defaults, omit } from 'lodash';
import React from 'react';
import { defaults } from 'lodash';
import { ForLastExpression } from '../../../../../triggers_actions_ui/public';
import { ENVIRONMENT_ALL } from '../../../../common/environment_filter_values';
import { asInteger } from '../../../../common/utils/formatters';
import { useUrlParams } from '../../../context/url_params_context/use_url_params';
import { useEnvironmentsFetcher } from '../../../hooks/use_environments_fetcher';
import { useFetcher } from '../../../hooks/use_fetcher';
import { ChartPreview } from '../chart_preview';
import { EnvironmentField, IsAboveField, ServiceField } from '../fields';
import { getAbsoluteTimeRange } from '../helper';
import { AlertMetadata, getAbsoluteTimeRange } from '../helper';
import { ServiceAlertTrigger } from '../service_alert_trigger';
import { useServiceName } from '../../../hooks/use_service_name';

export interface AlertParams {
windowSize: number;
Expand All @@ -30,33 +27,26 @@ export interface AlertParams {

interface Props {
alertParams: AlertParams;
metadata?: AlertMetadata;
setAlertParams: (key: string, value: any) => void;
setAlertProperty: (key: string, value: any) => void;
}

export function ErrorCountAlertTrigger(props: Props) {
const { setAlertParams, setAlertProperty, alertParams } = props;
const { alertParams, metadata, setAlertParams, setAlertProperty } = props;

const serviceNameFromUrl = useServiceName();

const { urlParams } = useUrlParams();
const { start, end, environment: environmentFromUrl } = urlParams;
const { environmentOptions } = useEnvironmentsFetcher({
serviceName: serviceNameFromUrl,
start,
end,
serviceName: metadata?.serviceName,
start: metadata?.start,
end: metadata?.end,
});

const params = defaults(
{
...alertParams,
},
{ ...omit(metadata, ['start', 'end']), ...alertParams },
{
threshold: 25,
windowSize: 1,
windowUnit: 'm',
environment: environmentFromUrl || ENVIRONMENT_ALL.value,
serviceName: serviceNameFromUrl,
}
);

Expand Down
16 changes: 10 additions & 6 deletions x-pack/plugins/apm/public/components/alerting/fields.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,17 @@ export function EnvironmentField({
options: EuiSelectOption[];
onChange: (event: React.ChangeEvent<HTMLSelectElement>) => void;
}) {
const title = i18n.translate('xpack.apm.alerting.fields.environment', {
defaultMessage: 'Environment',
});

// "1" means "All" is the only option and we should not show a select.
if (options.length === 1) {
return <EuiExpression description={title} value={currentValue} />;
}

return (
<PopoverExpression
value={getEnvironmentLabel(currentValue)}
title={i18n.translate('xpack.apm.alerting.fields.environment', {
defaultMessage: 'Environment',
})}
>
<PopoverExpression value={getEnvironmentLabel(currentValue)} title={title}>
<EuiSelect
defaultValue={currentValue}
options={options}
Expand Down
8 changes: 8 additions & 0 deletions x-pack/plugins/apm/public/components/alerting/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@

import datemath from '@elastic/datemath';

export interface AlertMetadata {
environment: string;
serviceName?: string;
transactionType?: string;
start?: string;
end?: string;
}

export function getAbsoluteTimeRange(windowSize: number, windowUnit: string) {
const now = new Date().toISOString();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export function registerApmAlerts(
validate: () => ({
errors: [],
}),
requiresAppContext: true,
requiresAppContext: false,
defaultActionMessage: i18n.translate(
'xpack.apm.alertTypes.errorCount.defaultActionMessage',
{
Expand Down Expand Up @@ -126,7 +126,7 @@ export function registerApmAlerts(
validate: () => ({
errors: [],
}),
requiresAppContext: true,
requiresAppContext: false,
defaultActionMessage: i18n.translate(
'xpack.apm.alertTypes.transactionDuration.defaultActionMessage',
{
Expand Down Expand Up @@ -182,7 +182,7 @@ export function registerApmAlerts(
validate: () => ({
errors: [],
}),
requiresAppContext: true,
requiresAppContext: false,
defaultActionMessage: i18n.translate(
'xpack.apm.alertTypes.transactionErrorRate.defaultActionMessage',
{
Expand Down Expand Up @@ -237,7 +237,7 @@ export function registerApmAlerts(
validate: () => ({
errors: [],
}),
requiresAppContext: true,
requiresAppContext: false,
defaultActionMessage: i18n.translate(
'xpack.apm.alertTypes.transactionDurationAnomaly.defaultActionMessage',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,69 +6,51 @@
*/

import { Story } from '@storybook/react';
import { cloneDeep, merge } from 'lodash';
import React, { ComponentType } from 'react';
import { MemoryRouter, Route } from 'react-router-dom';
import { TransactionDurationAlertTrigger } from '.';
import { ApmPluginContextValue } from '../../../context/apm_plugin/apm_plugin_context';
import {
mockApmPluginContextValue,
MockApmPluginContextWrapper,
} from '../../../context/apm_plugin/mock_apm_plugin_context';
import { ApmServiceContextProvider } from '../../../context/apm_service/apm_service_context';
import { MockUrlParamsContextProvider } from '../../../context/url_params_context/mock_url_params_context_provider';
import React, { ComponentType, useState } from 'react';
import { AlertParams, TransactionDurationAlertTrigger } from '.';
import { CoreStart } from '../../../../../../../src/core/public';
import { createKibanaReactContext } from '../../../../../../../src/plugins/kibana_react/public';

const KibanaReactContext = createKibanaReactContext(({
notifications: { toasts: { add: () => {} } },
} as unknown) as Partial<CoreStart>);

export default {
title: 'alerting/TransactionDurationAlertTrigger',
component: TransactionDurationAlertTrigger,
decorators: [
(StoryComponent: ComponentType) => {
const contextMock = (merge(cloneDeep(mockApmPluginContextValue), {
core: {
http: {
get: (endpoint: string) => {
if (endpoint === '/api/apm/environments') {
return Promise.resolve({ environments: ['production'] });
} else {
return Promise.resolve({
transactionTypes: ['request'],
});
}
},
},
},
}) as unknown) as ApmPluginContextValue;

return (
<div style={{ width: 400 }}>
<MemoryRouter initialEntries={['/services/test-service-name']}>
<Route path="/services/:serviceName">
<MockApmPluginContextWrapper value={contextMock}>
<MockUrlParamsContextProvider>
<ApmServiceContextProvider>
<StoryComponent />
</ApmServiceContextProvider>
</MockUrlParamsContextProvider>
</MockApmPluginContextWrapper>
</Route>
</MemoryRouter>
</div>
<KibanaReactContext.Provider>
<div style={{ width: 400 }}>
<StoryComponent />
</div>
</KibanaReactContext.Provider>
);
},
],
};

export const Example: Story = () => {
const params = {
threshold: 1500,
const [params, setParams] = useState<AlertParams>({
aggregationType: 'avg' as const,
window: '5m',
};
environment: 'testEnvironment',
serviceName: 'testServiceName',
threshold: 1500,
transactionType: 'testTransactionType',
windowSize: 5,
windowUnit: 'm',
});

function setAlertParams(property: string, value: any) {
setParams({ ...params, [property]: value });
}

return (
<TransactionDurationAlertTrigger
alertParams={params as any}
setAlertParams={() => undefined}
setAlertProperty={() => undefined}
alertParams={params}
setAlertParams={setAlertParams}
setAlertProperty={() => {}}
/>
);
};
Loading

0 comments on commit a6211f8

Please sign in to comment.