Skip to content

Commit 47f772d

Browse files
gggritsoclaude
andauthored
ref(dashboards): Replace internal sqlish module with @sentry/sqlish (#112147)
Replaces the internal `sqlish` module with the `@sentry/sqlish` npm package which we published recently. No fuss, no muss, it's all the same code. Added a little wrapper around it, since that's good practice and a good place to keep having telemetry. Took this moment to add a sprinkle of tests to make sure things are working. I waffled on whether to add a CJS export to `sqlish` but decided that it's not worth it. You tell me though, it's easy to add. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 108ee22 commit 47f772d

File tree

21 files changed

+146
-667
lines changed

21 files changed

+146
-667
lines changed

jest.config.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,14 @@ if (
273273
* node_modules, but some packages which use ES6 syntax only NEED to be
274274
* transformed.
275275
*/
276-
const ESM_NODE_MODULES = ['screenfull', 'cbor2', 'nuqs', 'color', 'marked'];
276+
const ESM_NODE_MODULES = [
277+
'screenfull',
278+
'cbor2',
279+
'nuqs',
280+
'color',
281+
'marked',
282+
'@sentry\\+sqlish',
283+
];
277284

278285
const config: Config.InitialOptions = {
279286
verbose: false,
@@ -301,6 +308,11 @@ const config: Config.InitialOptions = {
301308
'^echarts/(.*)': '<rootDir>/tests/js/sentry-test/mocks/echartsMock.js',
302309
'^zrender/(.*)': '<rootDir>/tests/js/sentry-test/mocks/echartsMock.js',
303310

311+
// @sentry/sqlish is ESM-only with `exports` that only define `import`
312+
// conditions. Jest's CJS resolver can't follow them without explicit mapping.
313+
'^@sentry/sqlish/react$': '<rootDir>/node_modules/@sentry/sqlish/dist/react.js',
314+
'^@sentry/sqlish$': '<rootDir>/node_modules/@sentry/sqlish/dist/index.js',
315+
304316
// Disabled @sentry/toolbar in tests. It depends on iframes and global
305317
// window/cookies state.
306318
'@sentry/toolbar': '<rootDir>/tests/js/sentry-test/mocks/sentryToolbarMock.js',

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@
116116
"@sentry/node": "10.41.0-beta.0",
117117
"@sentry/react": "10.41.0-beta.0",
118118
"@sentry/release-parser": "^1.3.1",
119+
"@sentry/sqlish": "1.0.1",
119120
"@sentry/status-page-list": "^0.6.1",
120121
"@sentry/toolbar": "1.0.0-beta.16",
121122
"@sentry/webpack-plugin": "4.6.1",

pnpm-lock.yaml

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

static/app/components/events/interfaces/performance/spanEvidenceKeyValueList.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import type {Organization} from 'sentry/types/organization';
3939
import {formatBytesBase2} from 'sentry/utils/bytes/formatBytesBase2';
4040
import {generateLinkToEventInTraceView} from 'sentry/utils/discover/urls';
4141
import {toRoundedPercent} from 'sentry/utils/number/toRoundedPercent';
42-
import {SQLishFormatter} from 'sentry/utils/sqlish/SQLishFormatter';
42+
import {SQLishFormatter} from 'sentry/utils/sqlish';
4343
import {safeURL} from 'sentry/utils/url/safeURL';
4444
import {useLocation} from 'sentry/utils/useLocation';
4545
import {useOrganization} from 'sentry/utils/useOrganization';

static/app/utils/sqlish.tsx

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/**
2+
* Thin wrapper around @sentry/sqlish that adds Sentry observability
3+
* (performance spans and error capture with fingerprinting on parse failures)
4+
* and re-adds `toSimpleMarkup()` which the npm package doesn't include as a
5+
* class method — it ships `simpleMarkup` as a standalone function from
6+
* `@sentry/sqlish/react` instead.
7+
*/
8+
import * as Sentry from '@sentry/react';
9+
import {SQLishFormatter as BaseSQLishFormatter, SQLishParser} from '@sentry/sqlish';
10+
import {simpleMarkup} from '@sentry/sqlish/react';
11+
12+
type StringFormatterOptions = Parameters<BaseSQLishFormatter['toString']>[1];
13+
14+
export class SQLishFormatter {
15+
private formatter: BaseSQLishFormatter;
16+
private parser: SQLishParser;
17+
18+
constructor() {
19+
this.formatter = new BaseSQLishFormatter();
20+
this.parser = new SQLishParser();
21+
}
22+
23+
toString(sql: string, options?: StringFormatterOptions): string {
24+
return this._withSentry('string', () => this.formatter.toString(sql, options), sql);
25+
}
26+
27+
toSimpleMarkup(sql: string): React.ReactElement[] {
28+
return this._withSentry(
29+
'simpleMarkup',
30+
() => simpleMarkup(this.parser.parse(sql)),
31+
sql
32+
);
33+
}
34+
35+
private _withSentry<T>(format: string, fn: () => T, sql: string): T {
36+
const sentrySpan = Sentry.startInactiveSpan({
37+
op: 'function',
38+
name: 'SQLishFormatter.toFormat',
39+
attributes: {format},
40+
onlyIfParent: true,
41+
});
42+
43+
try {
44+
const result = fn();
45+
return result;
46+
} catch (error: any) {
47+
Sentry.withScope(scope => {
48+
const fingerprint = ['sqlish-parse-error'];
49+
50+
if (error?.message) {
51+
scope.setExtra('message', error.message?.slice(-100));
52+
scope.setExtra('found', error.found);
53+
54+
if (error.message.includes('Expected')) {
55+
fingerprint.push(error.message.slice(-10));
56+
}
57+
}
58+
59+
scope.setFingerprint(fingerprint);
60+
Sentry.captureException(error);
61+
});
62+
63+
// If we fail to parse the SQL, return the original string
64+
return sql as unknown as T;
65+
} finally {
66+
sentrySpan?.end();
67+
}
68+
}
69+
}

static/app/utils/sqlish/SQLishFormatter.spec.tsx

Lines changed: 0 additions & 148 deletions
This file was deleted.

static/app/utils/sqlish/SQLishFormatter.tsx

Lines changed: 0 additions & 82 deletions
This file was deleted.

0 commit comments

Comments
 (0)