Skip to content

Commit 42aef6a

Browse files
malwilleymalwilley
committed
feat(aci): Add automatic naming when creating a new metric monitor
1 parent 61174be commit 42aef6a

File tree

9 files changed

+210
-4
lines changed

9 files changed

+210
-4
lines changed

‎static/app/components/workflowEngine/form/getFormFieldValue.tsx‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import type FormModel from 'sentry/components/forms/model';
2+
import type {FieldValue} from 'sentry/components/forms/types';
3+
4+
/**
5+
* The form values returned by form.getValue are not typed, so this is a
6+
* convenient helper to do a type assertion while getting the value.
7+
*/
8+
export function getFormFieldValue<T extends FieldValue = FieldValue>(
9+
form: FormModel,
10+
field: string
11+
): T {
12+
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
13+
return form.getValue(field) as FieldValue;
14+
}

‎static/app/components/workflowEngine/form/useFormField.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {observe} from 'mobx';
44

55
import FormContext from 'sentry/components/forms/formContext';
66
import type {FieldValue} from 'sentry/components/forms/types';
7+
import {getFormFieldValue} from 'sentry/components/workflowEngine/form/getFormFieldValue';
78

89
export function useFormField<Value extends FieldValue = FieldValue>(
910
field: string
@@ -42,8 +43,7 @@ export function useFormField<Value extends FieldValue = FieldValue>(
4243
return undefined;
4344
}
4445

45-
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
46-
return context.form.getValue(field) as Value;
46+
return getFormFieldValue<Value>(context.form, field);
4747
}, [context.form, field]);
4848

4949
return useSyncExternalStore(subscribe, getSnapshot);

‎static/app/views/detectors/components/forms/metric/metric.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import {
4646
} from 'sentry/views/detectors/components/forms/metric/metricFormData';
4747
import {MetricDetectorPreviewChart} from 'sentry/views/detectors/components/forms/metric/previewChart';
4848
import {ResolveSection} from 'sentry/views/detectors/components/forms/metric/resolveSection';
49+
import {useAutoMetricDetectorName} from 'sentry/views/detectors/components/forms/metric/useAutoMetricDetectorName';
4950
import {useInitialMetricDetectorFormData} from 'sentry/views/detectors/components/forms/metric/useInitialMetricDetectorFormData';
5051
import {useIntervalChoices} from 'sentry/views/detectors/components/forms/metric/useIntervalChoices';
5152
import {Visualize} from 'sentry/views/detectors/components/forms/metric/visualize';
@@ -57,6 +58,8 @@ import {getStaticDetectorThresholdSuffix} from 'sentry/views/detectors/utils/met
5758
import {deprecateTransactionAlerts} from 'sentry/views/insights/common/utils/hasEAPAlerts';
5859

