-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(anomaly): add seer anomaly thresholds to metric monitor graph #104074
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,173 @@ | ||
| import {useMemo} from 'react'; | ||
| import {useTheme} from '@emotion/react'; | ||
| import type {LineSeriesOption} from 'echarts'; | ||
|
|
||
| import LineSeries from 'sentry/components/charts/series/lineSeries'; | ||
| import type {Series} from 'sentry/types/echarts'; | ||
| import {useApiQuery} from 'sentry/utils/queryClient'; | ||
| import type RequestError from 'sentry/utils/requestError/requestError'; | ||
| import useOrganization from 'sentry/utils/useOrganization'; | ||
|
|
||
| interface AnomalyThresholdDataPoint { | ||
| external_alert_id: number; | ||
| timestamp: number; | ||
| value: number; | ||
| yhat_lower: number; | ||
| yhat_upper: number; | ||
| } | ||
|
|
||
| interface AnomalyThresholdDataResponse { | ||
| data: AnomalyThresholdDataPoint[]; | ||
| } | ||
|
|
||
| interface UseMetricDetectorAnomalyThresholdsProps { | ||
| detectorId: string; | ||
| endTimestamp?: number; | ||
| series?: Series[]; | ||
| startTimestamp?: number; | ||
| } | ||
|
|
||
| interface UseMetricDetectorAnomalyThresholdsResult { | ||
| anomalyThresholdSeries: LineSeriesOption[]; | ||
| error: RequestError | null; | ||
| isLoading: boolean; | ||
| } | ||
|
|
||
| /** | ||
| * Fetches anomaly detection threshold data and transforms it into chart series | ||
| */ | ||
| export function useMetricDetectorAnomalyThresholds({ | ||
| detectorId, | ||
| startTimestamp, | ||
| endTimestamp, | ||
| series = [], | ||
| }: UseMetricDetectorAnomalyThresholdsProps): UseMetricDetectorAnomalyThresholdsResult { | ||
| const organization = useOrganization(); | ||
| const theme = useTheme(); | ||
|
|
||
| const hasAnomalyDataFlag = organization.features.includes( | ||
| 'anomaly-detection-threshold-data' | ||
| ); | ||
|
|
||
| const { | ||
| data: anomalyData, | ||
| isLoading, | ||
| error, | ||
| } = useApiQuery<AnomalyThresholdDataResponse>( | ||
| [ | ||
| `/organizations/${organization.slug}/detectors/${detectorId}/anomaly-data/`, | ||
| { | ||
| query: { | ||
| start: startTimestamp, | ||
| end: endTimestamp, | ||
| }, | ||
| }, | ||
| ], | ||
| { | ||
| staleTime: 0, | ||
| enabled: | ||
| hasAnomalyDataFlag && Boolean(detectorId && startTimestamp && endTimestamp), | ||
| } | ||
| ); | ||
|
|
||
| const anomalyThresholdSeries = useMemo(() => { | ||
| if (!anomalyData?.data || anomalyData.data.length === 0 || series.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| const data = anomalyData.data; | ||
| const metricData = series[0]?.data; | ||
|
|
||
| if (!metricData || metricData.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| const anomalyMap = new Map(data.map(point => [point.timestamp * 1000, point])); | ||
|
|
||
| const upperBoundData: Array<[number, number]> = []; | ||
| const lowerBoundData: Array<[number, number]> = []; | ||
| const seerValueData: Array<[number, number]> = []; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we including the seer value just for debugging with plans to remove it later?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep! there are some things that seem out of sync so im including it for now for debugging(this is why I changed the feature flag to be email only) |
||
|
|
||
| metricData.forEach(metricPoint => { | ||
| const timestamp = | ||
| typeof metricPoint.name === 'number' | ||
| ? metricPoint.name | ||
| : new Date(metricPoint.name).getTime(); | ||
| const anomalyPoint = anomalyMap.get(timestamp); | ||
|
|
||
| if (anomalyPoint) { | ||
| upperBoundData.push([timestamp, anomalyPoint.yhat_upper]); | ||
| lowerBoundData.push([timestamp, anomalyPoint.yhat_lower]); | ||
| seerValueData.push([timestamp, anomalyPoint.value]); | ||
| } | ||
| }); | ||
|
|
||
| const lineColor = theme.red300; | ||
| const seerValueColor = theme.yellow300; | ||
|
|
||
| return [ | ||
| LineSeries({ | ||
| name: 'Upper Threshold', | ||
| data: upperBoundData, | ||
| lineStyle: { | ||
| color: lineColor, | ||
| type: 'dashed', | ||
| width: 1, | ||
| dashOffset: 0, | ||
| }, | ||
| areaStyle: { | ||
| color: lineColor, | ||
| opacity: 0.05, | ||
| origin: 'end', | ||
| }, | ||
| itemStyle: {color: lineColor}, | ||
| animation: false, | ||
| animationThreshold: 1, | ||
| animationDuration: 0, | ||
| symbol: 'none', | ||
| connectNulls: true, | ||
| step: false, | ||
| }), | ||
| LineSeries({ | ||
| name: 'Lower Threshold', | ||
| data: lowerBoundData, | ||
| lineStyle: { | ||
| color: lineColor, | ||
| type: 'dashed', | ||
| width: 1, | ||
| dashOffset: 0, | ||
| }, | ||
| areaStyle: { | ||
| color: lineColor, | ||
| opacity: 0.05, | ||
| origin: 'start', | ||
| }, | ||
| itemStyle: {color: lineColor}, | ||
| animation: false, | ||
| animationThreshold: 1, | ||
| animationDuration: 0, | ||
| symbol: 'none', | ||
| connectNulls: true, | ||
| step: false, | ||
| }), | ||
| LineSeries({ | ||
| name: 'Seer Historical Value', | ||
| data: seerValueData, | ||
| lineStyle: { | ||
| color: seerValueColor, | ||
| type: 'solid', | ||
| width: 2, | ||
| }, | ||
| itemStyle: {color: seerValueColor}, | ||
| animation: false, | ||
| animationThreshold: 1, | ||
| animationDuration: 0, | ||
| symbol: 'circle', | ||
| symbolSize: 4, | ||
| connectNulls: true, | ||
| }), | ||
| ]; | ||
| }, [anomalyData, series, theme]); | ||
|
|
||
| return {anomalyThresholdSeries, isLoading, error}; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Y-axis bounds don't account for anomaly threshold values
The
maxValuecalculation only considersseriesandthresholdMaxValue, but doesn't include values from the newanomalyThresholdSeries(specificallyyhat_upper,yhat_lower, andvalue). The existinguseMetricDetectorThresholdSerieshook returns amaxValueto ensure thresholds are visible, butuseMetricDetectorAnomalyThresholdsonly returns the series without a max value. When the Y-axismaxis explicitly set at line 306, anomaly threshold lines that exceed this bound will be clipped and not visible to users.Additional Locations (1)
static/app/views/detectors/hooks/useMetricDetectorAnomalyThresholds.tsx#L29-L34