Skip to content

Commit a03ef82

Browse files
authored
fix(dashboards): Charts displaying count(span.duration) (#97502)
Adds the prettify function call to: - rendering the sort options for tables properly - These render as a full string option in the dropdown because tables only allow current selections. It was showing `count(span.duration)` - rendering in charts - Updating the widget legend name decoder provides a single point that has the effect we want. This is essentially the deserialization layer from the URL params to the names used in the series. I use a regex match because as we add groupings or aliases, the series name is no longer simply `count(span.duration)` - Had to update the delimiter from `;` to something more unique because `>` was escaped to `>` which caused issues with the hover text. I went with something that was going to likely be more unique with `|~|`
1 parent 68c915a commit a03ef82

File tree

9 files changed

+59
-13
lines changed

9 files changed

+59
-13
lines changed

static/app/components/modals/widgetViewerModal.spec.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ describe('Modals -> WidgetViewerModal', function () {
425425
expect.objectContaining({
426426
option: expect.objectContaining({
427427
legend: expect.objectContaining({
428-
selected: {[`Query Name;${mockWidget.id}`]: false},
428+
selected: {[`Query Name|~|${mockWidget.id}`]: false},
429429
}),
430430
}),
431431
})

static/app/components/modals/widgetViewerModal.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ import {
4545
isAggregateField,
4646
isEquation,
4747
isEquationAlias,
48+
parseFunction,
49+
prettifyParsedFunction,
4850
} from 'sentry/utils/discover/fields';
4951
import {
5052
createOnDemandFilterWarning,
@@ -1324,6 +1326,14 @@ function ViewerTableV2({
13241326
: {}
13251327
);
13261328

1329+
// Inject any prettified function names that aren't currently aliased into the aliases
1330+
for (const column of tableColumns) {
1331+
const parsedFunction = parseFunction(column.key);
1332+
if (!aliases[column.key] && parsedFunction) {
1333+
aliases[column.key] = prettifyParsedFunction(parsedFunction);
1334+
}
1335+
}
1336+
13271337
if (loading) {
13281338
return (
13291339
<TableWidgetVisualization.LoadingPlaceholder

static/app/utils/discover/fields.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,7 @@ export function getMeasurementSlug(field: string): string | null {
900900

901901
const AGGREGATE_PATTERN = /^(\w+)\((.*)?\)$/;
902902
// Identical to AGGREGATE_PATTERN, but without the $ for newline, or ^ for start of line
903-
const AGGREGATE_BASE = /(\w+)\((.*)?\)/g;
903+
export const AGGREGATE_BASE = /(\w+)\((.*)?\)/g;
904904

905905
export function getAggregateArg(field: string): string | null {
906906
// only returns the first argument if field is an aggregate

static/app/views/dashboards/datasetConfig/errorsAndTransactions.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ import {
3333
isEquation,
3434
isEquationAlias,
3535
isLegalYAxisType,
36+
parseFunction,
37+
prettifyParsedFunction,
3638
SPAN_OP_BREAKDOWN_FIELDS,
3739
stripEquationPrefix,
3840
} from 'sentry/utils/discover/fields';
@@ -176,13 +178,18 @@ export function getTableSortOptions(
176178
.filter(field => !!field)
177179
.forEach(field => {
178180
let alias: any;
179-
const label = stripEquationPrefix(field);
181+
let label = stripEquationPrefix(field);
180182
// Equations are referenced via a standard alias following this pattern
181183
if (isEquation(field)) {
182184
alias = `equation[${equations}]`;
183185
equations += 1;
184186
}
185187

188+
const parsedFunction = parseFunction(field);
189+
if (parsedFunction) {
190+
label = prettifyParsedFunction(parsedFunction);
191+
}
192+
186193
options.push({label, value: alias ?? field});
187194
});
188195

static/app/views/dashboards/widgetCard/chart.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ import {
5050
isAggregateField,
5151
isEquation,
5252
maybeEquationAlias,
53+
parseFunction,
54+
prettifyParsedFunction,
5355
stripDerivedMetricsPrefix,
5456
stripEquationPrefix,
5557
} from 'sentry/utils/discover/fields';
@@ -195,6 +197,14 @@ class WidgetCardChart extends Component<WidgetCardChartProps> {
195197
const tableData = convertTableDataToTabularData(tableResults?.[i]);
196198
const sort = decodeSorts(widget.queries[0]?.orderby)?.[0];
197199

200+
// Inject any prettified function names that aren't currently aliased into the aliases
201+
for (const column of columns) {
202+
const parsedFunction = parseFunction(column.key);
203+
if (!aliases[column.key] && parsedFunction) {
204+
aliases[column.key] = prettifyParsedFunction(parsedFunction);
205+
}
206+
}
207+
198208
return (
199209
<TableWrapper key={`table:${result.title}`}>
200210
{organization.features.includes('dashboards-use-widget-table-visualization') ? (
@@ -481,7 +491,7 @@ class WidgetCardChart extends Component<WidgetCardChartProps> {
481491
};
482492

483493
const nameFormatter = (name: string) => {
484-
return WidgetLegendNameEncoderDecoder.decodeSeriesNameForLegend(name)!;
494+
return WidgetLegendNameEncoderDecoder.decodeSeriesNameForLegend(name);
485495
};
486496

487497
const chartOptions = {

static/app/views/dashboards/widgetLegendNameEncoderDecoder.tsx

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,34 @@
11
import type {Series} from 'sentry/types/echarts';
2+
import {
3+
AGGREGATE_BASE,
4+
parseFunction,
5+
prettifyParsedFunction,
6+
} from 'sentry/utils/discover/fields';
27
import type {Widget} from 'sentry/views/dashboards/types';
38

4-
const SERIES_NAME_DELIMITER = ';';
9+
const SERIES_NAME_DELIMITER = '|~|';
510

611
const WidgetLegendNameEncoderDecoder = {
712
encodeSeriesNameForLegend(seriesName: string, widgetId?: string) {
813
return `${seriesName}${SERIES_NAME_DELIMITER}${widgetId}`;
914
},
1015

1116
decodeSeriesNameForLegend(encodedSeriesName: string) {
12-
return encodedSeriesName.split(SERIES_NAME_DELIMITER)[0];
17+
let seriesName = encodedSeriesName.split(SERIES_NAME_DELIMITER)[0];
18+
if (!seriesName) {
19+
return '';
20+
}
21+
22+
// If the series name contains an aggregate function, prettify it
23+
const functionMatch = seriesName.match(AGGREGATE_BASE);
24+
if (functionMatch?.[0] && parseFunction(functionMatch[0])) {
25+
seriesName = seriesName.replace(
26+
functionMatch[0],
27+
prettifyParsedFunction(parseFunction(functionMatch[0])!)
28+
);
29+
}
30+
31+
return seriesName;
1332
},
1433

1534
// change timeseries names to SeriesName:widgetID

static/app/views/dashboards/widgetLegendSelectionState.spec.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {DisplayType, WidgetType} from 'sentry/views/dashboards/types';
1010
import WidgetLegendSelectionState from './widgetLegendSelectionState';
1111

1212
const WIDGET_ID_DELIMITER = ':';
13-
const SERIES_NAME_DELIMITER = ';';
13+
const SERIES_NAME_DELIMITER = '|~|';
1414

1515
describe('WidgetLegend functions util', () => {
1616
let legendFunctions: WidgetLegendSelectionState;

static/app/views/dashboards/widgetLegendSelectionState.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ class WidgetLegendSelectionState {
177177
.filter(key => !selected[key])
178178
.map(series =>
179179
encodeURIComponent(
180-
WidgetLegendNameEncoderDecoder.decodeSeriesNameForLegend(series)!
180+
WidgetLegendNameEncoderDecoder.decodeSeriesNameForLegend(series)
181181
)
182182
)
183183
.join(SERIES_LIST_DELIMITER)

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import {formatTimeSeriesName} from './formatTimeSeriesName';
55
describe('formatSeriesName', () => {
66
describe('releases', () => {
77
it.each([
8-
['p75(span.duration);11762', 'p75(span.duration)'],
9-
['Releases;', 'Releases'],
8+
['p75(span.duration)|~|11762', 'p75(span.duration)'],
9+
['Releases|~|', 'Releases'],
1010
])('Formats %s as %s', (name, result) => {
1111
const timeSeries = TimeSeriesFixture({
1212
yAxis: name,
@@ -72,8 +72,8 @@ describe('formatSeriesName', () => {
7272

7373
describe('combinations', () => {
7474
it.each([
75-
['equation|p75(measurements.cls) + 1;76123', 'p75(measurements.cls) + 1'],
76-
['equation|p75(measurements.cls);76123', 'p75(measurements.cls)'],
75+
['equation|p75(measurements.cls) + 1|~|76123', 'p75(measurements.cls) + 1'],
76+
['equation|p75(measurements.cls)|~|76123', 'p75(measurements.cls)'],
7777
])('Formats %s as %s', (name, result) => {
7878
const timeSeries = TimeSeriesFixture({
7979
yAxis: name,
@@ -86,7 +86,7 @@ describe('formatSeriesName', () => {
8686
describe('groupBy', () => {
8787
it.each([
8888
[
89-
'equation|p75(measurements.cls);76123',
89+
'equation|p75(measurements.cls)|~|76123',
9090
[{key: 'release', value: 'v0.0.2'}],
9191
'v0.0.2',
9292
],

0 commit comments

Comments
 (0)