5960
function MetricDetectorForm() {
61+
useAutoMetricDetectorName();
62+
6063
return (
6164
<FormStack>
6265
<TransactionsDatasetWarningListener />

‎static/app/views/detectors/components/forms/metric/useAutoMetricDetectorName.tsx‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import type FormModel from 'sentry/components/forms/model';
2+
import {getFormFieldValue} from 'sentry/components/workflowEngine/form/getFormFieldValue';
3+
import {t} from 'sentry/locale';
4+
import {DataConditionType} from 'sentry/types/workflowEngine/dataConditions';
5+
import getDuration from 'sentry/utils/duration/getDuration';
6+
import {unreachable} from 'sentry/utils/unreachable';
7+
import {useSetAutomaticName} from 'sentry/views/detectors/components/forms/common/useSetAutomaticName';
8+
import {
9+
METRIC_DETECTOR_FORM_FIELDS,
10+
type MetricDetectorFormData,
11+
} from 'sentry/views/detectors/components/forms/metric/metricFormData';
12+
import {getDatasetConfig} from 'sentry/views/detectors/datasetConfig/getDatasetConfig';
13+
import {getStaticDetectorThresholdSuffix} from 'sentry/views/detectors/utils/metricDetectorSuffix';
14+
15+
/**
16+
* Automatically generates a name for the metric detector form based on the values:
17+
* - Dataset
18+
* - Aggregate function
19+
* - Detection type
20+
* - Direction (above or below)
21+
* - Threshold value
22+
* - Interval
23+
*
24+
* e.g. "p95(span.duration) above 100ms compared to past 1 hour"
25+
*/
26+
export function useAutoMetricDetectorName() {
27+
useSetAutomaticName((form: FormModel): string | null => {
28+
const detectionType = getFormFieldValue<MetricDetectorFormData['detectionType']>(
29+
form,
30+
METRIC_DETECTOR_FORM_FIELDS.detectionType
31+
);
32+
const aggregate = getFormFieldValue<MetricDetectorFormData['aggregateFunction']>(
33+
form,
34+
METRIC_DETECTOR_FORM_FIELDS.aggregateFunction
35+
);
36+
const interval = getFormFieldValue<MetricDetectorFormData['interval']>(
37+
form,
38+
METRIC_DETECTOR_FORM_FIELDS.interval
39+
);
40+
const dataset = getFormFieldValue<MetricDetectorFormData['dataset']>(
41+
form,
42+
METRIC_DETECTOR_FORM_FIELDS.dataset
43+
);
44+
const datasetConfig = getDatasetConfig(dataset);
45+
46+
if (!aggregate || !interval || !detectionType || !dataset) {
47+
return null;
48+
}
49+
50+
switch (detectionType) {
51+
case 'static': {
52+
const conditionType = getFormFieldValue<MetricDetectorFormData['conditionType']>(
53+
form,
54+
METRIC_DETECTOR_FORM_FIELDS.conditionType
55+
);
56+
const conditionValue = getFormFieldValue<
57+
MetricDetectorFormData['conditionValue']
58+
>(form, METRIC_DETECTOR_FORM_FIELDS.conditionValue);
59+
60+
if (!conditionType || !conditionValue) {
61+
return null;
62+
}
63+
64+
const suffix = getStaticDetectorThresholdSuffix(aggregate);
65+
const direction =
66+
conditionType === DataConditionType.GREATER ? t('above') : t('below');
67+
68+
return t(
69+
'%(aggregate)s %(aboveOrBelow)s %(value)s%(unit)s over past %(interval)s',
70+
{
71+
aggregate: datasetConfig.formatAggregateForTitle?.(aggregate) ?? aggregate,
72+
aboveOrBelow: direction,
73+
value: conditionValue,
74+
unit: suffix,
75+
interval: getDuration(interval),
76+
}
77+
);
78+
}
79+
case 'percent': {
80+
const conditionType = getFormFieldValue<MetricDetectorFormData['conditionType']>(
81+
form,
82+
METRIC_DETECTOR_FORM_FIELDS.conditionType
83+
);
84+
const conditionValue = getFormFieldValue<
85+
MetricDetectorFormData['conditionValue']
86+
>(form, METRIC_DETECTOR_FORM_FIELDS.conditionValue);
87+
88+
if (!conditionType || !conditionValue) {
89+
return null;
90+
}
91+
92+
const direction =
93+
conditionType === DataConditionType.GREATER ? t('higher') : t('lower');
94+
95+
return t(
96+
'%(aggregate)s %(aboveOrBelow)s by %(value)s%% compared to past %(interval)s',
97+
{
98+
aggregate: datasetConfig.formatAggregateForTitle?.(aggregate) ?? aggregate,
99+
aboveOrBelow: direction,
100+
value: conditionValue,
101+
interval: getDuration(interval),
102+
}
103+
);
104+
}
105+
case 'dynamic':
106+
return t('%(aggregate)s is anomalous', {
107+
aggregate: datasetConfig.formatAggregateForTitle?.(aggregate) ?? aggregate,
108+
});
109+
default:
110+
unreachable(detectionType);
111+
return null;
112+
}
113+
});
114+
}

‎static/app/views/detectors/datasetConfig/base.tsx‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,4 +138,12 @@ export interface DetectorDatasetConfig<SeriesResponse> {
138138
data: SeriesResponse | undefined,
139139
aggregate: string
140140
) => Series[];
141+
142+
/**
143+
* When automatically generating a detector name, this function will be called to format the aggregate function.
144+
* If this function is not provided, the aggregate function will be used as is.
145+
*
146+
* e.g. For the errors dataset, count() will be formatted as 'Number of errors'
147+
*/
148+
formatAggregateForTitle?: (aggregate: string) => string;
141149
}

‎static/app/views/detectors/datasetConfig/errors.tsx‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,4 +131,13 @@ export const DetectorErrorsConfig: DetectorDatasetConfig<ErrorsSeriesResponse> =
131131
parseEventTypesFromQuery(query, DEFAULT_EVENT_TYPES),
132132
// TODO: This should use the discover dataset unless `is:unresolved` is in the query
133133
getDiscoverDataset: () => DiscoverDatasets.ERRORS,
134+
formatAggregateForTitle: aggregate => {
135+
if (aggregate === 'count()') {
136+
return t('Number of errors');
137+
}
138+
if (aggregate === 'count_unique(user)') {
139+
return t('Users experiencing errors');
140+
}
141+
return aggregate;
142+
},
134143
};

