Skip to content

Commit 0b23a17

Browse files
gggritsopriscilawebdev
authored andcommitted
fix(timeseries): Improve parsing of null groups (#102087)
Unlike `/events-timeseries/`, `/events-stats/` has an issue where a `null` group gets converted to `"None"` by stringifying that key in Python. This is a mismatch between the endpoints, and I'm resolving it here by correctly accounting for `"None"` as a key. I've also taken this opportunity to improve how `null` groups are handled. They are now showns as `"(no value)"` in more places. N.B. Checking for `"null"` in the `ModelName` is a hack, sorry! It's due to how the code is arranged. `ModelName` is used in a field renderer, which checks the data values beforehand. It's also used directly by some table components, where checking the value is a little more annoying. I opted to check for `"null"`, so it's all in once place. We could make this situation a lot better by using the field renderers in the table widgets! With this fix, the values in the chart tooltips and table values in Explore and the AI modules work correctly. **e.g.,** <img width="168" height="419" alt="Screenshot 2025-10-24 at 12 08 28 PM" src="https://github.com/user-attachments/assets/70bf6f59-7981-475b-b042-3cad7e939db7" /> <img width="356" height="296" alt="Screenshot 2025-10-24 at 12 10 35 PM" src="https://github.com/user-attachments/assets/ddc7c466-23dd-4ef4-aa79-78b29a0d7e90" /> <img width="178" height="216" alt="Screenshot 2025-10-24 at 12 54 15 PM" src="https://github.com/user-attachments/assets/836e4478-fb5c-45f2-9922-d372124c56f7" />
1 parent 30c6852 commit 0b23a17

File tree

5 files changed

+22
-4
lines changed

5 files changed

+22
-4
lines changed

static/app/utils/timeSeries/parseGroupBy.spec.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ describe('parseGroupBy', () => {
66
expect(result).toBeNull();
77
});
88

9+
it('returns null for "None" group name', () => {
10+
const result = parseGroupBy('None', ['field1']);
11+
expect(result).toEqual([{key: 'field1', value: null}]);
12+
});
13+
914
it('parses single field and value correctly', () => {
1015
const result = parseGroupBy('value1', ['field1']);
1116
expect(result).toEqual([{key: 'field1', value: 'value1'}]);

static/app/utils/timeSeries/parseGroupBy.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export function parseGroupBy(
66
groupName: string,
77
fields: string[]
88
): TimeSeriesGroupBy[] | null {
9+
// "Other", by definition, does not have any known group by values
910
if (groupName === 'Other') {
1011
return null;
1112
}
@@ -15,6 +16,15 @@ export function parseGroupBy(
1516
}
1617

1718
const groupKeys = fields;
19+
20+
// `/events-stats/` converts Python's `None` into `"None"`. Here, we do the reverse
21+
if (groupName === 'None') {
22+
return groupKeys.map(groupKey => ({
23+
key: groupKey,
24+
value: null,
25+
}));
26+
}
27+
1828
const groupValues = groupName.split(DELIMITER);
1929

2030
// If the `groupName` contains commas, that will result in more values than

static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesLabel.spec.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ describe('formatSeriesName', () => {
9191
'v0.0.2',
9292
],
9393
['p95(span.duration)', [{key: 'release', value: 'v0.0.2'}], 'v0.0.2'],
94-
['p95(span.duration)', [{key: 'gen_ai.request.model', value: null}], 'null'],
94+
['p95(span.duration)', [{key: 'gen_ai.request.model', value: null}], '(no value)'],
9595
[
9696
'p95(span.duration)',
9797
[

static/app/views/dashboards/widgets/timeSeriesWidget/formatters/formatTimeSeriesLabel.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,10 @@ export function formatTimeSeriesLabel(timeSeries: TimeSeries): string {
2828
return formatVersion(groupBy.value);
2929
}
3030
31-
// String interpolation converts `null` to `"null"` and `undefined` to
32-
// `"undefined"`, which is what we want, rather than an empty string
31+
if (groupBy.value === null) {
32+
return t('(no value)');
33+
}
34+
3335
return `${groupBy.value}`;
3436
})
3537
.join(',')}`;

static/app/views/insights/agents/components/modelName.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import styled from '@emotion/styled';
22
import {PlatformIcon} from 'platformicons';
33

44
import {Flex} from 'sentry/components/core/layout';
5+
import {t} from 'sentry/locale';
56
import type {Space} from 'sentry/utils/theme/theme';
67

78
interface ModelNameProps {
@@ -19,7 +20,7 @@ export function ModelName({modelId, provider, size = 16, gap = 'md'}: ModelNameP
1920
<IconWrapper>
2021
<PlatformIcon platform={platform ?? 'unknown'} size={size} />
2122
</IconWrapper>
22-
<NameWrapper>{modelId}</NameWrapper>
23+
<NameWrapper>{modelId === 'null' ? t('(no value)') : modelId}</NameWrapper>
2324
</Flex>
2425
);
2526
}

0 commit comments

Comments
 (0)