Skip to content

Commit 820160b

Browse files
authored
feat(aci): Add manual resolution threshold to metric detectors (#100925)
Adds the ability to manually set a resolution threshold on metric detector automatic <img width="506" height="161" alt="image" src="https://github.com/user-attachments/assets/dfe86394-e18c-4f38-84da-d3366a1444b0" /> manual w/ validation <img width="687" height="188" alt="image" src="https://github.com/user-attachments/assets/17e7e5d2-0ddb-4c62-817f-3d58634ef8e3" /> graph with resolution threshold <img width="795" height="239" alt="image" src="https://github.com/user-attachments/assets/02b79984-b9fd-43b5-badd-429dd62bdbaf" />
1 parent 104d087 commit 820160b

File tree

8 files changed

+312
-42
lines changed

8 files changed

+312
-42
lines changed

static/app/components/workflowEngine/form/control/priorityControl.spec.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ describe('PriorityControl', () => {
118118
await userEvent.type(highField, '5');
119119

120120
expect(formModel.getError(METRIC_DETECTOR_FORM_FIELDS.highThreshold)).toBe(
121-
'High threshold must be higher than medium threshold'
121+
'High threshold must be higher than medium threshold (10)'
122122
);
123123

124124
// Test valid case: high (15) > medium (10)

static/app/components/workflowEngine/form/control/priorityControl.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,9 @@ function validateHighThreshold({
114114
!validateThresholdOrder(highNum, conditionNum, conditionType, true)
115115
) {
116116
const message = t(
117-
'High threshold must be %s than medium threshold',
118-
conditionType === DataConditionType.GREATER ? t('higher') : t('lower')
117+
'High threshold must be %s than medium threshold (%s)',
118+
conditionType === DataConditionType.GREATER ? t('higher') : t('lower'),
119+
String(conditionNum)
119120
);
120121
return [createValidationError(METRIC_DETECTOR_FORM_FIELDS.highThreshold, message)];
121122
}

static/app/components/workflowEngine/ui/section.tsx

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ type SectionProps = {
66
title: React.ReactNode;
77
children?: React.ReactNode;
88
className?: string;
9-
description?: string;
9+
description?: React.ReactNode;
1010
};
1111

1212
export default function Section({children, className, title, description}: SectionProps) {
@@ -25,12 +25,19 @@ export const SectionSubHeading = styled('h5')`
2525
margin: 0;
2626
`;
2727

28+
const SectionDescription = styled('div')`
29+
font-size: ${p => p.theme.fontSize.md};
30+
font-weight: ${p => p.theme.fontWeight.normal};
31+
color: ${p => p.theme.subText};
32+
margin: 0;
33+
`;
34+
2835
const SectionContainer = styled(Flex)`
29-
> p {
36+
> ${SectionDescription} {
3037
margin-bottom: ${p => p.theme.space['0']};
3138
}
3239
33-
p + p {
40+
${SectionDescription} + ${SectionDescription} {
3441
margin-top: ${p => p.theme.space.md};
3542
}
3643
`;
@@ -41,9 +48,4 @@ const SectionHeading = styled('h4')`
4148
margin: 0;
4249
`;
4350

44-
const SectionDescription = styled('p')`
45-
font-size: ${p => p.theme.fontSize.md};
46-
font-weight: ${p => p.theme.fontWeight.normal};
47-
color: ${p => p.theme.subText};
48-
margin: 0;
49-
`;
51+
// moved above to reference in SectionContainer

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

Lines changed: 142 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,49 @@ import {DetectorDataset} from 'sentry/views/detectors/datasetConfig/types';
4949
import {getResolutionDescription} from 'sentry/views/detectors/utils/getDetectorResolutionDescription';
5050
import {getStaticDetectorThresholdSuffix} from 'sentry/views/detectors/utils/metricDetectorSuffix';
5151

52+
function validateThresholdOrder(
53+
value: number,
54+
reference: number,
55+
conditionType: DataConditionType,
56+
isGreaterExpected: boolean
57+
): boolean {
58+
if (conditionType === DataConditionType.GREATER) {
59+
return isGreaterExpected ? value > reference : value < reference;
60+
}
61+
// For LESS condition type, logic is inverted
62+
return isGreaterExpected ? value < reference : value > reference;
63+
}
64+
65+
function validateResolutionThreshold({
66+
form,
67+
}: {
68+
form: MetricDetectorFormData;
69+
id: string;
70+
}): Array<[string, string]> {
71+
const {conditionType, conditionValue, detectionType, resolutionStrategy} = form;
72+
if (!conditionType || detectionType !== 'static' || resolutionStrategy !== 'manual') {
73+
return [];
74+
}
75+
76+
const resolutionNum = Number(form.resolutionValue);
77+
const conditionNum = Number(conditionValue);
78+
79+
if (
80+
Number.isFinite(resolutionNum) &&
81+
Number.isFinite(conditionNum) &&
82+
!validateThresholdOrder(resolutionNum, conditionNum, conditionType, false)
83+
) {
84+
const message = t(
85+
'Resolution threshold must be %s than alert threshold (%s)',
86+
conditionType === DataConditionType.GREATER ? t('lower') : t('higher'),
87+
String(conditionNum)
88+
);
89+
return [[METRIC_DETECTOR_FORM_FIELDS.resolutionValue, message]];
90+
}
91+
92+
return [];
93+
}
94+
5295
function MetricDetectorForm() {
5396
return (
5497
<FormStack>
@@ -163,36 +206,96 @@ function ResolveSection() {
163206
const conditionComparisonAgo = useMetricDetectorFormField(
164207
METRIC_DETECTOR_FORM_FIELDS.conditionComparisonAgo
165208
);
209+
const resolutionStrategy = useMetricDetectorFormField(
210+
METRIC_DETECTOR_FORM_FIELDS.resolutionStrategy
211+
);
212+
const resolutionValue = useMetricDetectorFormField(
213+
METRIC_DETECTOR_FORM_FIELDS.resolutionValue
214+
);
166215
const aggregate = useMetricDetectorFormField(
167216
METRIC_DETECTOR_FORM_FIELDS.aggregateFunction
168217
);
169218
const thresholdSuffix = getStaticDetectorThresholdSuffix(aggregate);
170219

171-
const description = getResolutionDescription(
172-
detectionType === 'percent'
173-
? {
174-
detectionType: 'percent',
175-
conditionType,
176-
conditionValue,
177-
comparisonDelta: conditionComparisonAgo ?? 3600, // Default to 1 hour if not set
178-
thresholdSuffix,
179-
}
180-
: detectionType === 'static'
181-
? {
182-
detectionType: 'static',
183-
conditionType,
184-
conditionValue,
185-
thresholdSuffix,
186-
}
187-
: {
188-
detectionType: 'dynamic',
189-
thresholdSuffix,
190-
}
191-
);
220+
const effectiveConditionValue =
221+
resolutionStrategy === 'manual' &&
222+
resolutionValue !== undefined &&
223+
resolutionValue !== ''
224+
? resolutionValue
225+
: conditionValue;
226+
227+
const descriptionContent =
228+
detectionType === 'static' && resolutionStrategy === 'manual' ? (
229+
<Flex align="center" gap="sm">
230+
<div>
231+
<span>
232+
{t(
233+
'Issue will be resolved when the query value is %s',
234+
conditionType === DataConditionType.GREATER
235+
? t('less than')
236+
: t('more than')
237+
)}
238+
</span>
239+
</div>
240+
<ThresholdField
241+
aria-label={t('Resolution threshold')}
242+
name={METRIC_DETECTOR_FORM_FIELDS.resolutionValue}
243+
hideLabel
244+
inline={false}
245+
flexibleControlStateSize
246+
placeholder="0"
247+
suffix={thresholdSuffix}
248+
validate={validateResolutionThreshold}
249+
required
250+
preserveOnUnmount
251+
/>
252+
</Flex>
253+
) : (
254+
getResolutionDescription(
255+
detectionType === 'percent'
256+
? {
257+
detectionType: 'percent',
258+
conditionType,
259+
conditionValue: effectiveConditionValue,
260+
comparisonDelta: conditionComparisonAgo ?? 3600, // Default to 1 hour if not set
261+
thresholdSuffix,
262+
}
263+
: detectionType === 'static'
264+
? {
265+
detectionType: 'static',
266+
conditionType,
267+
conditionValue: effectiveConditionValue,
268+
thresholdSuffix,
269+
}
270+
: {
271+
detectionType: 'dynamic',
272+
thresholdSuffix,
273+
}
274+
)
275+
);
192276

193277
return (
194278
<Container>
195-
<Section title={t('Resolve')} description={description} />
279+
<Section title={t('Resolve')}>
280+
{detectionType !== 'dynamic' && (
281+
<Flex direction="column" gap="sm">
282+
<ResolutionStrategyField
283+
name={METRIC_DETECTOR_FORM_FIELDS.resolutionStrategy}
284+
label={t('Resolution method')}
285+
hideLabel
286+
inline={false}
287+
flexibleControlStateSize
288+
preserveOnUnmount
289+
defaultValue="automatic"
290+
choices={[
291+
['automatic', t('Automatic')],
292+
['manual', t('Manual')],
293+
]}
294+
/>
295+
</Flex>
296+
)}
297+
<DescriptionContainer>{descriptionContent}</DescriptionContainer>
298+
</Section>
196299
</Container>
197300
);
198301
}
@@ -523,14 +626,30 @@ const DetectionTypeField = styled(SegmentedRadioField)`
523626
padding: 0;
524627
}
525628
`;
629+
630+
const DescriptionContainer = styled('div')`
631+
display: flex;
632+
align-items: center;
633+
min-height: 36px;
634+
`;
635+
636+
const ResolutionStrategyField = styled(SegmentedRadioField)`
637+
padding: 0;
638+
max-width: 350px;
639+
640+
> div {
641+
padding: 0;
642+
}
643+
`;
644+
526645
const ThresholdField = styled(NumberField)`
527646
padding: 0;
528647
margin: 0;
529648
border: none;
530649
531650
> div {
532651
padding: 0;
533-
width: 10ch;
652+
width: 18ch;
534653
}
535654
`;
536655

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

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@ interface MetricDetectorConditionFormData {
6363
* Both kind=threshold and kind=change
6464
*/
6565
conditionValue?: string;
66+
/**
67+
* Strategy for how an issue should be resolved
68+
* - automatic: resolves based on the primary condition value
69+
* - manual: resolves based on a custom resolution value
70+
*/
71+
resolutionStrategy?: 'automatic' | 'manual';
72+
resolutionValue?: string;
6673
}
6774

6875
interface MetricDetectorDynamicFormData {
@@ -120,6 +127,8 @@ export const METRIC_DETECTOR_FORM_FIELDS = {
120127
conditionComparisonAgo: 'conditionComparisonAgo',
121128
conditionType: 'conditionType',
122129
conditionValue: 'conditionValue',
130+
resolutionStrategy: 'resolutionStrategy',
131+
resolutionValue: 'resolutionValue',
123132

124133
// Dynamic fields
125134
sensitivity: 'sensitivity',
@@ -134,6 +143,8 @@ export const DEFAULT_THRESHOLD_METRIC_FORM_DATA = {
134143
initialPriorityLevel: DetectorPriorityLevel.HIGH,
135144
conditionType: DataConditionType.GREATER,
136145
conditionValue: '',
146+
resolutionStrategy: 'automatic',
147+
resolutionValue: '',
137148
conditionComparisonAgo: 60 * 60, // One hour in seconds
138149

139150
// Default dynamic fields
@@ -177,7 +188,12 @@ interface NewDataSource {
177188
export function createConditions(
178189
data: Pick<
179190
MetricDetectorFormData,
180-
'conditionType' | 'conditionValue' | 'initialPriorityLevel' | 'highThreshold'
191+
| 'conditionType'
192+
| 'conditionValue'
193+
| 'initialPriorityLevel'
194+
| 'highThreshold'
195+
| 'resolutionStrategy'
196+
| 'resolutionValue'
181197
>
182198
): NewConditionGroup['conditions'] {
183199
if (!defined(data.conditionType) || !defined(data.conditionValue)) {
@@ -206,6 +222,24 @@ export function createConditions(
206222
});
207223
}
208224

225+
// Optionally add explicit resolution (OK) condition when manual strategy is chosen
226+
if (
227+
data.resolutionStrategy === 'manual' &&
228+
defined(data.resolutionValue) &&
229+
data.resolutionValue !== ''
230+
) {
231+
const resolutionConditionType =
232+
data.conditionType === DataConditionType.GREATER
233+
? DataConditionType.LESS
234+
: DataConditionType.GREATER;
235+
236+
conditions.push({
237+
type: resolutionConditionType,
238+
comparison: parseFloat(data.resolutionValue) || 0,
239+
conditionResult: DetectorPriorityLevel.OK,
240+
});
241+
}
242+
209243
return conditions;
210244
}
211245

@@ -319,7 +353,10 @@ export function metricDetectorFormDataToEndpointPayload(
319353
function processDetectorConditions(
320354
detector: MetricDetector
321355
): PrioritizeLevelFormData &
322-
Pick<MetricDetectorFormData, 'conditionValue' | 'conditionType'> {
356+
Pick<
357+
MetricDetectorFormData,
358+
'conditionValue' | 'conditionType' | 'resolutionStrategy' | 'resolutionValue'
359+
> {
323360
// Get conditions from the condition group
324361
const conditions = detector.conditionGroup?.conditions || [];
325362
// Sort by priority level, lowest first
@@ -337,6 +374,11 @@ function processDetectorConditions(
337374
condition => condition.conditionResult === DetectorPriorityLevel.HIGH
338375
);
339376

377+
// Find explicit resolution (OK) condition, if present
378+
const okCondition = conditions.find(
379+
condition => condition.conditionResult === DetectorPriorityLevel.OK
380+
);
381+
340382
// Determine initial priority level, ensuring it's valid for the form
341383
let initialPriorityLevel: DetectorPriorityLevel.MEDIUM | DetectorPriorityLevel.HIGH =
342384
DetectorPriorityLevel.MEDIUM;
@@ -368,6 +410,12 @@ function processDetectorConditions(
368410
typeof highCondition?.comparison === 'number'
369411
? highCondition.comparison.toString()
370412
: '',
413+
resolutionStrategy:
414+
typeof okCondition?.comparison === 'number' ? 'manual' : 'automatic',
415+
resolutionValue:
416+
typeof okCondition?.comparison === 'number'
417+
? okCondition.comparison.toString()
418+
: '',
371419
};
372420
}
373421

0 commit comments

Comments
 (0)