diff --git a/static/app/views/detectors/components/details/metric/chart.tsx b/static/app/views/detectors/components/details/metric/chart.tsx index 691cbf59b345b7..2c824f5cce645e 100644 --- a/static/app/views/detectors/components/details/metric/chart.tsx +++ b/static/app/views/detectors/components/details/metric/chart.tsx @@ -40,7 +40,7 @@ function MetricDetectorChart({ const comparisonDelta = detectionType === 'percent' ? detector.config.comparisonDelta : undefined; const dataset = getDetectorDataset(snubaQuery.dataset, snubaQuery.eventTypes); - const {series, comparisonSeries, isLoading, isError} = useMetricDetectorSeries({ + const {series, comparisonSeries, isLoading, error} = useMetricDetectorSeries({ dataset, aggregate: snubaQuery.aggregate, interval: snubaQuery.timeWindow, @@ -140,7 +140,7 @@ function MetricDetectorChart({ ); } - if (isError) { + if (error) { return ( diff --git a/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx b/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx index 46caac6974ded0..2bb400c085d9ff 100644 --- a/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx +++ b/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx @@ -1,28 +1,52 @@ -import {useMemo} from 'react'; +import {Fragment, useMemo} from 'react'; +import styled from '@emotion/styled'; import type {YAXisComponentOption} from 'echarts'; import {AreaChart} from 'sentry/components/charts/areaChart'; import ErrorPanel from 'sentry/components/charts/errorPanel'; +import {CompactSelect} from 'sentry/components/core/compactSelect'; import {Flex} from 'sentry/components/core/layout'; +import {Text} from 'sentry/components/core/text'; +import LoadingIndicator from 'sentry/components/loadingIndicator'; import Placeholder from 'sentry/components/placeholder'; import {IconWarning} from 'sentry/icons'; -import {t} from 'sentry/locale'; +import {t, tn} from 'sentry/locale'; import {space} from 'sentry/styles/space'; import type {DataCondition} from 'sentry/types/workflowEngine/dataConditions'; import type {MetricDetectorConfig} from 'sentry/types/workflowEngine/detectors'; import { AlertRuleSensitivity, AlertRuleThresholdType, - TimePeriod, } from 'sentry/views/alerts/rules/metric/types'; +import {getBackendDataset} from 'sentry/views/detectors/components/forms/metric/metricFormData'; import type {DetectorDataset} from 'sentry/views/detectors/datasetConfig/types'; import {useIncidentMarkers} from 'sentry/views/detectors/hooks/useIncidentMarkers'; import {useMetricDetectorAnomalyPeriods} from 'sentry/views/detectors/hooks/useMetricDetectorAnomalyPeriods'; import {useMetricDetectorSeries} from 'sentry/views/detectors/hooks/useMetricDetectorSeries'; import {useMetricDetectorThresholdSeries} from 'sentry/views/detectors/hooks/useMetricDetectorThresholdSeries'; +import {useTimePeriodSelection} from 'sentry/views/detectors/hooks/useTimePeriodSelection'; const CHART_HEIGHT = 180; +function ChartError() { + return ( + + + +
{t('Error loading chart data')}
+
+
+ ); +} + +function ChartLoading() { + return ( + + + + ); +} + interface MetricDetectorChartProps { /** * The aggregate function to use (e.g., "avg(span.duration)") @@ -61,10 +85,6 @@ interface MetricDetectorChartProps { * Used in anomaly detection */ sensitivity: AlertRuleSensitivity | undefined; - /** - * The time period for the chart data (optional, defaults to 7d) - */ - statsPeriod: TimePeriod; /** * Used in anomaly detection */ @@ -80,19 +100,24 @@ export function MetricDetectorChart({ projectId, conditions, detectionType, - statsPeriod, comparisonDelta, sensitivity, thresholdType, }: MetricDetectorChartProps) { - const {series, comparisonSeries, isLoading, isError} = useMetricDetectorSeries({ + const {selectedTimePeriod, setSelectedTimePeriod, timePeriodOptions} = + useTimePeriodSelection({ + dataset: getBackendDataset(dataset), + interval, + }); + + const {series, comparisonSeries, isLoading, error} = useMetricDetectorSeries({ dataset, aggregate, interval, query, environment, projectId, - statsPeriod, + statsPeriod: selectedTimePeriod, comparisonDelta, }); @@ -105,25 +130,26 @@ export function MetricDetectorChart({ const isAnomalyDetection = detectionType === 'dynamic'; const shouldFetchAnomalies = - isAnomalyDetection && !isLoading && !isError && series.length > 0; + isAnomalyDetection && !isLoading && !error && series.length > 0; // Fetch anomaly data when detection type is dynamic and series data is ready const { anomalyPeriods, isLoading: isLoadingAnomalies, - error: anomalyErrorObject, + error: anomalyError, } = useMetricDetectorAnomalyPeriods({ series: shouldFetchAnomalies ? series : [], + isLoadingSeries: isLoading, dataset, aggregate, query, environment, projectId, - statsPeriod, - timePeriod: interval, + statsPeriod: selectedTimePeriod, + interval, thresholdType, sensitivity, - enabled: shouldFetchAnomalies, + enabled: isAnomalyDetection, }); // Create anomaly marker rendering from pre-grouped anomaly periods @@ -134,9 +160,6 @@ export function MetricDetectorChart({ yAxisIndex: 1, // Use index 1 to avoid conflict with main chart axis }); - const anomalyLoading = shouldFetchAnomalies ? isLoadingAnomalies : false; - const anomalyError = shouldFetchAnomalies ? anomalyErrorObject : null; - // Calculate y-axis bounds to ensure all thresholds are visible const maxValue = useMemo(() => { // Get max from series data @@ -219,40 +242,80 @@ export function MetricDetectorChart({ return baseGrid; }, [isAnomalyDetection, anomalyMarkerResult.incidentMarkerGrid]); - if (isLoading || anomalyLoading) { - return ( - - - - ); - } - - if (isError || anomalyError) { - return ( - - - -
{t('Error loading chart data')}
-
-
- ); - } - return ( - 1 ? yAxes : undefined} - yAxis={yAxes.length === 1 ? yAxes[0] : undefined} - grid={grid} - xAxis={isAnomalyDetection ? anomalyMarkerResult.incidentMarkerXAxis : undefined} - ref={ - isAnomalyDetection ? anomalyMarkerResult.connectIncidentMarkerChartRef : undefined - } - /> + + {isLoading ? ( + + ) : error ? ( + + ) : ( + 1 ? yAxes : undefined} + yAxis={yAxes.length === 1 ? yAxes[0] : undefined} + grid={grid} + xAxis={isAnomalyDetection ? anomalyMarkerResult.incidentMarkerXAxis : undefined} + ref={ + isAnomalyDetection + ? anomalyMarkerResult.connectIncidentMarkerChartRef + : undefined + } + /> + )} + + {shouldFetchAnomalies ? ( + + {isLoadingAnomalies ? ( + + + {t('Loading anomalies...')} + + ) : anomalyError ? ( + {t('Error loading anomalies')} + ) : anomalyPeriods.length === 0 ? ( + {t('No anomalies found for this time period')} + ) : ( + + {tn('Found %s anomaly', 'Found %s anomalies', anomalyPeriods.length)} + + )} + + ) : ( +
+ )} + setSelectedTimePeriod(opt.value)} + triggerProps={{ + borderless: true, + prefix: t('Display'), + }} + /> + + ); } + +const ChartContainer = styled('div')` + max-width: 1440px; + border-top: 1px solid ${p => p.theme.border}; +`; + +const ChartFooter = styled('div')` + display: flex; + justify-content: space-between; + align-items: center; + padding: ${p => `${p.theme.space.sm} 0 ${p.theme.space.sm} ${p.theme.space.lg}`}; + border-top: 1px solid ${p => p.theme.border}; +`; + +const AnomalyLoadingIndicator = styled(LoadingIndicator)` + margin: 0; +`; diff --git a/static/app/views/detectors/components/forms/metric/previewChart.tsx b/static/app/views/detectors/components/forms/metric/previewChart.tsx index 88bce5bb01a425..6d948a17cf53c7 100644 --- a/static/app/views/detectors/components/forms/metric/previewChart.tsx +++ b/static/app/views/detectors/components/forms/metric/previewChart.tsx @@ -1,16 +1,11 @@ import {useMemo} from 'react'; -import styled from '@emotion/styled'; -import {CompactSelect} from 'sentry/components/core/compactSelect'; -import {t} from 'sentry/locale'; import {MetricDetectorChart} from 'sentry/views/detectors/components/forms/metric/metricDetectorChart'; import { createConditions, - getBackendDataset, METRIC_DETECTOR_FORM_FIELDS, useMetricDetectorFormField, } from 'sentry/views/detectors/components/forms/metric/metricFormData'; -import {useTimePeriodSelection} from 'sentry/views/detectors/hooks/useTimePeriodSelection'; export function MetricDetectorPreviewChart() { // Get all the form fields needed for the chart @@ -47,12 +42,6 @@ export function MetricDetectorPreviewChart() { METRIC_DETECTOR_FORM_FIELDS.thresholdType ); - const {selectedTimePeriod, setSelectedTimePeriod, timePeriodOptions} = - useTimePeriodSelection({ - dataset: getBackendDataset(dataset), - interval, - }); - // Create condition group from form data using the helper function const conditions = useMemo(() => { // Wait for a condition value to be defined @@ -69,46 +58,18 @@ export function MetricDetectorPreviewChart() { }, [conditionType, conditionValue, initialPriorityLevel, highThreshold, detectionType]); return ( - - - - setSelectedTimePeriod(opt.value)} - triggerProps={{ - borderless: true, - prefix: t('Display'), - }} - /> - - + ); } - -const ChartContainer = styled('div')` - max-width: 1440px; - border-top: 1px solid ${p => p.theme.border}; -`; - -const ChartFooter = styled('div')` - display: flex; - justify-content: flex-end; - align-items: center; - padding: ${p => `${p.theme.space.sm} 0`}; - border-top: 1px solid ${p => p.theme.border}; -`; diff --git a/static/app/views/detectors/components/forms/metric/useIntervalChoices.tsx b/static/app/views/detectors/components/forms/metric/useIntervalChoices.tsx index 3e6564b51ef1d5..3c45218cc1eb7c 100644 --- a/static/app/views/detectors/components/forms/metric/useIntervalChoices.tsx +++ b/static/app/views/detectors/components/forms/metric/useIntervalChoices.tsx @@ -2,8 +2,9 @@ import {useMemo} from 'react'; import getDuration from 'sentry/utils/duration/getDuration'; import {TimeWindow} from 'sentry/views/alerts/rules/metric/types'; -import {type MetricDetectorFormData} from 'sentry/views/detectors/components/forms/metric/metricFormData'; +import type {MetricDetectorFormData} from 'sentry/views/detectors/components/forms/metric/metricFormData'; import {DetectorDataset} from 'sentry/views/detectors/datasetConfig/types'; +import {isEapDataset} from 'sentry/views/detectors/datasetConfig/utils/isEapDataset'; const baseIntervals: TimeWindow[] = [ TimeWindow.ONE_MINUTE, @@ -41,9 +42,6 @@ export function useIntervalChoices({dataset, detectionType}: UseIntervalChoicesP // 4. Everything else → All intervals allowed const shouldExcludeSubHour = dataset === DetectorDataset.RELEASES; const isDynamicDetection = detectionType === 'dynamic'; - // EAP-derived datasets (spans, logs) exclude 1-minute intervals - const isEAPDerivedDataset = - dataset === DetectorDataset.SPANS || dataset === DetectorDataset.LOGS; const filteredIntervals = baseIntervals.filter(timeWindow => { if (shouldExcludeSubHour) { @@ -52,7 +50,8 @@ export function useIntervalChoices({dataset, detectionType}: UseIntervalChoicesP if (isDynamicDetection) { return dynamicIntervalChoices.includes(timeWindow); } - if (isEAPDerivedDataset) { + // EAP-derived datasets (spans, logs) exclude 1-minute intervals + if (isEapDataset(dataset)) { return timeWindow !== TimeWindow.ONE_MINUTE; } return true; diff --git a/static/app/views/detectors/datasetConfig/utils/isEapDataset.tsx b/static/app/views/detectors/datasetConfig/utils/isEapDataset.tsx new file mode 100644 index 00000000000000..b255da5b534963 --- /dev/null +++ b/static/app/views/detectors/datasetConfig/utils/isEapDataset.tsx @@ -0,0 +1,5 @@ +import {DetectorDataset} from 'sentry/views/detectors/datasetConfig/types'; + +export function isEapDataset(dataset: DetectorDataset): boolean { + return dataset === DetectorDataset.SPANS || dataset === DetectorDataset.LOGS; +} diff --git a/static/app/views/detectors/hooks/useMetricDetectorAnomalies.spec.tsx b/static/app/views/detectors/hooks/useMetricDetectorAnomalies.spec.tsx index f743d05ff379bb..d26d81eaaab0ca 100644 --- a/static/app/views/detectors/hooks/useMetricDetectorAnomalies.spec.tsx +++ b/static/app/views/detectors/hooks/useMetricDetectorAnomalies.spec.tsx @@ -75,8 +75,9 @@ describe('useMetricDetectorAnomalies', () => { historicalSeries, thresholdType: AlertRuleThresholdType.ABOVE, sensitivity: AlertRuleSensitivity.MEDIUM, - timePeriod: 900, // 15 minutes + interval: 900, // 15 minutes projectId: '1', + enabled: true, }), {wrapper: TestContext} ); diff --git a/static/app/views/detectors/hooks/useMetricDetectorAnomalies.tsx b/static/app/views/detectors/hooks/useMetricDetectorAnomalies.tsx index e121f97547ed23..ee517af4880c22 100644 --- a/static/app/views/detectors/hooks/useMetricDetectorAnomalies.tsx +++ b/static/app/views/detectors/hooks/useMetricDetectorAnomalies.tsx @@ -33,13 +33,16 @@ interface EventAnomalyPayload { } interface UseMetricDetectorAnomaliesProps { + enabled: boolean; historicalSeries: Series[]; + /** + * The metric detector interval in seconds + */ + interval: number; projectId: string; sensitivity: AlertRuleSensitivity | undefined; series: Series[]; thresholdType: AlertRuleThresholdType | undefined; - timePeriod: number; - enabled?: boolean; } function transformSeriesToDataPoints(series: Series[]): Array<[number, {count: number}]> { @@ -62,9 +65,9 @@ export function useMetricDetectorAnomalies({ historicalSeries, thresholdType, sensitivity, - timePeriod, + interval, projectId, - enabled = true, + enabled, }: UseMetricDetectorAnomaliesProps) { const organization = useOrganization(); @@ -89,7 +92,7 @@ export function useMetricDetectorAnomalies({ : 'both', expected_seasonality: 'auto', sensitivity: sensitivity || 'medium', - time_period: timePeriod / 60, + time_period: interval / 60, }, current_data: currentData, historical_data: filteredHistoricalData, diff --git a/static/app/views/detectors/hooks/useMetricDetectorAnomalyPeriods.spec.tsx b/static/app/views/detectors/hooks/useMetricDetectorAnomalyPeriods.spec.tsx index e1a340203f0485..fe6aedea310c2b 100644 --- a/static/app/views/detectors/hooks/useMetricDetectorAnomalyPeriods.spec.tsx +++ b/static/app/views/detectors/hooks/useMetricDetectorAnomalyPeriods.spec.tsx @@ -96,9 +96,10 @@ describe('useMetricDetectorAnomalyPeriods', () => { environment: undefined, projectId: '1', statsPeriod: TimePeriod.SEVEN_DAYS, - timePeriod: 900, // 15 minutes + interval: 900, // 15 minutes thresholdType: AlertRuleThresholdType.ABOVE, sensitivity: AlertRuleSensitivity.MEDIUM, + isLoadingSeries: false, enabled: true, }), {wrapper: TestContext} diff --git a/static/app/views/detectors/hooks/useMetricDetectorAnomalyPeriods.tsx b/static/app/views/detectors/hooks/useMetricDetectorAnomalyPeriods.tsx index f86c3d1467e4c2..67ef24ad638e19 100644 --- a/static/app/views/detectors/hooks/useMetricDetectorAnomalyPeriods.tsx +++ b/static/app/views/detectors/hooks/useMetricDetectorAnomalyPeriods.tsx @@ -12,10 +12,12 @@ import { import type {Anomaly} from 'sentry/views/alerts/types'; import {AnomalyType} from 'sentry/views/alerts/types'; import { + EAP_HISTORICAL_TIME_PERIOD_MAP, HISTORICAL_TIME_PERIOD_MAP, HISTORICAL_TIME_PERIOD_MAP_FIVE_MINS, } from 'sentry/views/alerts/utils/timePeriods'; import {DetectorDataset} from 'sentry/views/detectors/datasetConfig/types'; +import {isEapDataset} from 'sentry/views/detectors/datasetConfig/utils/isEapDataset'; import {useMetricDetectorSeries} from 'sentry/views/detectors/hooks/useMetricDetectorSeries'; import type {IncidentPeriod} from './useIncidentMarkers'; @@ -26,13 +28,17 @@ interface UseMetricDetectorAnomalyPeriodsProps { dataset: DetectorDataset; enabled: boolean; environment: string | undefined; + interval: number; + /** + * Should not fetch anomalies if series is loading + */ + isLoadingSeries: boolean; projectId: string; query: string; sensitivity: AlertRuleSensitivity | undefined; series: Series[]; statsPeriod: TimePeriod; thresholdType: AlertRuleThresholdType | undefined; - timePeriod: number; } interface UseMetricDetectorAnomalyPeriodsResult { @@ -116,13 +122,14 @@ function groupAnomaliesForBubbles( */ export function useMetricDetectorAnomalyPeriods({ series, + isLoadingSeries, dataset, aggregate, query, environment, projectId, statsPeriod, - timePeriod, + interval, thresholdType, sensitivity, enabled, @@ -130,53 +137,78 @@ export function useMetricDetectorAnomalyPeriods({ const theme = useTheme(); // Fetch historical data with extended time period for anomaly detection baseline comparison - const isFiveMinuteTimePeriod = timePeriod === 300; - const historicalPeriod = isFiveMinuteTimePeriod - ? HISTORICAL_TIME_PERIOD_MAP_FIVE_MINS[ - statsPeriod as keyof typeof HISTORICAL_TIME_PERIOD_MAP_FIVE_MINS + const isFiveMinuteInterval = interval === 300; + // EAP datasets have to select fewer historical data points + const historicalPeriod = isEapDataset(dataset) + ? EAP_HISTORICAL_TIME_PERIOD_MAP[ + statsPeriod as keyof typeof EAP_HISTORICAL_TIME_PERIOD_MAP ] - : HISTORICAL_TIME_PERIOD_MAP[statsPeriod as keyof typeof HISTORICAL_TIME_PERIOD_MAP]; - - const {series: historicalSeries, isLoading: isHistoricalLoading} = - useMetricDetectorSeries({ - dataset, - aggregate, - interval: timePeriod, - query, - environment, - projectId, - statsPeriod: historicalPeriod as TimePeriod, - options: { - enabled, - }, - }); + : // 5-minute intervals also require fewer historical data points + isFiveMinuteInterval + ? HISTORICAL_TIME_PERIOD_MAP_FIVE_MINS[ + statsPeriod as keyof typeof HISTORICAL_TIME_PERIOD_MAP_FIVE_MINS + ] + : HISTORICAL_TIME_PERIOD_MAP[ + statsPeriod as keyof typeof HISTORICAL_TIME_PERIOD_MAP + ]; + + const { + series: historicalSeries, + isLoading: isHistoricalLoading, + error: historicalError, + } = useMetricDetectorSeries({ + dataset, + aggregate, + interval, + query, + environment, + projectId, + statsPeriod: historicalPeriod as TimePeriod, + options: { + enabled, + }, + }); const { data: anomalies, - isLoading, - error, + isLoading: isAnomalyLoading, + error: anomalyError, } = useMetricDetectorAnomalies({ series, historicalSeries, projectId, thresholdType, sensitivity, - timePeriod, - enabled, + interval, + // Wait until both regular series and historical series are loaded + enabled: enabled && !isLoadingSeries && !isHistoricalLoading, }); const anomalyPeriods = useMemo(() => { - if (!anomalies || anomalies.length === 0 || isHistoricalLoading || isLoading) { + if ( + !anomalies || + anomalies.length === 0 || + isHistoricalLoading || + isAnomalyLoading || + isLoadingSeries + ) { return []; } // Convert timePeriod from seconds to milliseconds for minimum anomaly width - const timePeriodMs = timePeriod ? timePeriod * 1000 : undefined; + const timePeriodMs = interval ? interval * 1000 : undefined; return groupAnomaliesForBubbles(anomalies, theme, timePeriodMs); - }, [anomalies, theme, timePeriod, isHistoricalLoading, isLoading]); + }, [ + anomalies, + theme, + interval, + isHistoricalLoading, + isAnomalyLoading, + isLoadingSeries, + ]); return { anomalyPeriods, - isLoading: isHistoricalLoading || isLoading, - error, + isLoading: isHistoricalLoading || isAnomalyLoading, + error: historicalError || anomalyError, }; } diff --git a/static/app/views/detectors/hooks/useMetricDetectorSeries.tsx b/static/app/views/detectors/hooks/useMetricDetectorSeries.tsx index d27dbdd589a789..38a23c685672d4 100644 --- a/static/app/views/detectors/hooks/useMetricDetectorSeries.tsx +++ b/static/app/views/detectors/hooks/useMetricDetectorSeries.tsx @@ -2,6 +2,7 @@ import {useMemo} from 'react'; import type {Series} from 'sentry/types/echarts'; import {useApiQuery, type UseApiQueryOptions} from 'sentry/utils/queryClient'; +import type RequestError from 'sentry/utils/requestError/requestError'; import useOrganization from 'sentry/utils/useOrganization'; import {TimePeriod} from 'sentry/views/alerts/rules/metric/types'; import {getDatasetConfig} from 'sentry/views/detectors/datasetConfig/getDatasetConfig'; @@ -22,7 +23,7 @@ interface UseMetricDetectorSeriesProps { interface UseMetricDetectorSeriesResult { comparisonSeries: Series[]; - isError: boolean; + error: RequestError | null; isLoading: boolean; series: Series[]; } @@ -55,7 +56,7 @@ export function useMetricDetectorSeries({ comparisonDelta, }); - const {data, isLoading, isError} = useApiQuery< + const {data, isLoading, error} = useApiQuery< Parameters[0] >(seriesQueryOptions, { // 5 minutes @@ -82,5 +83,5 @@ export function useMetricDetectorSeries({ }; }, [datasetConfig, data, aggregate, comparisonDelta]); - return {series, comparisonSeries, isLoading, isError}; + return {series, comparisonSeries, isLoading, error}; }