diff --git a/static/app/components/workflowEngine/form/control/priorityControl.spec.tsx b/static/app/components/workflowEngine/form/control/priorityControl.spec.tsx index 3ea83da1012382..2b1718eb1aa685 100644 --- a/static/app/components/workflowEngine/form/control/priorityControl.spec.tsx +++ b/static/app/components/workflowEngine/form/control/priorityControl.spec.tsx @@ -118,7 +118,7 @@ describe('PriorityControl', () => { await userEvent.type(highField, '5'); expect(formModel.getError(METRIC_DETECTOR_FORM_FIELDS.highThreshold)).toBe( - 'High threshold must be higher than medium threshold' + 'High threshold must be higher than medium threshold (10)' ); // Test valid case: high (15) > medium (10) diff --git a/static/app/components/workflowEngine/form/control/priorityControl.tsx b/static/app/components/workflowEngine/form/control/priorityControl.tsx index a2510d6beb63bf..64181a48e54146 100644 --- a/static/app/components/workflowEngine/form/control/priorityControl.tsx +++ b/static/app/components/workflowEngine/form/control/priorityControl.tsx @@ -114,8 +114,9 @@ function validateHighThreshold({ !validateThresholdOrder(highNum, conditionNum, conditionType, true) ) { const message = t( - 'High threshold must be %s than medium threshold', - conditionType === DataConditionType.GREATER ? t('higher') : t('lower') + 'High threshold must be %s than medium threshold (%s)', + conditionType === DataConditionType.GREATER ? t('higher') : t('lower'), + String(conditionNum) ); return [createValidationError(METRIC_DETECTOR_FORM_FIELDS.highThreshold, message)]; } diff --git a/static/app/components/workflowEngine/ui/section.tsx b/static/app/components/workflowEngine/ui/section.tsx index 4c79afa16252b0..484a942090a0d2 100644 --- a/static/app/components/workflowEngine/ui/section.tsx +++ b/static/app/components/workflowEngine/ui/section.tsx @@ -6,7 +6,7 @@ type SectionProps = { title: React.ReactNode; children?: React.ReactNode; className?: string; - description?: string; + description?: React.ReactNode; }; export default function Section({children, className, title, description}: SectionProps) { @@ -25,12 +25,19 @@ export const SectionSubHeading = styled('h5')` margin: 0; `; +const SectionDescription = styled('div')` + font-size: ${p => p.theme.fontSize.md}; + font-weight: ${p => p.theme.fontWeight.normal}; + color: ${p => p.theme.subText}; + margin: 0; +`; + const SectionContainer = styled(Flex)` - > p { + > ${SectionDescription} { margin-bottom: ${p => p.theme.space['0']}; } - p + p { + ${SectionDescription} + ${SectionDescription} { margin-top: ${p => p.theme.space.md}; } `; @@ -41,9 +48,4 @@ const SectionHeading = styled('h4')` margin: 0; `; -const SectionDescription = styled('p')` - font-size: ${p => p.theme.fontSize.md}; - font-weight: ${p => p.theme.fontWeight.normal}; - color: ${p => p.theme.subText}; - margin: 0; -`; +// moved above to reference in SectionContainer diff --git a/static/app/views/detectors/components/forms/metric/metric.tsx b/static/app/views/detectors/components/forms/metric/metric.tsx index a1d6829b4dc75a..b632cca56f6987 100644 --- a/static/app/views/detectors/components/forms/metric/metric.tsx +++ b/static/app/views/detectors/components/forms/metric/metric.tsx @@ -49,6 +49,49 @@ import {DetectorDataset} from 'sentry/views/detectors/datasetConfig/types'; import {getResolutionDescription} from 'sentry/views/detectors/utils/getDetectorResolutionDescription'; import {getStaticDetectorThresholdSuffix} from 'sentry/views/detectors/utils/metricDetectorSuffix'; +function validateThresholdOrder( + value: number, + reference: number, + conditionType: DataConditionType, + isGreaterExpected: boolean +): boolean { + if (conditionType === DataConditionType.GREATER) { + return isGreaterExpected ? value > reference : value < reference; + } + // For LESS condition type, logic is inverted + return isGreaterExpected ? value < reference : value > reference; +} + +function validateResolutionThreshold({ + form, +}: { + form: MetricDetectorFormData; + id: string; +}): Array<[string, string]> { + const {conditionType, conditionValue, detectionType, resolutionStrategy} = form; + if (!conditionType || detectionType !== 'static' || resolutionStrategy !== 'manual') { + return []; + } + + const resolutionNum = Number(form.resolutionValue); + const conditionNum = Number(conditionValue); + + if ( + Number.isFinite(resolutionNum) && + Number.isFinite(conditionNum) && + !validateThresholdOrder(resolutionNum, conditionNum, conditionType, false) + ) { + const message = t( + 'Resolution threshold must be %s than alert threshold (%s)', + conditionType === DataConditionType.GREATER ? t('lower') : t('higher'), + String(conditionNum) + ); + return [[METRIC_DETECTOR_FORM_FIELDS.resolutionValue, message]]; + } + + return []; +} + function MetricDetectorForm() { return ( @@ -163,36 +206,96 @@ function ResolveSection() { const conditionComparisonAgo = useMetricDetectorFormField( METRIC_DETECTOR_FORM_FIELDS.conditionComparisonAgo ); + const resolutionStrategy = useMetricDetectorFormField( + METRIC_DETECTOR_FORM_FIELDS.resolutionStrategy + ); + const resolutionValue = useMetricDetectorFormField( + METRIC_DETECTOR_FORM_FIELDS.resolutionValue + ); const aggregate = useMetricDetectorFormField( METRIC_DETECTOR_FORM_FIELDS.aggregateFunction ); const thresholdSuffix = getStaticDetectorThresholdSuffix(aggregate); - const description = getResolutionDescription( - detectionType === 'percent' - ? { - detectionType: 'percent', - conditionType, - conditionValue, - comparisonDelta: conditionComparisonAgo ?? 3600, // Default to 1 hour if not set - thresholdSuffix, - } - : detectionType === 'static' - ? { - detectionType: 'static', - conditionType, - conditionValue, - thresholdSuffix, - } - : { - detectionType: 'dynamic', - thresholdSuffix, - } - ); + const effectiveConditionValue = + resolutionStrategy === 'manual' && + resolutionValue !== undefined && + resolutionValue !== '' + ? resolutionValue + : conditionValue; + + const descriptionContent = + detectionType === 'static' && resolutionStrategy === 'manual' ? ( + +
+ + {t( + 'Issue will be resolved when the query value is %s', + conditionType === DataConditionType.GREATER + ? t('less than') + : t('more than') + )} + +
+ +
+ ) : ( + getResolutionDescription( + detectionType === 'percent' + ? { + detectionType: 'percent', + conditionType, + conditionValue: effectiveConditionValue, + comparisonDelta: conditionComparisonAgo ?? 3600, // Default to 1 hour if not set + thresholdSuffix, + } + : detectionType === 'static' + ? { + detectionType: 'static', + conditionType, + conditionValue: effectiveConditionValue, + thresholdSuffix, + } + : { + detectionType: 'dynamic', + thresholdSuffix, + } + ) + ); return ( -
+
+ {detectionType !== 'dynamic' && ( + + + + )} + {descriptionContent} +
); } @@ -523,6 +626,22 @@ const DetectionTypeField = styled(SegmentedRadioField)` padding: 0; } `; + +const DescriptionContainer = styled('div')` + display: flex; + align-items: center; + min-height: 36px; +`; + +const ResolutionStrategyField = styled(SegmentedRadioField)` + padding: 0; + max-width: 350px; + + > div { + padding: 0; + } +`; + const ThresholdField = styled(NumberField)` padding: 0; margin: 0; @@ -530,7 +649,7 @@ const ThresholdField = styled(NumberField)` > div { padding: 0; - width: 10ch; + width: 18ch; } `; diff --git a/static/app/views/detectors/components/forms/metric/metricFormData.tsx b/static/app/views/detectors/components/forms/metric/metricFormData.tsx index e63546632ac0ba..ef068a97c0a0a6 100644 --- a/static/app/views/detectors/components/forms/metric/metricFormData.tsx +++ b/static/app/views/detectors/components/forms/metric/metricFormData.tsx @@ -63,6 +63,13 @@ interface MetricDetectorConditionFormData { * Both kind=threshold and kind=change */ conditionValue?: string; + /** + * Strategy for how an issue should be resolved + * - automatic: resolves based on the primary condition value + * - manual: resolves based on a custom resolution value + */ + resolutionStrategy?: 'automatic' | 'manual'; + resolutionValue?: string; } interface MetricDetectorDynamicFormData { @@ -120,6 +127,8 @@ export const METRIC_DETECTOR_FORM_FIELDS = { conditionComparisonAgo: 'conditionComparisonAgo', conditionType: 'conditionType', conditionValue: 'conditionValue', + resolutionStrategy: 'resolutionStrategy', + resolutionValue: 'resolutionValue', // Dynamic fields sensitivity: 'sensitivity', @@ -134,6 +143,8 @@ export const DEFAULT_THRESHOLD_METRIC_FORM_DATA = { initialPriorityLevel: DetectorPriorityLevel.HIGH, conditionType: DataConditionType.GREATER, conditionValue: '', + resolutionStrategy: 'automatic', + resolutionValue: '', conditionComparisonAgo: 60 * 60, // One hour in seconds // Default dynamic fields @@ -177,7 +188,12 @@ interface NewDataSource { export function createConditions( data: Pick< MetricDetectorFormData, - 'conditionType' | 'conditionValue' | 'initialPriorityLevel' | 'highThreshold' + | 'conditionType' + | 'conditionValue' + | 'initialPriorityLevel' + | 'highThreshold' + | 'resolutionStrategy' + | 'resolutionValue' > ): NewConditionGroup['conditions'] { if (!defined(data.conditionType) || !defined(data.conditionValue)) { @@ -206,6 +222,24 @@ export function createConditions( }); } + // Optionally add explicit resolution (OK) condition when manual strategy is chosen + if ( + data.resolutionStrategy === 'manual' && + defined(data.resolutionValue) && + data.resolutionValue !== '' + ) { + const resolutionConditionType = + data.conditionType === DataConditionType.GREATER + ? DataConditionType.LESS + : DataConditionType.GREATER; + + conditions.push({ + type: resolutionConditionType, + comparison: parseFloat(data.resolutionValue) || 0, + conditionResult: DetectorPriorityLevel.OK, + }); + } + return conditions; } @@ -319,7 +353,10 @@ export function metricDetectorFormDataToEndpointPayload( function processDetectorConditions( detector: MetricDetector ): PrioritizeLevelFormData & - Pick { + Pick< + MetricDetectorFormData, + 'conditionValue' | 'conditionType' | 'resolutionStrategy' | 'resolutionValue' + > { // Get conditions from the condition group const conditions = detector.conditionGroup?.conditions || []; // Sort by priority level, lowest first @@ -337,6 +374,11 @@ function processDetectorConditions( condition => condition.conditionResult === DetectorPriorityLevel.HIGH ); + // Find explicit resolution (OK) condition, if present + const okCondition = conditions.find( + condition => condition.conditionResult === DetectorPriorityLevel.OK + ); + // Determine initial priority level, ensuring it's valid for the form let initialPriorityLevel: DetectorPriorityLevel.MEDIUM | DetectorPriorityLevel.HIGH = DetectorPriorityLevel.MEDIUM; @@ -368,6 +410,12 @@ function processDetectorConditions( typeof highCondition?.comparison === 'number' ? highCondition.comparison.toString() : '', + resolutionStrategy: + typeof okCondition?.comparison === 'number' ? 'manual' : 'automatic', + resolutionValue: + typeof okCondition?.comparison === 'number' + ? okCondition.comparison.toString() + : '', }; } diff --git a/static/app/views/detectors/components/forms/metric/previewChart.tsx b/static/app/views/detectors/components/forms/metric/previewChart.tsx index 9dd281540d36c9..b99893a03701c7 100644 --- a/static/app/views/detectors/components/forms/metric/previewChart.tsx +++ b/static/app/views/detectors/components/forms/metric/previewChart.tsx @@ -33,6 +33,12 @@ export function MetricDetectorPreviewChart() { const initialPriorityLevel = useMetricDetectorFormField( METRIC_DETECTOR_FORM_FIELDS.initialPriorityLevel ); + const resolutionStrategy = useMetricDetectorFormField( + METRIC_DETECTOR_FORM_FIELDS.resolutionStrategy + ); + const resolutionValue = useMetricDetectorFormField( + METRIC_DETECTOR_FORM_FIELDS.resolutionValue + ); const detectionType = useMetricDetectorFormField( METRIC_DETECTOR_FORM_FIELDS.detectionType ); @@ -56,8 +62,18 @@ export function MetricDetectorPreviewChart() { conditionValue, initialPriorityLevel, highThreshold, + resolutionStrategy, + resolutionValue, }); - }, [conditionType, conditionValue, initialPriorityLevel, highThreshold, detectionType]); + }, [ + conditionType, + conditionValue, + initialPriorityLevel, + highThreshold, + resolutionStrategy, + resolutionValue, + detectionType, + ]); const datasetConfig = getDatasetConfig(dataset); const {query, eventTypes} = datasetConfig.separateEventTypesFromQuery(rawQuery); diff --git a/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx b/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx index 88b7cf15fb73ac..f8b7471d9cd12c 100644 --- a/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx +++ b/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx @@ -87,6 +87,7 @@ function extractThresholdsFromConditions( type: DataConditionType; value: number; }>; + resolution?: {type: DataConditionType; value: number}; } { const thresholds = conditions .filter( @@ -95,13 +96,22 @@ function extractThresholdsFromConditions( typeof condition.comparison === 'number' ) .map(condition => ({ - value: condition.comparison as number, + value: Number(condition.comparison), priority: condition.conditionResult || DetectorPriorityLevel.MEDIUM, type: condition.type, })) .sort((a, b) => a.value - b.value); - return {thresholds}; + const resolutionCondition = conditions.find( + condition => condition.conditionResult === DetectorPriorityLevel.OK + ); + + const resolution = + resolutionCondition && typeof resolutionCondition.comparison === 'number' + ? {type: resolutionCondition.type, value: Number(resolutionCondition.comparison)} + : undefined; + + return {thresholds, resolution}; } interface UseMetricDetectorThresholdSeriesProps { @@ -133,7 +143,7 @@ export function useMetricDetectorThresholdSeries({ return {maxValue: undefined, additionalSeries: []}; } - const {thresholds} = extractThresholdsFromConditions(conditions); + const {thresholds, resolution} = extractThresholdsFromConditions(conditions); const additional: LineSeriesOption[] = []; if (detectionType === 'percent') { @@ -216,7 +226,26 @@ export function useMetricDetectorThresholdSeries({ }); additional.push(...thresholdSeries); - const maxValue = Math.max(...thresholds.map(threshold => threshold.value)); + // Add manual resolution line and safe area if present (green) + if (resolution) { + const resolutionSeries: LineSeriesOption = { + type: 'line', + markLine: createThresholdMarkLine(theme.green300, resolution.value), + markArea: createThresholdMarkArea( + theme.green300, + resolution.value, + resolution.type === DataConditionType.GREATER + ), + data: [], + }; + additional.push(resolutionSeries); + } + + const valuesForMax = [ + ...thresholds.map(threshold => threshold.value), + ...(resolution ? [resolution.value] : []), + ]; + const maxValue = valuesForMax.length > 0 ? Math.max(...valuesForMax) : undefined; return {maxValue, additionalSeries: additional}; } diff --git a/static/app/views/detectors/new-setting.spec.tsx b/static/app/views/detectors/new-setting.spec.tsx index 5700d0234d9bfe..d9a1bdf81b9554 100644 --- a/static/app/views/detectors/new-setting.spec.tsx +++ b/static/app/views/detectors/new-setting.spec.tsx @@ -291,6 +291,61 @@ describe('DetectorEdit', () => { ); }); }, 10_000); + + it('submits manual resolution threshold when selected', async () => { + const mockCreateDetector = MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/detectors/`, + method: 'POST', + body: MetricDetectorFixture({id: '321'}), + }); + + render(, { + organization, + initialRouterConfig: metricRouterConfig, + }); + + // Set initial trigger threshold + await userEvent.type(screen.getByRole('spinbutton', {name: 'Threshold'}), '100'); + + // Enable manual resolution and set resolution threshold + await userEvent.click(screen.getByRole('radio', {name: 'Manual'})); + await userEvent.type( + screen.getByRole('spinbutton', {name: 'Resolution threshold'}), + '80' + ); + + await userEvent.click(screen.getByRole('button', {name: 'Create Monitor'})); + + await waitFor(() => { + expect(mockCreateDetector).toHaveBeenCalled(); + }); + + expect(mockCreateDetector).toHaveBeenCalledWith( + `/organizations/${organization.slug}/detectors/`, + expect.objectContaining({ + data: expect.objectContaining({ + type: 'metric_issue', + conditionGroup: { + logicType: 'any', + conditions: [ + // Main trigger condition at HIGH + { + comparison: 100, + conditionResult: 75, + type: 'gt', + }, + // Manual resolution condition at OK + { + comparison: 80, + conditionResult: 0, + type: 'lt', + }, + ], + }, + }), + }) + ); + }); }); describe('Uptime Detector', () => {