Skip to content

Commit ee3778b

Browse files
authored
fix: Escape filters with newlines etc (#723)
* fix: Filters with newlines etc * Fix test
1 parent 21fde20 commit ee3778b

5 files changed

Lines changed: 37 additions & 14 deletions

File tree

src/components/Explore/TracesByService/Tabs/Exceptions/ExceptionUtils.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,6 @@ export function normalizeExceptionMessage(message: string): string {
114114
return message.replace(/\s+/g, ' ').trim();
115115
}
116116

117-
export function escapeTraceQlString(value: string) {
118-
return value.replace(/["\\]/g, (s) => `\\${s}`);
119-
}
120-
121117
export function getDatasourceUidOrThrow(scene: SceneObject) {
122118
const datasourceUid = getDatasourceVariable(scene).state.value?.toString();
123119
if (!datasourceUid) {

src/components/Explore/TracesByService/Tabs/Exceptions/accordion/ExceptionAccordion.test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,13 @@ jest.mock('utils/utils', () => ({
55
getFiltersVariable: () => ({ state: { filters: [{ key: 'foo', operator: '=', value: 'bar' }] } }),
66
}));
77

8-
jest.mock('utils/filters-renderer', () => ({
9-
renderTraceQLLabelFilters: () => 'foo="bar"',
10-
}));
8+
jest.mock('utils/filters-renderer', () => {
9+
const actual = jest.requireActual<typeof import('utils/filters-renderer')>('utils/filters-renderer');
10+
return {
11+
...actual,
12+
renderTraceQLLabelFilters: () => 'foo="bar"',
13+
};
14+
});
1115

1216
describe('ExceptionAccordion helpers', () => {
1317
describe('getMessageHighlight', () => {

src/components/Explore/TracesByService/Tabs/Exceptions/accordion/ExceptionAccordion.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@ import { css } from '@emotion/css';
77
import { SceneObject } from '@grafana/scenes';
88

99
import { AttributesSidebar } from 'components/Explore/AttributesSidebar';
10-
import { renderTraceQLLabelFilters } from 'utils/filters-renderer';
10+
import { escapeTraceQlStringLiteral, renderTraceQLLabelFilters } from 'utils/filters-renderer';
1111
import { getFiltersVariable, getPrimarySignalVariable, getSpanListColumnsVariable, getTraceByServiceScene } from 'utils/utils';
1212
import { ExceptionRow } from '../ExceptionsTable';
1313
import { ExceptionComparison } from './ExceptionComparison';
1414
import { ExceptionTraceResults } from './ExceptionTraceResults';
15-
import { escapeTraceQlString } from '../ExceptionUtils';
1615

1716
interface ExceptionAccordionProps {
1817
row: ExceptionRow;
@@ -130,8 +129,8 @@ export const buildExceptionFilterExpr = ({
130129
const primarySignalExpr = (getPrimarySignalVariable(scene).state.value as string) || 'true';
131130
const filtersExpr = renderTraceQLLabelFilters(getFiltersVariable(scene).state.filters);
132131

133-
const escapedMessage = escapeTraceQlString(exceptionMessage);
134-
const typeFilter = exceptionType && exceptionType !== 'Unknown' ? ` && event.exception.type = "${escapeTraceQlString(exceptionType)}"` : '';
132+
const escapedMessage = escapeTraceQlStringLiteral(exceptionMessage);
133+
const typeFilter = exceptionType && exceptionType !== 'Unknown' ? ` && event.exception.type = "${escapeTraceQlStringLiteral(exceptionType)}"` : '';
135134

136135
return `{${primarySignalExpr} && ${filtersExpr} && status = error && event.exception.message = "${escapedMessage}"${typeFilter}}`;
137136
};

src/utils/filters-renderer.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,19 @@ describe('filters-renderer', () => {
125125
const filters: AdHocVariableFilter[] = [{ key: 'name', operator: '=', value: 'some "query" \\ ' }];
126126
expect(renderTraceQLLabelFilters(filters)).toBe('name="some \\"query\\" \\\\ "');
127127
});
128+
129+
it('should escape newlines tabs and carriage returns so TraceQL string literals stay closed', () => {
130+
const filters: AdHocVariableFilter[] = [
131+
{
132+
key: 'event.exception.message',
133+
operator: '=',
134+
value: 'No result found for query [\n SELECT a FROM stores a \n WHERE x = 1\n ]',
135+
},
136+
];
137+
expect(renderTraceQLLabelFilters(filters)).toBe(
138+
'event.exception.message="No result found for query [\\n SELECT a FROM stores a \\n WHERE x = 1\\n ]"'
139+
);
140+
});
128141
});
129142
});
130143
});

src/utils/filters-renderer.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
import { AdHocVariableFilter } from '@grafana/data';
22

3+
/**
4+
* Escapes a value for use inside TraceQL double-quoted string literals.
5+
* Raw newlines (and tabs/CR) break parsing with "literal not terminated"; they must be written as \\n etc.
6+
*/
7+
export function escapeTraceQlStringLiteral(value: string): string {
8+
return value
9+
.replace(/\\/g, '\\\\')
10+
.replace(/"/g, '\\"')
11+
.replace(/\n/g, '\\n')
12+
.replace(/\r/g, '\\r')
13+
.replace(/\t/g, '\\t');
14+
}
15+
316
export function renderTraceQLLabelFilters(filters: AdHocVariableFilter[]) {
417
const expr = filters
518
.filter((f) => f.key && f.operator && f.value)
@@ -29,9 +42,7 @@ function renderFilter(filter: AdHocVariableFilter) {
2942
!isQuotedNumericString(val)
3043
) {
3144
if (typeof val === 'string') {
32-
// Escape " and \ to \" and \\ respectively
33-
val = val.replace(/["\\]/g, (s) => `\\${s}`);
34-
val = `"${val}"`;
45+
val = `"${escapeTraceQlStringLiteral(val)}"`;
3546
}
3647
}
3748

0 commit comments

Comments
 (0)