From 0510a2a98eac491edb480dc6b2676ee71d59b8fd Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Tue, 25 Nov 2025 16:10:11 -0800 Subject: [PATCH] feat(aci): Format percentage based thresholds, data - fixes an issue where release dataset wasn't formatted correctly on aci details - fixes thresholds on % based charts being (10 being rendered as 10k percent instead of 0.1 --- .../components/details/metric/chart.tsx | 58 ++++++++----------- .../utils/useDetectorChartAxisBounds.tsx | 57 ++++++++++++++++++ .../forms/metric/metricDetectorChart.tsx | 43 +++++--------- .../useMetricDetectorThresholdSeries.tsx | 36 +++++++++--- 4 files changed, 123 insertions(+), 71 deletions(-) create mode 100644 static/app/views/detectors/components/details/metric/utils/useDetectorChartAxisBounds.tsx diff --git a/static/app/views/detectors/components/details/metric/chart.tsx b/static/app/views/detectors/components/details/metric/chart.tsx index 580de6a05fb9b4..590757443868aa 100644 --- a/static/app/views/detectors/components/details/metric/chart.tsx +++ b/static/app/views/detectors/components/details/metric/chart.tsx @@ -23,6 +23,7 @@ import { buildDetectorZoomQuery, computeZoomRangeMs, } from 'sentry/views/detectors/components/details/common/buildDetectorZoomQuery'; +import {useDetectorChartAxisBounds} from 'sentry/views/detectors/components/details/metric/utils/useDetectorChartAxisBounds'; import {getDatasetConfig} from 'sentry/views/detectors/datasetConfig/getDatasetConfig'; import {getDetectorDataset} from 'sentry/views/detectors/datasetConfig/getDetectorDataset'; import { @@ -161,11 +162,12 @@ export function useMetricDetectorChart({ const snubaQuery = detector.dataSources[0].queryObj.snubaQuery; const dataset = getDetectorDataset(snubaQuery.dataset, snubaQuery.eventTypes); const datasetConfig = getDatasetConfig(dataset); + const aggregate = datasetConfig.fromApiAggregate(snubaQuery.aggregate); const {series, comparisonSeries, isLoading, error} = useMetricDetectorSeries({ detectorDataset: dataset, dataset: snubaQuery.dataset, extrapolationMode: snubaQuery.extrapolationMode, - aggregate: datasetConfig.fromApiAggregate(snubaQuery.aggregate), + aggregate, interval: snubaQuery.timeWindow, query: snubaQuery.query, environment: snubaQuery.environment, @@ -181,6 +183,7 @@ export function useMetricDetectorChart({ useMetricDetectorThresholdSeries({ conditions: detector.conditionGroup?.conditions, detectionType, + aggregate, comparisonSeries, }); @@ -221,30 +224,7 @@ export function useMetricDetectorChart({ usePageDate: true, }); - // Calculate y-axis bounds to ensure all thresholds are visible - const maxValue = useMemo(() => { - // Get max from series data - let seriesMax = 0; - if (series.length > 0) { - const allSeriesValues = series.flatMap(s => - s.data - .map(point => point.value) - .filter(val => typeof val === 'number' && !isNaN(val)) - ); - seriesMax = allSeriesValues.length > 0 ? Math.max(...allSeriesValues) : 0; - } - - // Combine with threshold max and round to nearest whole number - const combinedMax = thresholdMaxValue - ? Math.max(seriesMax, thresholdMaxValue) - : seriesMax; - - const roundedMax = Math.round(combinedMax); - - // Add padding to the bounds - const padding = roundedMax * 0.1; - return roundedMax + padding; - }, [series, thresholdMaxValue]); + const {maxValue, minValue} = useDetectorChartAxisBounds({series, thresholdMaxValue}); const additionalSeries = useMemo(() => { const baseSeries = [...thresholdAdditionalSeries]; @@ -256,18 +236,25 @@ export function useMetricDetectorChart({ }, [thresholdAdditionalSeries, openPeriodMarkerResult.incidentMarkerSeries]); const yAxes = useMemo(() => { - const {formatYAxisLabel} = getDetectorChartFormatters({ + const {formatYAxisLabel, outputType} = getDetectorChartFormatters({ detectionType, - aggregate: snubaQuery.aggregate, + aggregate, }); + const isPercentage = outputType === 'percentage'; + // For percentage aggregates, use fixed max of 1 (100%) and calculated min + const yAxisMax = isPercentage ? 1 : maxValue > 0 ? maxValue : undefined; + // Start charts at 0 for non-percentage aggregates + const yAxisMin = isPercentage ? minValue : 0; + const mainYAxis: YAXisComponentOption = { - max: maxValue > 0 ? maxValue : undefined, - min: 0, + max: yAxisMax, + min: yAxisMin, axisLabel: { - // Hide the maximum y-axis label to avoid showing arbitrary threshold values - showMaxLabel: false, - formatter: (value: number) => formatYAxisLabel(value), + // Show max label for percentage (100%) but hide for other types to avoid arbitrary values + showMaxLabel: isPercentage, + // Format the axis labels with units + formatter: formatYAxisLabel, }, // Disable the y-axis grid lines splitLine: {show: false}, @@ -282,8 +269,9 @@ export function useMetricDetectorChart({ return axes; }, [ detectionType, - snubaQuery.aggregate, + aggregate, maxValue, + minValue, openPeriodMarkerResult.incidentMarkerYAxis, ]); @@ -314,7 +302,7 @@ export function useMetricDetectorChart({ tooltip: { valueFormatter: getDetectorChartFormatters({ detectionType, - aggregate: snubaQuery.aggregate, + aggregate, }).formatTooltipValue, }, ...chartZoomProps, @@ -325,6 +313,7 @@ export function useMetricDetectorChart({ }; }, [ additionalSeries, + aggregate, chartZoomProps, detectionType, error, @@ -333,7 +322,6 @@ export function useMetricDetectorChart({ isLoading, openPeriodMarkerResult, series, - snubaQuery.aggregate, yAxes, ]); diff --git a/static/app/views/detectors/components/details/metric/utils/useDetectorChartAxisBounds.tsx b/static/app/views/detectors/components/details/metric/utils/useDetectorChartAxisBounds.tsx new file mode 100644 index 00000000000000..c400671c59bb12 --- /dev/null +++ b/static/app/views/detectors/components/details/metric/utils/useDetectorChartAxisBounds.tsx @@ -0,0 +1,57 @@ +import {useMemo} from 'react'; + +import type {Series} from 'sentry/types/echarts'; + +interface UseChartAxisBoundsProps { + series: Series[]; + thresholdMaxValue: number | undefined; +} + +interface ChartAxisBounds { + maxValue: number; + minValue: number; +} + +/** + * Calculates y-axis bounds for detector charts based on series data and threshold values. + * Adds padding to ensure all data points and thresholds are visible. + */ +export function useDetectorChartAxisBounds({ + series, + thresholdMaxValue, +}: UseChartAxisBoundsProps): ChartAxisBounds { + return useMemo(() => { + if (series.length === 0) { + return {maxValue: 0, minValue: 0}; + } + + const allSeriesValues = series.flatMap(s => + s.data + .map(point => point.value) + .filter(val => typeof val === 'number' && !isNaN(val)) + ); + + if (allSeriesValues.length === 0) { + return {maxValue: 0, minValue: 0}; + } + + const seriesMax = Math.max(...allSeriesValues); + const seriesMin = Math.min(...allSeriesValues); + + // Combine with threshold max and round to nearest whole number + const combinedMax = thresholdMaxValue + ? Math.max(seriesMax, thresholdMaxValue) + : seriesMax; + + const roundedMax = Math.round(combinedMax); + + // Add padding to the bounds + const maxPadding = roundedMax * 0.1; + const minPadding = seriesMin * 0.1; + + return { + maxValue: roundedMax + maxPadding, + minValue: Math.max(0, seriesMin - minPadding), + }; + }, [series, thresholdMaxValue]); +} diff --git a/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx b/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx index a06ce86fce1eb3..790e0421bdcb07 100644 --- a/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx +++ b/static/app/views/detectors/components/forms/metric/metricDetectorChart.tsx @@ -25,6 +25,7 @@ import { EventTypes, ExtrapolationMode, } from 'sentry/views/alerts/rules/metric/types'; +import {useDetectorChartAxisBounds} from 'sentry/views/detectors/components/details/metric/utils/useDetectorChartAxisBounds'; 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'; @@ -177,6 +178,7 @@ export function MetricDetectorChart({ useMetricDetectorThresholdSeries({ conditions, detectionType, + aggregate, comparisonSeries, }); @@ -216,30 +218,7 @@ export function MetricDetectorChart({ markLineTooltip: anomalyMarklineTooltip, }); - // Calculate y-axis bounds to ensure all thresholds are visible - const maxValue = useMemo(() => { - // Get max from series data - let seriesMax = 0; - if (series.length > 0) { - const allSeriesValues = series.flatMap(s => - s.data - .map(point => point.value) - .filter(val => typeof val === 'number' && !isNaN(val)) - ); - seriesMax = allSeriesValues.length > 0 ? Math.max(...allSeriesValues) : 0; - } - - // Combine with threshold max and round to nearest whole number - const combinedMax = thresholdMaxValue - ? Math.max(seriesMax, thresholdMaxValue) - : seriesMax; - - const roundedMax = Math.round(combinedMax); - - // Add padding to the bounds - const padding = roundedMax * 0.1; - return roundedMax + padding; - }, [series, thresholdMaxValue]); + const {maxValue, minValue} = useDetectorChartAxisBounds({series, thresholdMaxValue}); const additionalSeries = useMemo(() => { const baseSeries = [...thresholdAdditionalSeries]; @@ -257,17 +236,22 @@ export function MetricDetectorChart({ ]); const yAxes = useMemo(() => { - const {formatYAxisLabel} = getDetectorChartFormatters({ + const {formatYAxisLabel, outputType} = getDetectorChartFormatters({ detectionType, aggregate, }); + const isPercentage = outputType === 'percentage'; + // For percentage aggregates, use fixed max of 1 (100%) and calculated min + const yAxisMax = isPercentage ? 1 : maxValue > 0 ? maxValue : undefined; + const yAxisMin = isPercentage ? minValue : 0; + const mainYAxis: YAXisComponentOption = { - max: maxValue > 0 ? maxValue : undefined, - min: 0, + max: yAxisMax, + min: yAxisMin, axisLabel: { - // Hide the maximum y-axis label to avoid showing arbitrary threshold values - showMaxLabel: false, + // Show max label for percentage (100%) but hide for other types to avoid arbitrary values + showMaxLabel: isPercentage, // Format the axis labels with units formatter: formatYAxisLabel, }, @@ -285,6 +269,7 @@ export function MetricDetectorChart({ return axes; }, [ maxValue, + minValue, isAnomalyDetection, anomalyMarkerResult.incidentMarkerYAxis, detectionType, diff --git a/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx b/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx index 581c0a8f0ab2d8..10560d068dc051 100644 --- a/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx +++ b/static/app/views/detectors/hooks/useMetricDetectorThresholdSeries.tsx @@ -15,6 +15,7 @@ import type { MetricCondition, MetricDetectorConfig, } from 'sentry/types/workflowEngine/detectors'; +import {aggregateOutputType} from 'sentry/utils/discover/fields'; function createThresholdMarkLine(lineColor: string, threshold: number) { return MarkLine({ @@ -117,6 +118,10 @@ function extractThresholdsFromConditions( interface UseMetricDetectorThresholdSeriesProps { conditions: Array> | undefined; detectionType: MetricDetectorConfig['detectionType']; + /** + * The aggregate function to determine if thresholds should be scaled for percentage display + */ + aggregate?: string; comparisonSeries?: Series[]; } @@ -134,11 +139,16 @@ interface UseMetricDetectorThresholdSeriesResult { export function useMetricDetectorThresholdSeries({ conditions, detectionType, + aggregate, comparisonSeries = [], }: UseMetricDetectorThresholdSeriesProps): UseMetricDetectorThresholdSeriesResult { const theme = useTheme(); return useMemo((): UseMetricDetectorThresholdSeriesResult => { + // For percentage aggregates (e.g., crash-free rate), thresholds are input as whole numbers + // (e.g., 95 for 95%) but need to be displayed as decimals (0.95) on the chart + const isPercentageAggregate = + aggregate && aggregateOutputType(aggregate) === 'percentage'; if (!conditions) { return {maxValue: undefined, additionalSeries: []}; } @@ -216,11 +226,15 @@ export function useMetricDetectorThresholdSeries({ ? theme.red300 : theme.yellow300; const areaColor = lineColor; + // Scale threshold for percentage aggregates (e.g., 95 -> 0.95) + const displayThreshold = isPercentageAggregate + ? threshold.value / 100 + : threshold.value; return { type: 'line', - markLine: createThresholdMarkLine(lineColor, threshold.value), - markArea: createThresholdMarkArea(areaColor, threshold.value, isAbove), + markLine: createThresholdMarkLine(lineColor, displayThreshold), + markArea: createThresholdMarkArea(areaColor, displayThreshold, isAbove), data: [], }; }); @@ -231,12 +245,16 @@ export function useMetricDetectorThresholdSeries({ resolution && !thresholds.some(threshold => threshold.value === resolution.value) ); if (resolution && isResolutionManual) { + // Scale resolution threshold for percentage aggregates + const displayResolution = isPercentageAggregate + ? resolution.value / 100 + : resolution.value; const resolutionSeries: LineSeriesOption = { type: 'line', - markLine: createThresholdMarkLine(theme.green300, resolution.value), + markLine: createThresholdMarkLine(theme.green300, displayResolution), markArea: createThresholdMarkArea( theme.green300, - resolution.value, + displayResolution, resolution.type === DataConditionType.GREATER ), data: [], @@ -245,8 +263,12 @@ export function useMetricDetectorThresholdSeries({ } const valuesForMax = [ - ...thresholds.map(threshold => threshold.value), - ...(resolution && isResolutionManual ? [resolution.value] : []), + ...thresholds.map(threshold => + isPercentageAggregate ? threshold.value / 100 : threshold.value + ), + ...(resolution && isResolutionManual + ? [isPercentageAggregate ? resolution.value / 100 : resolution.value] + : []), ]; const maxValue = valuesForMax.length > 0 ? Math.max(...valuesForMax) : undefined; return {maxValue, additionalSeries: additional}; @@ -254,5 +276,5 @@ export function useMetricDetectorThresholdSeries({ // Other detection types not supported yet return {maxValue: undefined, additionalSeries: additional}; - }, [conditions, detectionType, comparisonSeries, theme]); + }, [conditions, detectionType, aggregate, comparisonSeries, theme]); }