From e7a94ca8239453635d92443ab156f6363c03504d Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Mon, 29 Sep 2025 15:38:45 -0400 Subject: [PATCH 1/9] Improve chart rendering for "Other" series --- .../formatters/formatTimeSeriesName.spec.tsx | 12 ++++++++++++ .../formatters/formatTimeSeriesName.tsx | 5 +++++ 2 files changed, 17 insertions(+) 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 => { From 26a9d6ef9f072b75494202a1994f619562fddb0b Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Mon, 29 Sep 2025 15:39:59 -0400 Subject: [PATCH 2/9] Improve convertion to `TimeSeries` - correctly parse `groupBy`, `isOther`, and `yAxis` --- .../utils/timeSeries/parseGroupBy.spec.tsx | 46 ++++++ static/app/utils/timeSeries/parseGroupBy.tsx | 25 +++ .../views/dashboards/widgets/common/types.tsx | 2 +- .../components/chart/chartVisualization.tsx | 9 +- .../common/queries/useSortedTimeSeries.tsx | 151 +++++++++--------- 5 files changed, 149 insertions(+), 84 deletions(-) create mode 100644 static/app/utils/timeSeries/parseGroupBy.spec.tsx create mode 100644 static/app/utils/timeSeries/parseGroupBy.tsx diff --git a/static/app/utils/timeSeries/parseGroupBy.spec.tsx b/static/app/utils/timeSeries/parseGroupBy.spec.tsx new file mode 100644 index 00000000000000..3750b0eb620167 --- /dev/null +++ b/static/app/utils/timeSeries/parseGroupBy.spec.tsx @@ -0,0 +1,46 @@ +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('returns undefined when more values than fields', () => { + const result = parseGroupBy('value1,value2', ['field1']); + expect(result).toBeUndefined(); + }); + + it('returns undefined when more fields than values', () => { + const result = parseGroupBy('value1', ['field1', 'field2']); + expect(result).toBeUndefined(); + }); + + 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..5a92c05285c89f --- /dev/null +++ b/static/app/utils/timeSeries/parseGroupBy.tsx @@ -0,0 +1,25 @@ +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(','); + + // The number of keys and values should be the same in all cases, if they're + // not, something went majorly wrong, and we should bail + if (groupKeys.length !== groupValues.length) { + return undefined; + } + + return zipWith(groupKeys, groupValues, (key, value) => { + return {key, value}; + }); +} 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/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..2b4749c9dfe678 100644 --- a/static/app/views/insights/common/queries/useSortedTimeSeries.tsx +++ b/static/app/views/insights/common/queries/useSortedTimeSeries.tsx @@ -15,16 +15,19 @@ 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 { TimeSeries, + TimeSeriesGroupBy, TimeSeriesItem, } from 'sentry/views/dashboards/widgets/common/types'; import type {SamplingMode} from 'sentry/views/explore/hooks/useProgressiveQuery'; @@ -139,8 +142,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 +157,93 @@ 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); + + 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; + } + } - if (!hasMultipleYAxes) { - return { - [firstYAxis]: processedResults - .sort(([a], [b]) => a - b) - .map(([, series]) => series), - }; + allTimeSeries.push(timeSeries); + }); } - - return processedResults - .sort(([a], [b]) => a - b) - .reduce((acc, [, series]) => { - acc[series.yAxis] = [series]; - return acc; - }, {} as SeriesMap); } - // 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 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; + } + + const seriesData = groupData[axis] as EventsStats; + const [, timeSeries] = convertEventsStatsToTimeSeriesData(axis, seriesData); - 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 - ); - - 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, - seriesData: EventsStats, - alias?: string, - order?: number + yAxis: string, + seriesData: EventsStats ): [number, TimeSeries] { - const label = alias ?? (seriesName || FALLBACK_SERIES_NAME); - const values: TimeSeriesItem[] = seriesData.data.map( ([timestamp, countsForTimestamp], index) => { const item: TimeSeriesItem = { @@ -273,19 +272,19 @@ 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, }, }; - if (defined(order)) { - serie.meta.order = order; + if (defined(seriesData.order)) { + serie.meta.order = seriesData.order; } - return [seriesData.order ?? 0, serie]; + return [serie.meta.order ?? 0, serie]; } From 34264a1787a8c2731859eaf0ed296ee2d0610abf Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Mon, 29 Sep 2025 15:51:46 -0400 Subject: [PATCH 3/9] Remove unused helper --- .../timeSeries/isTimeSeriesOther.spec.tsx | 29 ------------------- .../utils/timeSeries/isTimeSeriesOther.tsx | 8 ----- 2 files changed, 37 deletions(-) delete mode 100644 static/app/utils/timeSeries/isTimeSeriesOther.spec.tsx delete mode 100644 static/app/utils/timeSeries/isTimeSeriesOther.tsx 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)); -} From ea8e06222000bab562a92f52541a0fd98255a743 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Mon, 29 Sep 2025 16:18:54 -0400 Subject: [PATCH 4/9] Remove unused import --- static/app/views/insights/common/queries/useSortedTimeSeries.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/static/app/views/insights/common/queries/useSortedTimeSeries.tsx b/static/app/views/insights/common/queries/useSortedTimeSeries.tsx index 2b4749c9dfe678..ca4a5381266356 100644 --- a/static/app/views/insights/common/queries/useSortedTimeSeries.tsx +++ b/static/app/views/insights/common/queries/useSortedTimeSeries.tsx @@ -27,7 +27,6 @@ import { } from 'sentry/views/dashboards/utils/isEventsStats'; import type { TimeSeries, - TimeSeriesGroupBy, TimeSeriesItem, } from 'sentry/views/dashboards/widgets/common/types'; import type {SamplingMode} from 'sentry/views/explore/hooks/useProgressiveQuery'; From 3cfd9cc084cb4f574efce51eb664934609f5bd41 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Wed, 1 Oct 2025 11:01:56 -0400 Subject: [PATCH 5/9] Add more robust order determination --- .../common/queries/useSortedTimeSeries.tsx | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/static/app/views/insights/common/queries/useSortedTimeSeries.tsx b/static/app/views/insights/common/queries/useSortedTimeSeries.tsx index ca4a5381266356..a56c761fbb458e 100644 --- a/static/app/views/insights/common/queries/useSortedTimeSeries.tsx +++ b/static/app/views/insights/common/queries/useSortedTimeSeries.tsx @@ -189,7 +189,11 @@ export function transformToSeriesMap( // 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); + const [, timeSeries] = convertEventsStatsToTimeSeriesData( + firstYAxis, + seriesData, + seriesData.order + ); if (fields) { const groupByFields = fields.filter(field => !yAxis.includes(field)); @@ -217,7 +221,11 @@ export function transformToSeriesMap( } const seriesData = groupData[axis] as EventsStats; - const [, timeSeries] = convertEventsStatsToTimeSeriesData(axis, seriesData); + const [, timeSeries] = convertEventsStatsToTimeSeriesData( + axis, + seriesData, + seriesData.order + ); if (fields) { const groupByFields = fields.filter(field => !yAxis.includes(field)); @@ -241,7 +249,8 @@ export function transformToSeriesMap( export function convertEventsStatsToTimeSeriesData( yAxis: string, - seriesData: EventsStats + seriesData: EventsStats, + order?: number ): [number, TimeSeries] { const values: TimeSeriesItem[] = seriesData.data.map( ([timestamp, countsForTimestamp], index) => { @@ -281,7 +290,7 @@ export function convertEventsStatsToTimeSeriesData( }, }; - if (defined(seriesData.order)) { + if (defined(order)) { serie.meta.order = seriesData.order; } From 13b5c91021084526d88e2ee53f56a4aee8edbfac Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Wed, 1 Oct 2025 11:02:29 -0400 Subject: [PATCH 6/9] Fix comment typo --- .../app/views/insights/common/queries/useSortedTimeSeries.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/views/insights/common/queries/useSortedTimeSeries.tsx b/static/app/views/insights/common/queries/useSortedTimeSeries.tsx index a56c761fbb458e..3adadf94bab109 100644 --- a/static/app/views/insights/common/queries/useSortedTimeSeries.tsx +++ b/static/app/views/insights/common/queries/useSortedTimeSeries.tsx @@ -209,7 +209,7 @@ export function transformToSeriesMap( } } - // Multiple series, _and_ grouped. The top level keys are groups, the lower-level are are the axes + // 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; From f70e6aaaa4ca360153761700d9b9fe312c9f00d7 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Wed, 1 Oct 2025 11:09:41 -0400 Subject: [PATCH 7/9] Whoops --- .../app/views/insights/common/queries/useSortedTimeSeries.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/views/insights/common/queries/useSortedTimeSeries.tsx b/static/app/views/insights/common/queries/useSortedTimeSeries.tsx index 3adadf94bab109..48d0a4a9285d66 100644 --- a/static/app/views/insights/common/queries/useSortedTimeSeries.tsx +++ b/static/app/views/insights/common/queries/useSortedTimeSeries.tsx @@ -291,7 +291,7 @@ export function convertEventsStatsToTimeSeriesData( }; if (defined(order)) { - serie.meta.order = seriesData.order; + serie.meta.order = order; } return [serie.meta.order ?? 0, serie]; From f66855d1d1d79d045bbb9de93a6011fdca2ebe29 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Wed, 1 Oct 2025 15:15:53 -0400 Subject: [PATCH 8/9] Allow mismatched value and key counts --- .../utils/timeSeries/parseGroupBy.spec.tsx | 38 +++++++++++++++++-- static/app/utils/timeSeries/parseGroupBy.tsx | 17 +++++---- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/static/app/utils/timeSeries/parseGroupBy.spec.tsx b/static/app/utils/timeSeries/parseGroupBy.spec.tsx index 3750b0eb620167..4a47eadb772556 100644 --- a/static/app/utils/timeSeries/parseGroupBy.spec.tsx +++ b/static/app/utils/timeSeries/parseGroupBy.spec.tsx @@ -20,14 +20,20 @@ describe('parseGroupBy', () => { ]); }); - it('returns undefined when more values than fields', () => { + it('handles more values than fields by using empty strings for extra values', () => { const result = parseGroupBy('value1,value2', ['field1']); - expect(result).toBeUndefined(); + expect(result).toEqual([ + {key: 'field1', value: 'value1'}, + {key: '', value: 'value2'}, + ]); }); - it('returns undefined when more fields than values', () => { + it('handles more fields than values by using empty strings for missing values', () => { const result = parseGroupBy('value1', ['field1', 'field2']); - expect(result).toBeUndefined(); + expect(result).toEqual([ + {key: 'field1', value: 'value1'}, + {key: 'field2', value: ''}, + ]); }); it('handles empty strings in values', () => { @@ -43,4 +49,28 @@ describe('parseGroupBy', () => { const result = parseGroupBy('', []); expect(result).toBeUndefined(); }); + + it('handles complex mismatch scenarios', () => { + // More values than fields - extra values ignored + const result1 = parseGroupBy('val1,val2,val3,val4', ['field1', 'field2']); + expect(result1).toEqual([ + {key: 'field1', value: 'val1'}, + {key: 'field2', value: 'val2'}, + ]); + + // More fields than values - missing values become empty strings + const result2 = parseGroupBy('val1', ['field1', 'field2', 'field3']); + expect(result2).toEqual([ + {key: 'field1', value: 'val1'}, + {key: 'field2', value: ''}, + {key: 'field3', value: ''}, + ]); + + // Empty group name with fields - all values become empty strings + const result3 = parseGroupBy('', ['field1', 'field2']); + expect(result3).toEqual([ + {key: 'field1', value: ''}, + {key: 'field2', value: ''}, + ]); + }); }); diff --git a/static/app/utils/timeSeries/parseGroupBy.tsx b/static/app/utils/timeSeries/parseGroupBy.tsx index 5a92c05285c89f..40c7ddd89a42bb 100644 --- a/static/app/utils/timeSeries/parseGroupBy.tsx +++ b/static/app/utils/timeSeries/parseGroupBy.tsx @@ -13,13 +13,14 @@ export function parseGroupBy( const groupKeys = fields; const groupValues = groupName.split(','); - // The number of keys and values should be the same in all cases, if they're - // not, something went majorly wrong, and we should bail - if (groupKeys.length !== groupValues.length) { - return undefined; - } - - return zipWith(groupKeys, groupValues, (key, value) => { - return {key, value}; + 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; } From 5325425aa17294eac702ba89f025deda26a33447 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Wed, 1 Oct 2025 15:51:02 -0400 Subject: [PATCH 9/9] Remove stray tests --- .../utils/timeSeries/parseGroupBy.spec.tsx | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/static/app/utils/timeSeries/parseGroupBy.spec.tsx b/static/app/utils/timeSeries/parseGroupBy.spec.tsx index 4a47eadb772556..f75a6e091b90f0 100644 --- a/static/app/utils/timeSeries/parseGroupBy.spec.tsx +++ b/static/app/utils/timeSeries/parseGroupBy.spec.tsx @@ -49,28 +49,4 @@ describe('parseGroupBy', () => { const result = parseGroupBy('', []); expect(result).toBeUndefined(); }); - - it('handles complex mismatch scenarios', () => { - // More values than fields - extra values ignored - const result1 = parseGroupBy('val1,val2,val3,val4', ['field1', 'field2']); - expect(result1).toEqual([ - {key: 'field1', value: 'val1'}, - {key: 'field2', value: 'val2'}, - ]); - - // More fields than values - missing values become empty strings - const result2 = parseGroupBy('val1', ['field1', 'field2', 'field3']); - expect(result2).toEqual([ - {key: 'field1', value: 'val1'}, - {key: 'field2', value: ''}, - {key: 'field3', value: ''}, - ]); - - // Empty group name with fields - all values become empty strings - const result3 = parseGroupBy('', ['field1', 'field2']); - expect(result3).toEqual([ - {key: 'field1', value: ''}, - {key: 'field2', value: ''}, - ]); - }); });