Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions static/app/types/workflowEngine/dataConditions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export enum DataConditionType {
EVENT_FREQUENCY = 'event_frequency',
EVENT_UNIQUE_USER_FREQUENCY = 'event_unique_user_frequency',
PERCENT_SESSIONS = 'percent_sessions',
ANOMALY_DETECTION = 'anomaly_detection',
}

export enum DataConditionGroupLogicType {
Expand Down
18 changes: 7 additions & 11 deletions static/app/types/workflowEngine/detectors.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,32 +80,28 @@ export type DetectorType =
| 'uptime_domain_failure'
| 'issue_stream';

interface BaseMetricDetectorConfig {
thresholdPeriod: number;
}

/**
* Configuration for static/threshold-based detection
*/
interface MetricDetectorConfigStatic extends BaseMetricDetectorConfig {
interface MetricDetectorConfigStatic {
detectionType: 'static';
}

/**
* Configuration for percentage-based change detection
*/
interface MetricDetectorConfigPercent extends BaseMetricDetectorConfig {
interface MetricDetectorConfigPercent {
comparisonDelta: number;
detectionType: 'percent';
}

/**
* Configuration for dynamic/anomaly detection
*/
interface MetricDetectorConfigDynamic extends BaseMetricDetectorConfig {
interface MetricDetectorConfigDynamic {
detectionType: 'dynamic';
seasonality?: 'auto' | 'daily' | 'weekly' | 'monthly';
sensitivity?: AlertRuleSensitivity;
sensitivity?: 'low' | 'medium' | 'high';
Copy link
Contributor

Choose a reason for hiding this comment

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

Sensitivity needs to be removed too 😅

thresholdType?: AlertRuleThresholdType;
}

Expand Down Expand Up @@ -233,7 +229,7 @@ export interface MetricCondition {
/**
* See AnomalyDetectionHandler
*/
interface AnomalyDetectionComparison {
export interface AnomalyDetectionComparison {
seasonality:
| 'auto'
| 'hourly'
Expand All @@ -243,8 +239,8 @@ interface AnomalyDetectionComparison {
| 'hourly_weekly'
| 'hourly_daily_weekly'
| 'daily_weekly';
sensitivity: 'low' | 'medium' | 'high';
threshold_type: 0 | 1 | 2;
sensitivity: AlertRuleSensitivity;
thresholdType: AlertRuleThresholdType;
}

type MetricDataCondition = AnomalyDetectionComparison | number;
12 changes: 8 additions & 4 deletions static/app/views/detectors/components/forms/metric/metric.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -584,10 +584,14 @@ function DetectSection() {
)}
</Flex>
</div>
<DefineThresholdParagraph>
<Text bold>{t('Resolve')}</Text>
</DefineThresholdParagraph>
<ResolveSection />
{detectionType !== 'dynamic' && (
<Fragment>
<DefineThresholdParagraph>
<Text bold>{t('Resolve')}</Text>
</DefineThresholdParagraph>
<ResolveSection />
</Fragment>
)}
</Flex>
</Container>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
DetectorPriorityLevel,
} from 'sentry/types/workflowEngine/dataConditions';
import type {
AnomalyDetectionComparison,
Detector,
MetricCondition,
MetricConditionGroup,
Expand Down Expand Up @@ -183,6 +184,22 @@ interface NewDataSource {
timeWindow: number;
}

function createAnomalyDetectionCondition(
data: Pick<MetricDetectorFormData, 'sensitivity' | 'thresholdType'>
): NewConditionGroup['conditions'] {
return [
{
type: DataConditionType.ANOMALY_DETECTION,
comparison: {
sensitivity: data.sensitivity,
seasonality: 'auto' as const,
thresholdType: data.thresholdType,
},
conditionResult: DetectorPriorityLevel.HIGH,
},
];
}

/**
* Creates escalation conditions based on priority level and available thresholds
*/
Expand Down Expand Up @@ -303,30 +320,31 @@ function createDataSource(data: MetricDetectorFormData): NewDataSource {
export function metricDetectorFormDataToEndpointPayload(
data: MetricDetectorFormData
): MetricDetectorUpdatePayload {
const conditions = createConditions(data);
const conditions =
data.detectionType === 'dynamic'
? createAnomalyDetectionCondition(data)
: createConditions(data);

const dataSource = createDataSource(data);

// Create config based on detection type
let config: MetricDetectorConfig;
switch (data.detectionType) {
case 'percent':
config = {
thresholdPeriod: 1,
detectionType: 'percent',
comparisonDelta: data.conditionComparisonAgo || 3600,
};
break;
case 'dynamic':
config = {
thresholdPeriod: 1,
detectionType: 'dynamic',
sensitivity: data.sensitivity,
};
break;
case 'static':
default:
config = {
thresholdPeriod: 1,
detectionType: 'static',
};
break;
Expand Down Expand Up @@ -423,6 +441,24 @@ function processDetectorConditions(
};
}

function getAnomalyCondition(detector: MetricDetector): AnomalyDetectionComparison {
const anomalyCondition = detector.conditionGroup?.conditions?.find(
condition => condition.type === DataConditionType.ANOMALY_DETECTION
);

const comparison = anomalyCondition?.comparison;
if (typeof comparison === 'object') {
return comparison;
}

// Fallback to default values
return {
sensitivity: AlertRuleSensitivity.MEDIUM,
seasonality: 'auto',
thresholdType: AlertRuleThresholdType.ABOVE_AND_BELOW,
};
}

/**
* Converts a Detector to MetricDetectorFormData for editing
*/
Expand All @@ -444,6 +480,7 @@ export function metricSavedDetectorToFormData(
: DetectorDataset.SPANS;

const datasetConfig = getDatasetConfig(dataset);
const anomalyCondition = getAnomalyCondition(detector);

return {
// Core detector fields
Expand Down Expand Up @@ -471,15 +508,8 @@ export function metricSavedDetectorToFormData(
? detector.config.comparisonDelta
: DEFAULT_THRESHOLD_METRIC_FORM_DATA.conditionComparisonAgo,

// Dynamic fields - extract from config for dynamic detectors
sensitivity:
detector.config.detectionType === 'dynamic' && defined(detector.config.sensitivity)
? detector.config.sensitivity
: DEFAULT_THRESHOLD_METRIC_FORM_DATA.sensitivity,
thresholdType:
detector.config.detectionType === 'dynamic' &&
defined(detector.config.thresholdType)
? detector.config.thresholdType
: DEFAULT_THRESHOLD_METRIC_FORM_DATA.thresholdType,
// Dynamic fields - extract from anomaly detection condition for dynamic detectors
sensitivity: anomalyCondition.sensitivity,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should also be removed here

Copy link
Member Author

Choose a reason for hiding this comment

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

this one is sensitivity

thresholdType: anomalyCondition.thresholdType,
};
}
61 changes: 59 additions & 2 deletions static/app/views/detectors/edit.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ import {
DataConditionType,
DetectorPriorityLevel,
} from 'sentry/types/workflowEngine/dataConditions';
import {Dataset, EventTypes} from 'sentry/views/alerts/rules/metric/types';
import {
AlertRuleSensitivity,
AlertRuleThresholdType,
Dataset,
EventTypes,
} from 'sentry/views/alerts/rules/metric/types';
import {SnubaQueryType} from 'sentry/views/detectors/components/forms/metric/metricFormData';
import DetectorEdit from 'sentry/views/detectors/edit';

Expand Down Expand Up @@ -312,6 +317,9 @@ describe('DetectorEdit', () => {
projectId: project.id,
type: 'metric_issue',
workflowIds: mockDetector.workflowIds,
config: {
detectionType: 'static',
},
dataSources: [
{
environment: 'production',
Expand All @@ -330,7 +338,6 @@ describe('DetectorEdit', () => {
],
logicType: 'any',
},
config: {detectionType: 'static', thresholdPeriod: 1},
},
})
);
Expand Down Expand Up @@ -627,6 +634,56 @@ describe('DetectorEdit', () => {
expect(await screen.findByText('15 minutes')).toBeInTheDocument();
});

it('prefills thresholdType from anomaly detection condition when editing dynamic detector', async () => {
const dynamicDetector = MetricDetectorFixture({
name: 'Dynamic Detector',
projectId: project.id,
config: {
detectionType: 'dynamic',
},
conditionGroup: {
id: 'cg-dynamic',
logicType: DataConditionGroupLogicType.ANY,
conditions: [
{
id: 'c-anomaly',
type: DataConditionType.ANOMALY_DETECTION,
comparison: {
sensitivity: AlertRuleSensitivity.HIGH,
seasonality: 'auto',
thresholdType: AlertRuleThresholdType.BELOW,
},
conditionResult: DetectorPriorityLevel.HIGH,
},
],
},
});

MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/detectors/${dynamicDetector.id}/`,
body: dynamicDetector,
});

render(<DetectorEdit />, {
organization,
initialRouterConfig: {
route: '/organizations/:orgId/monitors/:detectorId/edit/',
location: {
pathname: `/organizations/${organization.slug}/monitors/${dynamicDetector.id}/edit/`,
},
},
});

expect(
await screen.findByRole('link', {name: 'Dynamic Detector'})
).toBeInTheDocument();

expect(screen.getByRole('radio', {name: 'Dynamic'})).toBeChecked();

// Verify thresholdType field is prefilled with "Below"
expect(screen.getByText('Below')).toBeInTheDocument();
});

it('calls anomaly API when using dynamic detection', async () => {
MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/detectors/${mockDetector.id}/`,
Expand Down
1 change: 0 additions & 1 deletion static/app/views/detectors/list/allMonitors.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ describe('DetectorsList', () => {
config: {
detectionType: 'percent',
comparisonDelta: 10,
thresholdPeriod: 10,
},
conditionGroup: {
id: '1',
Expand Down
20 changes: 14 additions & 6 deletions static/app/views/detectors/new-setting.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ describe('DetectorEdit', () => {
},
config: {
detectionType: 'static',
thresholdPeriod: 1,
},
dataSources: [
{
Expand Down Expand Up @@ -282,7 +281,7 @@ describe('DetectorEdit', () => {
],
logicType: 'any',
},
config: {detectionType: 'static', thresholdPeriod: 1},
config: {detectionType: 'static'},
dataSources: [
{
aggregate: 'count_unique(tags[sentry:user])',
Expand Down Expand Up @@ -352,7 +351,7 @@ describe('DetectorEdit', () => {
],
logicType: 'any',
},
config: {detectionType: 'static', thresholdPeriod: 1},
config: {detectionType: 'static'},
dataSources: [
{
aggregate: 'count()',
Expand Down Expand Up @@ -570,15 +569,24 @@ describe('DetectorEdit', () => {
projectId: project.id,
owner: null,
workflowIds: [],
// Dynamic detection should have empty conditions (no resolution thresholds)
// Dynamic detection should have anomaly detection condition
conditionGroup: {
conditions: [],
conditions: [
{
type: 'anomaly_detection',
comparison: {
sensitivity: 'high',
seasonality: 'auto',
thresholdType: 0,
},
conditionResult: 75,
},
],
logicType: 'any',
},
config: {
detectionType: 'dynamic',
sensitivity: 'high',
thresholdPeriod: 1,
},
dataSources: [
{
Expand Down
1 change: 0 additions & 1 deletion tests/js/fixtures/detectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ export function MetricDetectorFixture(
name: 'detector',
config: {
detectionType: 'static',
thresholdPeriod: 1,
},
type: 'metric_issue',
enabled: true,
Expand Down
Loading