Skip to content

Commit 23618c5

Browse files
authored
feat(aci): Add missing anomaly detection conditions (#103004)
Apparently we were missing these entirely. Also fixes some issues when editing an existing anomaly detector and cleans up an unused config value.
1 parent 32f8d80 commit 23618c5

File tree

8 files changed

+132
-43
lines changed

8 files changed

+132
-43
lines changed

static/app/types/workflowEngine/dataConditions.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ export enum DataConditionType {
5050
EVENT_FREQUENCY = 'event_frequency',
5151
EVENT_UNIQUE_USER_FREQUENCY = 'event_unique_user_frequency',
5252
PERCENT_SESSIONS = 'percent_sessions',
53+
ANOMALY_DETECTION = 'anomaly_detection',
5354
}
5455

5556
export enum DataConditionGroupLogicType {

static/app/types/workflowEngine/detectors.tsx

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,33 +80,26 @@ export type DetectorType =
8080
| 'uptime_domain_failure'
8181
| 'issue_stream';
8282

83-
interface BaseMetricDetectorConfig {
84-
thresholdPeriod: number;
85-
}
86-
8783
/**
8884
* Configuration for static/threshold-based detection
8985
*/
90-
interface MetricDetectorConfigStatic extends BaseMetricDetectorConfig {
86+
interface MetricDetectorConfigStatic {
9187
detectionType: 'static';
9288
}
9389

9490
/**
9591
* Configuration for percentage-based change detection
9692
*/
97-
interface MetricDetectorConfigPercent extends BaseMetricDetectorConfig {
93+
interface MetricDetectorConfigPercent {
9894
comparisonDelta: number;
9995
detectionType: 'percent';
10096
}
10197

10298
/**
10399
* Configuration for dynamic/anomaly detection
104100
*/
105-
interface MetricDetectorConfigDynamic extends BaseMetricDetectorConfig {
101+
interface MetricDetectorConfigDynamic {
106102
detectionType: 'dynamic';
107-
seasonality?: 'auto' | 'daily' | 'weekly' | 'monthly';
108-
sensitivity?: AlertRuleSensitivity;
109-
thresholdType?: AlertRuleThresholdType;
110103
}
111104

112105
export type MetricDetectorConfig =
@@ -233,7 +226,7 @@ export interface MetricCondition {
233226
/**
234227
* See AnomalyDetectionHandler
235228
*/
236-
interface AnomalyDetectionComparison {
229+
export interface AnomalyDetectionComparison {
237230
seasonality:
238231
| 'auto'
239232
| 'hourly'
@@ -243,8 +236,8 @@ interface AnomalyDetectionComparison {
243236
| 'hourly_weekly'
244237
| 'hourly_daily_weekly'
245238
| 'daily_weekly';
246-
sensitivity: 'low' | 'medium' | 'high';
247-
threshold_type: 0 | 1 | 2;
239+
sensitivity: AlertRuleSensitivity;
240+
thresholdType: AlertRuleThresholdType;
248241
}
249242

250243
type MetricDataCondition = AnomalyDetectionComparison | number;

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -540,10 +540,14 @@ function DetectSection() {
540540
)}
541541
</Flex>
542542
</div>
543-
<DefineThresholdParagraph>
544-
<Text bold>{t('Resolve')}</Text>
545-
</DefineThresholdParagraph>
546-
<ResolveSection />
543+
{detectionType !== 'dynamic' && (
544+
<Fragment>
545+
<DefineThresholdParagraph>
546+
<Text bold>{t('Resolve')}</Text>
547+
</DefineThresholdParagraph>
548+
<ResolveSection />
549+
</Fragment>
550+
)}
547551
</Flex>
548552
</Container>
549553
);

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

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
DetectorPriorityLevel,
66
} from 'sentry/types/workflowEngine/dataConditions';
77
import type {
8+
AnomalyDetectionComparison,
89
Detector,
910
MetricCondition,
1011
MetricConditionGroup,
@@ -183,6 +184,22 @@ interface NewDataSource {
183184
timeWindow: number;
184185
}
185186

187+
function createAnomalyDetectionCondition(
188+
data: Pick<MetricDetectorFormData, 'sensitivity' | 'thresholdType'>
189+
): NewConditionGroup['conditions'] {
190+
return [
191+
{
192+
type: DataConditionType.ANOMALY_DETECTION,
193+
comparison: {
194+
sensitivity: data.sensitivity,
195+
seasonality: 'auto' as const,
196+
thresholdType: data.thresholdType,
197+
},
198+
conditionResult: DetectorPriorityLevel.HIGH,
199+
},
200+
];
201+
}
202+
186203
/**
187204
* Creates escalation conditions based on priority level and available thresholds
188205
*/
@@ -303,30 +320,30 @@ function createDataSource(data: MetricDetectorFormData): NewDataSource {
303320
export function metricDetectorFormDataToEndpointPayload(
304321
data: MetricDetectorFormData
305322
): MetricDetectorUpdatePayload {
306-
const conditions = createConditions(data);
323+
const conditions =
324+
data.detectionType === 'dynamic'
325+
? createAnomalyDetectionCondition(data)
326+
: createConditions(data);
327+
307328
const dataSource = createDataSource(data);
308329

309330
// Create config based on detection type
310331
let config: MetricDetectorConfig;
311332
switch (data.detectionType) {
312333
case 'percent':
313334
config = {
314-
thresholdPeriod: 1,
315335
detectionType: 'percent',
316336
comparisonDelta: data.conditionComparisonAgo || 3600,
317337
};
318338
break;
319339
case 'dynamic':
320340
config = {
321-
thresholdPeriod: 1,
322341
detectionType: 'dynamic',
323-
sensitivity: data.sensitivity,
324342
};
325343
break;
326344
case 'static':
327345
default:
328346
config = {
329-
thresholdPeriod: 1,
330347
detectionType: 'static',
331348
};
332349
break;
@@ -423,6 +440,24 @@ function processDetectorConditions(
423440
};
424441
}
425442

443+
function getAnomalyCondition(detector: MetricDetector): AnomalyDetectionComparison {
444+
const anomalyCondition = detector.conditionGroup?.conditions?.find(
445+
condition => condition.type === DataConditionType.ANOMALY_DETECTION
446+
);
447+
448+
const comparison = anomalyCondition?.comparison;
449+
if (typeof comparison === 'object') {
450+
return comparison;
451+
}
452+
453+
// Fallback to default values
454+
return {
455+
sensitivity: AlertRuleSensitivity.MEDIUM,
456+
seasonality: 'auto',
457+
thresholdType: AlertRuleThresholdType.ABOVE_AND_BELOW,
458+
};
459+
}
460+
426461
/**
427462
* Converts a Detector to MetricDetectorFormData for editing
428463
*/
@@ -444,6 +479,7 @@ export function metricSavedDetectorToFormData(
444479
: DetectorDataset.SPANS;
445480

446481
const datasetConfig = getDatasetConfig(dataset);
482+
const anomalyCondition = getAnomalyCondition(detector);
447483

448484
return {
449485
// Core detector fields
@@ -471,15 +507,8 @@ export function metricSavedDetectorToFormData(
471507
? detector.config.comparisonDelta
472508
: DEFAULT_THRESHOLD_METRIC_FORM_DATA.conditionComparisonAgo,
473509

474-
// Dynamic fields - extract from config for dynamic detectors
475-
sensitivity:
476-
detector.config.detectionType === 'dynamic' && defined(detector.config.sensitivity)
477-
? detector.config.sensitivity
478-
: DEFAULT_THRESHOLD_METRIC_FORM_DATA.sensitivity,
479-
thresholdType:
480-
detector.config.detectionType === 'dynamic' &&
481-
defined(detector.config.thresholdType)
482-
? detector.config.thresholdType
483-
: DEFAULT_THRESHOLD_METRIC_FORM_DATA.thresholdType,
510+
// Dynamic fields - extract from anomaly detection condition for dynamic detectors
511+
sensitivity: anomalyCondition.sensitivity,
512+
thresholdType: anomalyCondition.thresholdType,
484513
};
485514
}

static/app/views/detectors/edit.spec.tsx

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ import {
2424
DataConditionType,
2525
DetectorPriorityLevel,
2626
} from 'sentry/types/workflowEngine/dataConditions';
27-
import {Dataset, EventTypes} from 'sentry/views/alerts/rules/metric/types';
27+
import {
28+
AlertRuleSensitivity,
29+
AlertRuleThresholdType,
30+
Dataset,
31+
EventTypes,
32+
} from 'sentry/views/alerts/rules/metric/types';
2833
import {SnubaQueryType} from 'sentry/views/detectors/components/forms/metric/metricFormData';
2934
import DetectorEdit from 'sentry/views/detectors/edit';
3035

@@ -312,6 +317,9 @@ describe('DetectorEdit', () => {
312317
projectId: project.id,
313318
type: 'metric_issue',
314319
workflowIds: mockDetector.workflowIds,
320+
config: {
321+
detectionType: 'static',
322+
},
315323
dataSources: [
316324
{
317325
environment: 'production',
@@ -330,7 +338,6 @@ describe('DetectorEdit', () => {
330338
],
331339
logicType: 'any',
332340
},
333-
config: {detectionType: 'static', thresholdPeriod: 1},
334341
},
335342
})
336343
);
@@ -627,6 +634,56 @@ describe('DetectorEdit', () => {
627634
expect(await screen.findByText('15 minutes')).toBeInTheDocument();
628635
});
629636

637+
it('prefills thresholdType from anomaly detection condition when editing dynamic detector', async () => {
638+
const dynamicDetector = MetricDetectorFixture({
639+
name: 'Dynamic Detector',
640+
projectId: project.id,
641+
config: {
642+
detectionType: 'dynamic',
643+
},
644+
conditionGroup: {
645+
id: 'cg-dynamic',
646+
logicType: DataConditionGroupLogicType.ANY,
647+
conditions: [
648+
{
649+
id: 'c-anomaly',
650+
type: DataConditionType.ANOMALY_DETECTION,
651+
comparison: {
652+
sensitivity: AlertRuleSensitivity.HIGH,
653+
seasonality: 'auto',
654+
thresholdType: AlertRuleThresholdType.BELOW,
655+
},
656+
conditionResult: DetectorPriorityLevel.HIGH,
657+
},
658+
],
659+
},
660+
});
661+
662+
MockApiClient.addMockResponse({
663+
url: `/organizations/${organization.slug}/detectors/${dynamicDetector.id}/`,
664+
body: dynamicDetector,
665+
});
666+
667+
render(<DetectorEdit />, {
668+
organization,
669+
initialRouterConfig: {
670+
route: '/organizations/:orgId/monitors/:detectorId/edit/',
671+
location: {
672+
pathname: `/organizations/${organization.slug}/monitors/${dynamicDetector.id}/edit/`,
673+
},
674+
},
675+
});
676+
677+
expect(
678+
await screen.findByRole('link', {name: 'Dynamic Detector'})
679+
).toBeInTheDocument();
680+
681+
expect(screen.getByRole('radio', {name: 'Dynamic'})).toBeChecked();
682+
683+
// Verify thresholdType field is prefilled with "Below"
684+
expect(screen.getByText('Below')).toBeInTheDocument();
685+
});
686+
630687
it('calls anomaly API when using dynamic detection', async () => {
631688
MockApiClient.addMockResponse({
632689
url: `/organizations/${organization.slug}/detectors/${mockDetector.id}/`,

static/app/views/detectors/list/allMonitors.spec.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ describe('DetectorsList', () => {
5353
config: {
5454
detectionType: 'percent',
5555
comparisonDelta: 10,
56-
thresholdPeriod: 10,
5756
},
5857
conditionGroup: {
5958
id: '1',

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,6 @@ describe('DetectorEdit', () => {
197197
},
198198
config: {
199199
detectionType: 'static',
200-
thresholdPeriod: 1,
201200
},
202201
dataSources: [
203202
{
@@ -337,7 +336,7 @@ describe('DetectorEdit', () => {
337336
],
338337
logicType: 'any',
339338
},
340-
config: {detectionType: 'static', thresholdPeriod: 1},
339+
config: {detectionType: 'static'},
341340
dataSources: [
342341
{
343342
aggregate: 'count_unique(tags[sentry:user])',
@@ -407,7 +406,7 @@ describe('DetectorEdit', () => {
407406
],
408407
logicType: 'any',
409408
},
410-
config: {detectionType: 'static', thresholdPeriod: 1},
409+
config: {detectionType: 'static'},
411410
dataSources: [
412411
{
413412
aggregate: 'count()',
@@ -625,15 +624,23 @@ describe('DetectorEdit', () => {
625624
projectId: project.id,
626625
owner: null,
627626
workflowIds: [],
628-
// Dynamic detection should have empty conditions (no resolution thresholds)
627+
// Dynamic detection should have anomaly detection condition
629628
conditionGroup: {
630-
conditions: [],
629+
conditions: [
630+
{
631+
type: 'anomaly_detection',
632+
comparison: {
633+
sensitivity: 'high',
634+
seasonality: 'auto',
635+
thresholdType: 0,
636+
},
637+
conditionResult: 75,
638+
},
639+
],
631640
logicType: 'any',
632641
},
633642
config: {
634643
detectionType: 'dynamic',
635-
sensitivity: 'high',
636-
thresholdPeriod: 1,
637644
},
638645
dataSources: [
639646
{

tests/js/fixtures/detectors.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ export function MetricDetectorFixture(
7878
name: 'detector',
7979
config: {
8080
detectionType: 'static',
81-
thresholdPeriod: 1,
8281
},
8382
type: 'metric_issue',
8483
enabled: true,

0 commit comments

Comments
 (0)