diff --git a/static/app/utils/timeSeries/isTimeSeriesOther.spec.tsx b/static/app/utils/timeSeries/isTimeSeriesOther.spec.tsx deleted file mode 100644 index 0c6900301c8f82..00000000000000 --- a/static/app/utils/timeSeries/isTimeSeriesOther.spec.tsx +++ /dev/null @@ -1,29 +0,0 @@ -import {TimeSeriesFixture} from 'sentry-fixture/timeSeries'; - -import {isTimeSeriesOther} from './isTimeSeriesOther'; - -describe('isTimeSeriesOther', () => { - it('treats normal time series as not other', () => { - expect(isTimeSeriesOther(TimeSeriesFixture())).toBeFalsy(); - }); - - it('treats the title "Other" as other', () => { - expect( - isTimeSeriesOther( - TimeSeriesFixture({ - yAxis: 'Other', - }) - ) - ).toBeTruthy(); - }); - - it('treats top N "Other" as other', () => { - expect( - isTimeSeriesOther( - TimeSeriesFixture({ - yAxis: 'eps() : Other', - }) - ) - ).toBeTruthy(); - }); -}); diff --git a/static/app/utils/timeSeries/isTimeSeriesOther.tsx b/static/app/utils/timeSeries/isTimeSeriesOther.tsx deleted file mode 100644 index 99fe099c9a7963..00000000000000 --- a/static/app/utils/timeSeries/isTimeSeriesOther.tsx +++ /dev/null @@ -1,8 +0,0 @@ -import type {TimeSeries} from 'sentry/views/dashboards/widgets/common/types'; - -const OTHER = 'Other'; -const otherRegex = new RegExp(`(?:.* : ${OTHER}$)|^${OTHER}$`); - -export function isTimeSeriesOther(timeSeries: TimeSeries): boolean { - return Boolean(timeSeries.yAxis.match(otherRegex)); -} diff --git a/static/app/utils/timeSeries/parseGroupBy.spec.tsx b/static/app/utils/timeSeries/parseGroupBy.spec.tsx new file mode 100644 index 00000000000000..f75a6e091b90f0 --- /dev/null +++ b/static/app/utils/timeSeries/parseGroupBy.spec.tsx @@ -0,0 +1,52 @@ +import {parseGroupBy} from './parseGroupBy'; + +describe('parseGroupBy', () => { + it('returns undefined for "Other" group name', () => { + const result = parseGroupBy('Other', ['field1', 'field2']); + expect(result).toBeUndefined(); + }); + + it('parses single field and value correctly', () => { + const result = parseGroupBy('value1', ['field1']); + expect(result).toEqual([{key: 'field1', value: 'value1'}]); + }); + + it('parses multiple fields and values correctly', () => { + const result = parseGroupBy('value1,value2,value3', ['field1', 'field2', 'field3']); + expect(result).toEqual([ + {key: 'field1', value: 'value1'}, + {key: 'field2', value: 'value2'}, + {key: 'field3', value: 'value3'}, + ]); + }); + + it('handles more values than fields by using empty strings for extra values', () => { + const result = parseGroupBy('value1,value2', ['field1']); + expect(result).toEqual([ + {key: 'field1', value: 'value1'}, + {key: '', value: 'value2'}, + ]); + }); + + it('handles more fields than values by using empty strings for missing values', () => { + const result = parseGroupBy('value1', ['field1', 'field2']); + expect(result).toEqual([ + {key: 'field1', value: 'value1'}, + {key: 'field2', value: ''}, + ]); + }); + + it('handles empty strings in values', () => { + const result = parseGroupBy('value1,,value3', ['field1', 'field2', 'field3']); + expect(result).toEqual([ + {key: 'field1', value: 'value1'}, + {key: 'field2', value: ''}, + {key: 'field3', value: 'value3'}, + ]); + }); + + it('handles empty fields array', () => { + const result = parseGroupBy('', []); + expect(result).toBeUndefined(); + }); +}); diff --git a/static/app/utils/timeSeries/parseGroupBy.tsx b/static/app/utils/timeSeries/parseGroupBy.tsx new file mode 100644 index 00000000000000..40c7ddd89a42bb --- /dev/null +++ b/static/app/utils/timeSeries/parseGroupBy.tsx @@ -0,0 +1,26 @@ +import zipWith from 'lodash/zipWith'; + +import type {TimeSeriesGroupBy} from 'sentry/views/dashboards/widgets/common/types'; + +export function parseGroupBy( + groupName: string, + fields: string[] +): TimeSeriesGroupBy[] | undefined { + if (groupName === 'Other') { + return undefined; + } + + const groupKeys = fields; + const groupValues = groupName.split(','); + + const groupBys = zipWith(groupKeys, groupValues, (key, value) => { + return { + key: key ?? '', + value: value ?? '', + }; + }).filter(groupBy => { + return groupBy.key || groupBy.value; + }); + + return groupBys.length > 0 ? groupBys : undefined; +} diff --git a/static/app/views/dashboards/widgets/common/types.tsx b/static/app/views/dashboards/widgets/common/types.tsx index d4efbce2497b93..18dc7eaf15d6f0 100644 --- a/static/app/views/dashboards/widgets/common/types.tsx +++ b/static/app/views/dashboards/widgets/common/types.tsx @@ -65,7 +65,7 @@ export type TimeSeriesItem = { */ type IncompleteReason = 'INCOMPLETE_BUCKET'; -type TimeSeriesGroupBy = { +export type TimeSeriesGroupBy = { key: string; /** * The `value` of a `groupBy` can sometimes surprisingly be an array, because some datasets support array values. e.g., in the error dataset, the error type could be an array that looks like `["Exception", null, "TypeError"]` diff --git a/static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesName.spec.tsx b/static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesName.spec.tsx index d7343c1612b689..f42ed3aa01b38f 100644 --- a/static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesName.spec.tsx +++ b/static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesName.spec.tsx @@ -148,4 +148,16 @@ describe('formatSeriesName', () => { expect(formatTimeSeriesName(timeSeries)).toEqual(result); }); }); + + describe('other', () => { + it('Formats "Other"', () => { + const timeSeries = TimeSeriesFixture(); + timeSeries.meta = { + ...timeSeries.meta, + isOther: true, + }; + + expect(formatTimeSeriesName(timeSeries)).toBe('Other'); + }); + }); }); diff --git a/static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesName.tsx b/static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesName.tsx index 5b10a66a668e01..dd410ec9617456 100644 --- a/static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesName.tsx +++ b/static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesName.tsx @@ -1,3 +1,4 @@ +import {t} from 'sentry/locale'; import { getAggregateArg, getMeasurementSlug, @@ -12,6 +13,10 @@ export function formatTimeSeriesName(timeSeries: TimeSeries): string { // If the timeSeries has `groupBy` information, the label is made by // concatenating the values of the groupBy, since there's no point repeating // the name of the Y axis multiple times in the legend. + if (timeSeries.meta.isOther) { + return t('Other'); + } + if (timeSeries.groupBy?.length && timeSeries.groupBy.length > 0) { return `${timeSeries.groupBy ?.map(groupBy => { diff --git a/static/app/views/explore/components/chart/chartVisualization.tsx b/static/app/views/explore/components/chart/chartVisualization.tsx index 65ab10f8bd0c6f..a37345b7e0871f 100644 --- a/static/app/views/explore/components/chart/chartVisualization.tsx +++ b/static/app/views/explore/components/chart/chartVisualization.tsx @@ -6,7 +6,6 @@ import styled from '@emotion/styled'; import TransparentLoadingMask from 'sentry/components/charts/transparentLoadingMask'; import {t} from 'sentry/locale'; import type {ReactEchartsRef} from 'sentry/types/echarts'; -import {isTimeSeriesOther} from 'sentry/utils/timeSeries/isTimeSeriesOther'; import {markDelayedData} from 'sentry/utils/timeSeries/markDelayedData'; import usePrevious from 'sentry/utils/usePrevious'; import {Area} from 'sentry/views/dashboards/widgets/timeSeriesWidget/plottables/area'; @@ -17,7 +16,6 @@ import {TimeSeriesWidgetVisualization} from 'sentry/views/dashboards/widgets/tim import {Widget} from 'sentry/views/dashboards/widgets/widget/widget'; import type {ChartInfo} from 'sentry/views/explore/components/chart/types'; import {SAMPLING_MODE} from 'sentry/views/explore/hooks/useProgressiveQuery'; -import {prettifyAggregation} from 'sentry/views/explore/utils'; import {ChartType} from 'sentry/views/insights/common/components/chart'; import {INGESTION_DELAY} from 'sentry/views/insights/settings'; @@ -38,8 +36,6 @@ export function ChartVisualization({ const theme = useTheme(); const plottables = useMemo(() => { - const formattedYAxis = prettifyAggregation(chartInfo.yAxis) ?? chartInfo.yAxis; - const DataPlottableConstructor = chartInfo.chartType === ChartType.LINE ? Line @@ -55,13 +51,12 @@ export function ChartVisualization({ // values instead of the aggregate function. if (s.yAxis === chartInfo.yAxis) { return new DataPlottableConstructor(markDelayedData(s, INGESTION_DELAY), { - alias: formattedYAxis ?? chartInfo.yAxis, - color: isTimeSeriesOther(s) ? theme.chartOther : undefined, + color: s.meta.isOther ? theme.chartOther : undefined, stack: 'all', }); } return new DataPlottableConstructor(markDelayedData(s, INGESTION_DELAY), { - color: isTimeSeriesOther(s) ? theme.chartOther : undefined, + color: s.meta.isOther ? theme.chartOther : undefined, stack: 'all', }); }); diff --git a/static/app/views/insights/common/queries/useSortedTimeSeries.tsx b/static/app/views/insights/common/queries/useSortedTimeSeries.tsx index 98b3b568e39c2c..48d0a4a9285d66 100644 --- a/static/app/views/insights/common/queries/useSortedTimeSeries.tsx +++ b/static/app/views/insights/common/queries/useSortedTimeSeries.tsx @@ -15,12 +15,14 @@ import { import {DiscoverDatasets} from 'sentry/utils/discover/types'; import {intervalToMilliseconds} from 'sentry/utils/duration/intervalToMilliseconds'; import {getTimeSeriesInterval} from 'sentry/utils/timeSeries/getTimeSeriesInterval'; +import {parseGroupBy} from 'sentry/utils/timeSeries/parseGroupBy'; import type {MutableSearch} from 'sentry/utils/tokenizeSearch'; import {useLocation} from 'sentry/utils/useLocation'; import useOrganization from 'sentry/utils/useOrganization'; import usePageFilters from 'sentry/utils/usePageFilters'; import { isEventsStats, + isGroupedMultiSeriesEventsStats, isMultiSeriesEventsStats, } from 'sentry/views/dashboards/utils/isEventsStats'; import type { @@ -139,8 +141,8 @@ export const useSortedTimeSeries = < const isFetchingOrLoading = result.isPending || result.isFetching; const data: SeriesMap = useMemo(() => { - return isFetchingOrLoading ? {} : transformToSeriesMap(result.data, yAxis); - }, [isFetchingOrLoading, result.data, yAxis]); + return isFetchingOrLoading ? {} : transformToSeriesMap(result.data, yAxis, fields); + }, [isFetchingOrLoading, result.data, yAxis, fields]); const pageLinks = result.response?.getResponseHeader('Link') ?? undefined; @@ -154,97 +156,102 @@ export const useSortedTimeSeries = < export function transformToSeriesMap( result: MultiSeriesEventsStats | GroupedMultiSeriesEventsStats | undefined, - yAxis: string[] + yAxis: string[], + fields?: string[] ): SeriesMap { if (!result) { return {}; } - // Single series, applies to single axis queries + const allTimeSeries: TimeSeries[] = []; + + // Single series, applies to single axis queries. The yAxis is only knowable from the input data. There is no group const firstYAxis = yAxis[0] || ''; if (isEventsStats(result)) { - const [, series] = convertEventsStatsToTimeSeriesData(firstYAxis, result); - return { - [firstYAxis]: [series], - }; + const [, timeSeries] = convertEventsStatsToTimeSeriesData(firstYAxis, result); + allTimeSeries.push(timeSeries); } // Multiple series, applies to multi axis or topN events queries const hasMultipleYAxes = yAxis.length > 1; if (isMultiSeriesEventsStats(result)) { - const processedResults: Array<[number, TimeSeries]> = Object.keys(result).map( - seriesOrGroupName => { - // If this is a single-axis top N result, the keys in the response are - // group names. The field name is the first (and only) Y axis. If it's a - // multi-axis non-top-N result, the keys are the axis names. Figure out - // the field name and the group name (if different) and format accordingly - return convertEventsStatsToTimeSeriesData( - hasMultipleYAxes ? seriesOrGroupName : yAxis[0]!, - result[seriesOrGroupName]!, - hasMultipleYAxes ? undefined : seriesOrGroupName, - hasMultipleYAxes ? undefined : result[seriesOrGroupName]!.order + if (hasMultipleYAxes) { + // This is a multi-axis query. The keys in the response are the yAxis + // names, we can iterate the values. There is not grouping + yAxis.forEach(axis => { + const seriesData = result[axis]; // This is technically never `undefined` but better safe than sorry + if (seriesData) { + const [, timeSeries] = convertEventsStatsToTimeSeriesData(axis, seriesData); + allTimeSeries.push(timeSeries); + } + }); + } else { + // This is a top events query. The keys in the object will be the group names, and there is only one yAxis, known from the input + Object.keys(result).forEach(groupName => { + const seriesData = result[groupName]!; + const [, timeSeries] = convertEventsStatsToTimeSeriesData( + firstYAxis, + seriesData, + seriesData.order ); - } - ); - if (!hasMultipleYAxes) { - return { - [firstYAxis]: processedResults - .sort(([a], [b]) => a - b) - .map(([, series]) => series), - }; - } + if (fields) { + const groupByFields = fields.filter(field => !yAxis.includes(field)); + const groupBy = parseGroupBy(groupName, groupByFields); + timeSeries.groupBy = groupBy; + if (groupName === 'Other') { + timeSeries.meta.isOther = true; + } + } - return processedResults - .sort(([a], [b]) => a - b) - .reduce((acc, [, series]) => { - acc[series.yAxis] = [series]; - return acc; - }, {} as SeriesMap); + allTimeSeries.push(timeSeries); + }); + } } - // Grouped multi series, applies to topN events queries with multiple y-axes - // First, we process the grouped multi series into a list of [seriesName, order, {[aggFunctionAlias]: EventsStats}] - // to enable sorting. - const processedResults: Array<[string, number, MultiSeriesEventsStats]> = []; - Object.keys(result).forEach(groupName => { - const {order: groupOrder, ...groupData} = result[groupName]!; - processedResults.push([ - groupName, - groupOrder || 0, - groupData as MultiSeriesEventsStats, - ]); - }); + // Multiple series, _and_ grouped. The top level keys are groups, the lower-level are the axes + if (isGroupedMultiSeriesEventsStats(result)) { + Object.keys(result).forEach(groupName => { + const groupData = result[groupName] as MultiSeriesEventsStats; + + Object.keys(groupData).forEach(axis => { + if (axis === 'order') { + // `order` is a special key on grouped responses, we can skip over it + return; + } - return processedResults - .sort(([, orderA], [, orderB]) => orderA - orderB) - .reduce((acc, [groupName, groupOrder, groupData]) => { - Object.keys(groupData).forEach(seriesName => { - const [, series] = convertEventsStatsToTimeSeriesData( - seriesName, - groupData[seriesName]!, - groupName, - groupOrder + const seriesData = groupData[axis] as EventsStats; + const [, timeSeries] = convertEventsStatsToTimeSeriesData( + axis, + seriesData, + seriesData.order ); - if (acc[seriesName]) { - acc[seriesName].push(series); - } else { - acc[seriesName] = [series]; + if (fields) { + const groupByFields = fields.filter(field => !yAxis.includes(field)); + const groupBy = parseGroupBy(groupName, groupByFields); + timeSeries.groupBy = groupBy; + if (groupName === 'Other') { + timeSeries.meta.isOther = true; + } } + + allTimeSeries.push(timeSeries); }); - return acc; - }, {} as SeriesMap); + }); + } + + return Object.groupBy( + allTimeSeries.toSorted((a, b) => (a.meta.order ?? 0) - (b.meta.order ?? 0)), + ts => ts.yAxis + ) as SeriesMap; } export function convertEventsStatsToTimeSeriesData( - seriesName: string, + yAxis: string, seriesData: EventsStats, - alias?: string, order?: number ): [number, TimeSeries] { - const label = alias ?? (seriesName || FALLBACK_SERIES_NAME); - const values: TimeSeriesItem[] = seriesData.data.map( ([timestamp, countsForTimestamp], index) => { const item: TimeSeriesItem = { @@ -273,11 +280,11 @@ export function convertEventsStatsToTimeSeriesData( const interval = getTimeSeriesInterval(values); const serie: TimeSeries = { - yAxis: label, + yAxis: yAxis ?? FALLBACK_SERIES_NAME, values, meta: { - valueType: seriesData.meta?.fields?.[seriesName]!, - valueUnit: seriesData.meta?.units?.[seriesName] as DataUnit, + valueType: seriesData.meta?.fields?.[yAxis]!, + valueUnit: seriesData.meta?.units?.[yAxis] as DataUnit, interval, dataScanned: seriesData.meta?.dataScanned, }, @@ -287,5 +294,5 @@ export function convertEventsStatsToTimeSeriesData( serie.meta.order = order; } - return [seriesData.order ?? 0, serie]; + return [serie.meta.order ?? 0, serie]; }