Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)];
}
Expand Down
20 changes: 11 additions & 9 deletions static/app/components/workflowEngine/ui/section.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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};
}
`;
Expand All @@ -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
165 changes: 142 additions & 23 deletions static/app/views/detectors/components/forms/metric/metric.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<FormStack>
Expand Down Expand Up @@ -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' ? (
<Flex align="center" gap="sm">
<div>
<span>
{t(
'Issue will be resolved when the query value is %s',
conditionType === DataConditionType.GREATER
? t('less than')
: t('more than')
)}
</span>
</div>
<ThresholdField
aria-label={t('Resolution threshold')}
name={METRIC_DETECTOR_FORM_FIELDS.resolutionValue}
hideLabel
inline={false}
flexibleControlStateSize
placeholder="0"
suffix={thresholdSuffix}
validate={validateResolutionThreshold}
required
preserveOnUnmount
/>
</Flex>
) : (
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 (
<Container>
<Section title={t('Resolve')} description={description} />
<Section title={t('Resolve')}>
{detectionType !== 'dynamic' && (
<Flex direction="column" gap="sm">
<ResolutionStrategyField
name={METRIC_DETECTOR_FORM_FIELDS.resolutionStrategy}
label={t('Resolution method')}
hideLabel
inline={false}
flexibleControlStateSize
preserveOnUnmount
defaultValue="automatic"
choices={[
['automatic', t('Automatic')],
['manual', t('Manual')],
]}
/>
</Flex>
)}
<DescriptionContainer>{descriptionContent}</DescriptionContainer>
</Section>
</Container>
);
}
Expand Down Expand Up @@ -523,14 +626,30 @@ 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;
border: none;

> div {
padding: 0;
width: 10ch;
width: 18ch;
}
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -120,6 +127,8 @@ export const METRIC_DETECTOR_FORM_FIELDS = {
conditionComparisonAgo: 'conditionComparisonAgo',
conditionType: 'conditionType',
conditionValue: 'conditionValue',
resolutionStrategy: 'resolutionStrategy',
resolutionValue: 'resolutionValue',

// Dynamic fields
sensitivity: 'sensitivity',
Expand All @@ -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
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -319,7 +353,10 @@ export function metricDetectorFormDataToEndpointPayload(
function processDetectorConditions(
detector: MetricDetector
): PrioritizeLevelFormData &
Pick<MetricDetectorFormData, 'conditionValue' | 'conditionType'> {
Pick<
MetricDetectorFormData,
'conditionValue' | 'conditionType' | 'resolutionStrategy' | 'resolutionValue'
> {
// Get conditions from the condition group
const conditions = detector.conditionGroup?.conditions || [];
// Sort by priority level, lowest first
Expand All @@ -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;
Expand Down Expand Up @@ -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()
: '',
};
}

Expand Down
Loading
Loading