‎static/app/views/detectors/datasetConfig/logs.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,10 @@ export const DetectorLogsConfig: DetectorDatasetConfig<LogsSeriesRepsonse> = {
6363
},
6464
supportedDetectionTypes: ['static', 'percent', 'dynamic'],
6565
getDiscoverDataset: () => DiscoverDatasets.OURLOGS,
66+
formatAggregateForTitle: aggregate => {
67+
if (aggregate === 'count()') {
68+
return t('Number of logs');
69+
}
70+
return aggregate;
71+
},
6672
};

‎static/app/views/detectors/datasetConfig/spans.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,10 @@ export const DetectorSpansConfig: DetectorDatasetConfig<SpansSeriesResponse> = {
7979
},
8080
supportedDetectionTypes: ['static', 'percent', 'dynamic'],
8181
getDiscoverDataset: () => DiscoverDatasets.SPANS,
82+
formatAggregateForTitle: aggregate => {
83+
if (aggregate.startsWith('count(span.duration)')) {
84+
return t('Number of spans');
85+
}
86+
return aggregate;
87+
},
8288
};

‎static/app/views/detectors/new-setting.spec.tsx‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import DetectorNewSettings from 'sentry/views/detectors/new-settings';
1515

1616
describe('DetectorEdit', () => {
1717
const organization = OrganizationFixture({
18-
features: ['workflow-engine-ui', 'visibility-explore-view'],
18+
features: ['workflow-engine-ui', 'visibility-explore-view', 'performance-view'],
1919
});
2020
const project = ProjectFixture({organization, environments: ['production']});
2121
const initialRouterConfig = {
@@ -98,6 +98,52 @@ describe('DetectorEdit', () => {
9898
},
9999
};
100100

101+
it('auto-generates name', async () => {
102+
render(<DetectorNewSettings />, {
103+
organization,
104+
initialRouterConfig: metricRouterConfig,
105+
});
106+
await screen.findByText('New Monitor');
107+
108+
// Enter threshold value
109+
await userEvent.type(screen.getByRole('spinbutton', {name: 'Threshold'}), '100');
110+
111+
// Name should be auto-generated from defaults (Spans + count(span.duration))
112+
expect(await screen.findByTestId('editable-text-label')).toHaveTextContent(
113+
'Number of spans above 100 over past 1 hour'
114+
);
115+
116+
// Change aggregate from count() to p75(span.duration)
117+
await userEvent.click(screen.getByRole('button', {name: 'count'}));
118+
await userEvent.click(await screen.findByRole('option', {name: 'p75'}));
119+
120+
await waitFor(() => {
121+
expect(screen.getByTestId('editable-text-label')).toHaveTextContent(
122+
'p75(span.duration) above 100ms over past 1 hour'
123+
);
124+
});
125+
126+
// Change dataset from Spans to Errors
127+
await userEvent.click(screen.getByText('Spans'));
128+
await userEvent.click(await screen.findByRole('menuitemradio', {name: 'Errors'}));
129+
130+
await waitFor(() => {
131+
expect(screen.getByTestId('editable-text-label')).toHaveTextContent(
132+
'Number of errors above 100 over past 1 hour'
133+
);
134+
});
135+
136+
// Change interval from 1 hour to 4 hours
137+
await userEvent.click(screen.getByText('1 hour'));
138+
await userEvent.click(screen.getByRole('menuitemradio', {name: '4 hours'}));
139+
140+
await waitFor(() => {
141+
expect(screen.getByTestId('editable-text-label')).toHaveTextContent(
142+
'Number of errors above 100 over past 4 hours'
143+
);
144+
});
145+
});
146+
101147
it('can submit a new metric detector', async () => {
102148
const mockCreateDetector = MockApiClient.addMockResponse({
103149
url: `/organizations/${organization.slug}/detectors/`,
@@ -200,7 +246,7 @@ describe('DetectorEdit', () => {
200246
`/organizations/${organization.slug}/detectors/`,
201247
expect.objectContaining({
202248
data: expect.objectContaining({
203-
name: 'My Monitor',
249+
name: 'Users experiencing errors above 100 over past 1 hour',
204250
type: 'metric_issue',
205251
projectId: project.id,
206252
owner: null,

0 commit comments

Comments
 (0)