From bf40d636fd1c79347e8de5219e16f960512e2171 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Thu, 2 Apr 2026 15:41:40 -0400 Subject: [PATCH 1/3] ref(insights): Replace internal sqlish module with @sentry/sqlish package Replace the internal `static/app/utils/sqlish/` module (parser, formatter, PEG grammar, and supporting utilities) with the extracted `@sentry/sqlish` npm package. A thin wrapper at `static/app/utils/sqlish.tsx` preserves the same class API and adds Sentry observability (performance spans and error capture with fingerprinting on parse failures). Add `@sentry/sqlish` to Jest's ESM_NODE_MODULES allowlist and moduleNameMapper for ESM-only package resolution. Co-Authored-By: Claude --- jest.config.ts | 13 +- package.json | 1 + pnpm-lock.yaml | 15 ++ .../performance/spanEvidenceKeyValueList.tsx | 2 +- static/app/utils/sqlish.tsx | 63 ++++++++ .../app/utils/sqlish/SQLishFormatter.spec.tsx | 148 ------------------ static/app/utils/sqlish/SQLishFormatter.tsx | 82 ---------- static/app/utils/sqlish/SQLishParser.spec.tsx | 116 -------------- static/app/utils/sqlish/SQLishParser.ts | 9 -- .../utils/sqlish/formatters/simpleMarkup.tsx | 27 ---- static/app/utils/sqlish/formatters/string.ts | 98 ------------ .../sqlish/formatters/stringAccumulator.ts | 122 --------------- static/app/utils/sqlish/sqlish.pegjs | 46 ------ static/app/utils/sqlish/types.ts | 11 -- .../common/components/fullSpanDescription.tsx | 2 +- .../common/components/spanDescription.tsx | 2 +- .../tableCells/spanDescriptionCell.spec.tsx | 41 +++++ .../tableCells/spanDescriptionCell.tsx | 2 +- .../details/span/eapSections/description.tsx | 2 +- .../details/span/sections/description.tsx | 2 +- .../details/span/sections/generalInfo.tsx | 2 +- 21 files changed, 139 insertions(+), 667 deletions(-) create mode 100644 static/app/utils/sqlish.tsx delete mode 100644 static/app/utils/sqlish/SQLishFormatter.spec.tsx delete mode 100644 static/app/utils/sqlish/SQLishFormatter.tsx delete mode 100644 static/app/utils/sqlish/SQLishParser.spec.tsx delete mode 100644 static/app/utils/sqlish/SQLishParser.ts delete mode 100644 static/app/utils/sqlish/formatters/simpleMarkup.tsx delete mode 100644 static/app/utils/sqlish/formatters/string.ts delete mode 100644 static/app/utils/sqlish/formatters/stringAccumulator.ts delete mode 100644 static/app/utils/sqlish/sqlish.pegjs delete mode 100644 static/app/utils/sqlish/types.ts create mode 100644 static/app/views/insights/common/components/tableCells/spanDescriptionCell.spec.tsx diff --git a/jest.config.ts b/jest.config.ts index 04d12ff49b3685..c82e853ffa6f5d 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -273,7 +273,14 @@ if ( * node_modules, but some packages which use ES6 syntax only NEED to be * transformed. */ -const ESM_NODE_MODULES = ['screenfull', 'cbor2', 'nuqs', 'color', 'marked']; +const ESM_NODE_MODULES = [ + 'screenfull', + 'cbor2', + 'nuqs', + 'color', + 'marked', + '@sentry\\+sqlish', +]; const config: Config.InitialOptions = { verbose: false, @@ -301,6 +308,10 @@ const config: Config.InitialOptions = { '^echarts/(.*)': '/tests/js/sentry-test/mocks/echartsMock.js', '^zrender/(.*)': '/tests/js/sentry-test/mocks/echartsMock.js', + // @sentry/sqlish uses ESM exports which Jest can't resolve directly + '^@sentry/sqlish/react$': '/node_modules/@sentry/sqlish/dist/react.js', + '^@sentry/sqlish$': '/node_modules/@sentry/sqlish/dist/index.js', + // Disabled @sentry/toolbar in tests. It depends on iframes and global // window/cookies state. '@sentry/toolbar': '/tests/js/sentry-test/mocks/sentryToolbarMock.js', diff --git a/package.json b/package.json index ff38e7f980eac3..23cc5ef2f90ce2 100644 --- a/package.json +++ b/package.json @@ -115,6 +115,7 @@ "@sentry/node": "10.41.0-beta.0", "@sentry/react": "10.41.0-beta.0", "@sentry/release-parser": "^1.3.1", + "@sentry/sqlish": "1.0.1", "@sentry/status-page-list": "^0.6.1", "@sentry/toolbar": "1.0.0-beta.16", "@sentry/webpack-plugin": "4.6.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2f4a3bc04787ce..43fd946a46af26 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -207,6 +207,9 @@ importers: '@sentry/release-parser': specifier: ^1.3.1 version: 1.3.1 + '@sentry/sqlish': + specifier: 1.0.1 + version: 1.0.1(react@19.2.3) '@sentry/status-page-list': specifier: ^0.6.1 version: 0.6.1 @@ -3497,6 +3500,14 @@ packages: '@sentry/release-parser@1.3.1': resolution: {integrity: sha512-/dGpCq+j3sJhqQ14RNEEL45Ot/rgq3jAlZDD/8ufeqq+W8p4gUhSrbGWCRL82NEIWY9SYwxYXGXjRcVPSHiA1Q==} + '@sentry/sqlish@1.0.1': + resolution: {integrity: sha512-8Ioewv2Qo4Y18T/O8M9FdoUD70VFUs+gu3XKsXQmEqFbgLl1fVkInmk1IL98fJzNADGCyRXsHGptsISPVW8OsA==} + peerDependencies: + react: '>=17' + peerDependenciesMeta: + react: + optional: true + '@sentry/status-page-list@0.6.1': resolution: {integrity: sha512-MCDLIEvzFKeDae1rSEU30O8QpcWqSECvDQaViW3eeGGLzxhr9rEI7rdKuHmouWT9hXCmYEK1W1DWKE534+Vv1g==} engines: {node: '>=14'} @@ -13206,6 +13217,10 @@ snapshots: '@sentry/release-parser@1.3.1': {} + '@sentry/sqlish@1.0.1(react@19.2.3)': + optionalDependencies: + react: 19.2.3 + '@sentry/status-page-list@0.6.1': {} '@sentry/toolbar@1.0.0-beta.16(react@19.2.3)': diff --git a/static/app/components/events/interfaces/performance/spanEvidenceKeyValueList.tsx b/static/app/components/events/interfaces/performance/spanEvidenceKeyValueList.tsx index e85961069f8ce2..670007744daf49 100644 --- a/static/app/components/events/interfaces/performance/spanEvidenceKeyValueList.tsx +++ b/static/app/components/events/interfaces/performance/spanEvidenceKeyValueList.tsx @@ -39,7 +39,7 @@ import type {Organization} from 'sentry/types/organization'; import {formatBytesBase2} from 'sentry/utils/bytes/formatBytesBase2'; import {generateLinkToEventInTraceView} from 'sentry/utils/discover/urls'; import {toRoundedPercent} from 'sentry/utils/number/toRoundedPercent'; -import {SQLishFormatter} from 'sentry/utils/sqlish/SQLishFormatter'; +import {SQLishFormatter} from 'sentry/utils/sqlish'; import {safeURL} from 'sentry/utils/url/safeURL'; import {useLocation} from 'sentry/utils/useLocation'; import {useOrganization} from 'sentry/utils/useOrganization'; diff --git a/static/app/utils/sqlish.tsx b/static/app/utils/sqlish.tsx new file mode 100644 index 00000000000000..7c768e11720643 --- /dev/null +++ b/static/app/utils/sqlish.tsx @@ -0,0 +1,63 @@ +/** + * Thin wrapper around @sentry/sqlish that adds Sentry observability + * (performance spans and error capture with fingerprinting on parse failures) + * and re-adds `toSimpleMarkup()` which the npm package doesn't include as a + * class method — it ships `simpleMarkup` as a standalone function from + * `@sentry/sqlish/react` instead. + */ +import * as Sentry from '@sentry/react'; +import {SQLishFormatter as BaseSQLishFormatter} from '@sentry/sqlish'; +import {simpleMarkup} from '@sentry/sqlish/react'; + +export {SQLishParser} from '@sentry/sqlish'; +export type {Token} from '@sentry/sqlish'; + +type StringFormatterOptions = Parameters[1]; + +export class SQLishFormatter extends BaseSQLishFormatter { + override toString(sql: string, options?: StringFormatterOptions): string { + return this._withSentry('string', () => super.toString(sql, options), sql); + } + + toSimpleMarkup(sql: string): React.ReactElement[] { + return this._withSentry( + 'simpleMarkup', + () => simpleMarkup(this.parser.parse(sql)), + sql + ); + } + + private _withSentry(format: string, fn: () => T, sql: string): T { + const sentrySpan = Sentry.startInactiveSpan({ + op: 'function', + name: 'SQLishFormatter.toFormat', + attributes: {format}, + onlyIfParent: true, + }); + + try { + const result = fn(); + sentrySpan?.end(); + return result; + } catch (error: any) { + Sentry.withScope(scope => { + const fingerprint = ['sqlish-parse-error']; + + if (error?.message) { + scope.setExtra('message', error.message?.slice(-100)); + scope.setExtra('found', error.found); + + if (error.message.includes('Expected')) { + fingerprint.push(error.message.slice(-10)); + } + } + + scope.setFingerprint(fingerprint); + Sentry.captureException(error); + }); + + // If we fail to parse the SQL, return the original string + return sql as unknown as T; + } + } +} diff --git a/static/app/utils/sqlish/SQLishFormatter.spec.tsx b/static/app/utils/sqlish/SQLishFormatter.spec.tsx deleted file mode 100644 index 2cdefb948fcff4..00000000000000 --- a/static/app/utils/sqlish/SQLishFormatter.spec.tsx +++ /dev/null @@ -1,148 +0,0 @@ -import {Fragment} from 'react'; - -import {render} from 'sentry-test/reactTestingLibrary'; - -import {SQLishFormatter} from 'sentry/utils/sqlish/SQLishFormatter'; - -describe('SQLishFormatter', () => { - describe('SQLishFormatter.toString()', () => { - const formatter = new SQLishFormatter(); - - it('Falls back to original string if unable to parse', () => { - expect(formatter.toString('😤')).toBe('😤'); - }); - - it('Adds newlines for keywords in SELECTs', () => { - expect(formatter.toString('SELECT hello FROM users ORDER BY name DESC LIMIT 1;')) - .toMatchInlineSnapshot(` - "SELECT hello - FROM users - ORDER BY name DESC - LIMIT 1;" - `); - }); - - it('Adds newlines for keywords in INSERTs', () => { - expect( - formatter.toString('INSERT INTO users (id, name) VALUES (:c0, :c1) RETURNING *') - ).toMatchInlineSnapshot(` - "INSERT INTO users (id, name) - VALUES ( - :c0, :c1 - ) - RETURNING *" - `); - }); - - it('Adds indentation for keywords followed by parentheses', () => { - expect(formatter.toString('SELECT * FROM (SELECT * FROM users))')) - .toMatchInlineSnapshot(` - "SELECT * - FROM ( - SELECT * - FROM users - ))" - `); - }); - - it('Capitalizes lowercase keywords', () => { - expect(formatter.toString('select * from users;')).toMatchInlineSnapshot(` - "SELECT * - FROM users;" - `); - }); - - it('Adds indentation for SELECTS in conditions', () => { - expect( - formatter.toString( - 'SELECT * FROM "sentry_users" WHERE (id IN (SELECT VO."id" FROM "sentry_vips" VO LIMIT 1)) AND (id IN (SELECT V1."id" FROM "sentry_currentusers" V1 LIMIT 1)) LIMIT 1' - ) - ).toMatchInlineSnapshot(` - "SELECT * - FROM "sentry_users" - WHERE ( - id IN ( - SELECT VO."id" - FROM "sentry_vips" VO - LIMIT 1 - ) - ) AND (id IN ( - SELECT V1."id" - FROM "sentry_currentusers" V1 - LIMIT 1 - )) - LIMIT 1" - `); - }); - - it('Reflows long lines to a max length', () => { - expect( - formatter.toString( - 'SELECT "sentry_organization"."id", "sentry_organization"."name", "sentry_organization"."slug", "sentry_organization"."status", "sentry_organization"."date_added", "sentry_organization"."default_role", "sentry_organization"."is_test", "sentry_organization"."flags" FROM "sentry_organization" WHERE "sentry_organization"."id" = %s LIMIT 21' - ) - ).toMatchInlineSnapshot(` - "SELECT "sentry_organization"."id", "sentry_organization"."name", "sentry_organization"."slug", - "sentry_organization"."status", "sentry_organization"."date_added", - "sentry_organization"."default_role", "sentry_organization"."is_test", "sentry_organization"."flags" - FROM "sentry_organization" - WHERE "sentry_organization"."id" = %s - LIMIT 21" - `); - }); - - it('Reflows to specified width', () => { - expect( - formatter.toString( - 'SELECT "sentry_organization"."id", "sentry_organization"."name", "sentry_organization"."slug", "sentry_organization"."status", "sentry_organization"."date_added" FROM "sentry_organization" WHERE "sentry_organization"."id" = %s LIMIT 21', - {maxLineLength: 40} - ) - ).toMatchInlineSnapshot(` - "SELECT "sentry_organization"."id", - "sentry_organization"."name", - "sentry_organization"."slug", - "sentry_organization"."status", - "sentry_organization"."date_added" - FROM "sentry_organization" - WHERE "sentry_organization"."id" = %s - LIMIT 21" - `); - }); - - it('Reflows avoid unnecessary newlines', () => { - expect( - formatter.toString( - 'SELECT "sentry_team"."org_role" FROM "sentry_team" INNER JOIN "sentry_organizationmember_teams" ON ("sentry_team"."id" = "sentry_organizationmember_teams"."team_id" WHERE ( "sentry_organizationmember_teams"."organizationmember_id" = %s AND NOT ("sentry_team"."org_role" IS NULL)' - ) - ).toMatchInlineSnapshot(` - "SELECT "sentry_team"."org_role" - FROM "sentry_team" - INNER JOIN "sentry_organizationmember_teams" ON ("sentry_team"."id" = - "sentry_organizationmember_teams"."team_id" - WHERE ( - "sentry_organizationmember_teams"."organizationmember_id" = %s AND NOT ("sentry_team"."org_role" IS - NULL)" - `); - }); - }); - - describe('SQLishFormatter.toSimpleMarkup()', () => { - const formatter = new SQLishFormatter(); - const getMarkup = (markup: any): string => { - const {container} = render({markup}); - - return container.innerHTML; - }; - - it('Capitalizes keywords', () => { - expect(getMarkup(formatter.toSimpleMarkup('select hello'))).toMatchInlineSnapshot( - `"SELECT hello"` - ); - }); - - it('Wraps every token in a `` element', () => { - expect(getMarkup(formatter.toSimpleMarkup('SELECT hello;'))).toMatchInlineSnapshot( - `"SELECT hello;"` - ); - }); - }); -}); diff --git a/static/app/utils/sqlish/SQLishFormatter.tsx b/static/app/utils/sqlish/SQLishFormatter.tsx deleted file mode 100644 index c31726e5ec791c..00000000000000 --- a/static/app/utils/sqlish/SQLishFormatter.tsx +++ /dev/null @@ -1,82 +0,0 @@ -import * as Sentry from '@sentry/react'; - -import {simpleMarkup} from 'sentry/utils/sqlish/formatters/simpleMarkup'; -import {string} from 'sentry/utils/sqlish/formatters/string'; -import {SQLishParser} from 'sentry/utils/sqlish/SQLishParser'; - -type StringFormatterOptions = Parameters[1]; - -enum Format { - STRING = 'string', - SIMPLE_MARKUP = 'simpleMarkup', -} - -const FORMATTERS = { - [Format.STRING]: string, - [Format.SIMPLE_MARKUP]: simpleMarkup, -}; - -export class SQLishFormatter { - parser: SQLishParser; - - constructor() { - this.parser = new SQLishParser(); - } - - toString(sql: string, options?: StringFormatterOptions) { - return this.toFormat(sql, Format.STRING, options); - } - - toSimpleMarkup(sql: string) { - return this.toFormat(sql, Format.SIMPLE_MARKUP); - } - - toFormat(sql: string, format: Format.STRING, options?: StringFormatterOptions): string; - toFormat(sql: string, format: Format.SIMPLE_MARKUP): React.ReactElement[]; - toFormat(sql: string, format: Format, options?: StringFormatterOptions) { - let tokens: any; - - const sentrySpan = Sentry.startInactiveSpan({ - op: 'function', - name: 'SQLishFormatter.toFormat', - attributes: { - format, - }, - onlyIfParent: true, - }); - - try { - tokens = this.parser.parse(sql); - } catch (error: any) { - Sentry.withScope(scope => { - const fingerprint = ['sqlish-parse-error']; - - if (error?.message) { - // The error message might be a PEG grammar error, or some kind of - // other parser error. They're usually too long to be useful since PEG - // errors contain the entire current grammar. Take the last 100 - // characters instead. - scope.setExtra('message', error.message?.slice(-100)); - scope.setExtra('found', error.found); - - // Grammar error. Group these by the specific missing grammar, so we - // can make case-by-case decisions whether we should amend the - // grammar. - if (error.message.includes('Expected')) { - fingerprint.push(error.message.slice(-10)); - } - } - - scope.setFingerprint(fingerprint); - Sentry.captureException(error); - }); - // If we fail to parse the SQL, return the original string - return sql; - } - - const formattedString = FORMATTERS[format](tokens, options); - sentrySpan?.end(); - - return formattedString; - } -} diff --git a/static/app/utils/sqlish/SQLishParser.spec.tsx b/static/app/utils/sqlish/SQLishParser.spec.tsx deleted file mode 100644 index a9d21e73421e60..00000000000000 --- a/static/app/utils/sqlish/SQLishParser.spec.tsx +++ /dev/null @@ -1,116 +0,0 @@ -import {SQLishParser} from 'sentry/utils/sqlish/SQLishParser'; - -describe('SQLishParser', () => { - describe('SQLishParser()', () => { - const parser = new SQLishParser(); - - it.each([ - 'SELECT;', - 'SELECT hello;', - 'SELECT *;', // Wildcards - 'WHERE age = 10;', // Equality - 'WHERE age != 10;', // Inequality - 'total / time', // Division - 'sum(age)::numeric(0, 5)', // Type casting - 'WHERE age > 10 AND age < 20;', // Comparison - "WHERE$1 ILIKE ' % ' || 'text'", // Conditionals - 'SELECT id, name;', // Column lists - 'columns AS `tags[column]`', // ClickHouse backtics - 'SELECT * FROM #temp', // Temporary tables - '# Fetches', // Comments - '\r\n', // Windows newlines - '✌🏻', // Emoji - 'ă', // Unicode - `\u0000`, // Unicode null - 'SELECT id, nam*', // Truncation - 'AND created >= :c1', // PHP-Style I - 'LIMIT $2', // PHP-style II - 'created >= %s', // Python-style - 'created >= $1', // Rails-style - '@@ to_tsquery', // Postgres full-text search - 'flags & %s)', // Bitwise AND - 'flags | %s)', // Bitwise OR - 'flags ^ %s)', // Bitwise XOR - 'flags ~ %s)', // Bitwise NOT - 'FROM temp{%s}', // Relay integer stripping - '+ %s as count', // Arithmetic I - '- %s as count', // Arithmetic II - "ILIKE '\\_')", // Backslash - ])('Parses %s', sql => { - expect(() => { - parser.parse(sql); - }).not.toThrow(); - }); - }); - - describe('SQLishParser.parse', () => { - const parser = new SQLishParser(); - - it('Distinguishes between real keywords and interpolated words', () => { - expect(parser.parse('SELECT country')).toEqual([ - { - type: 'Keyword', - content: 'SELECT', - }, - { - type: 'Whitespace', - content: ' ', - }, - { - type: 'GenericToken', - content: 'country', - }, - ]); - - expect(parser.parse('SELECT discount')).toEqual([ - { - type: 'Keyword', - content: 'SELECT', - }, - { - type: 'Whitespace', - content: ' ', - }, - { - type: 'GenericToken', - content: 'discount', - }, - ]); - }); - - it('Detects collapsed columns', () => { - expect(parser.parse('select ..')).toEqual([ - { - type: 'Keyword', - content: 'SELECT', - }, - { - type: 'Whitespace', - content: ' ', - }, - { - type: 'CollapsedColumns', - content: '..', - }, - ]); - }); - - it('Detects whitespace between generic tokens and JOIN commands', () => { - expect(parser.parse('sentry_users INNER JOIN sentry_messages')).toEqual([ - { - type: 'GenericToken', - content: 'sentry_users', - }, - {type: 'Whitespace', content: ' '}, - {type: 'Keyword', content: 'INNER'}, - {type: 'Whitespace', content: ' '}, - {type: 'Keyword', content: 'JOIN'}, - {type: 'Whitespace', content: ' '}, - { - type: 'GenericToken', - content: 'sentry_messages', - }, - ]); - }); - }); -}); diff --git a/static/app/utils/sqlish/SQLishParser.ts b/static/app/utils/sqlish/SQLishParser.ts deleted file mode 100644 index 917b23c06466ea..00000000000000 --- a/static/app/utils/sqlish/SQLishParser.ts +++ /dev/null @@ -1,9 +0,0 @@ -import type {Token} from 'sentry/utils/sqlish/types'; - -import {parse} from './sqlish.pegjs'; - -export class SQLishParser { - parse(sql: string) { - return parse(sql) as Token[]; - } -} diff --git a/static/app/utils/sqlish/formatters/simpleMarkup.tsx b/static/app/utils/sqlish/formatters/simpleMarkup.tsx deleted file mode 100644 index fa3a0e2a16c409..00000000000000 --- a/static/app/utils/sqlish/formatters/simpleMarkup.tsx +++ /dev/null @@ -1,27 +0,0 @@ -import type {Token} from 'sentry/utils/sqlish/types'; - -export function simpleMarkup(tokens: Token[]): React.ReactElement[] { - const accumulator: React.ReactElement[] = []; - - function contentize(token: Token, index: number): void { - if (Array.isArray(token.content)) { - token.content.forEach(contentize); - return; - } - - if (typeof token.content === 'string') { - if (token.type === 'Keyword') { - accumulator.push({token.content.toUpperCase()}); - } else if (token.type === 'Whitespace') { - accumulator.push( ); - } else { - accumulator.push({token.content}); - } - } - - return; - } - - tokens.forEach(contentize); - return accumulator; -} diff --git a/static/app/utils/sqlish/formatters/string.ts b/static/app/utils/sqlish/formatters/string.ts deleted file mode 100644 index b8a66b5ae4af26..00000000000000 --- a/static/app/utils/sqlish/formatters/string.ts +++ /dev/null @@ -1,98 +0,0 @@ -import {StringAccumulator} from 'sentry/utils/sqlish/formatters/stringAccumulator'; -import type {Token} from 'sentry/utils/sqlish/types'; - -interface Options { - maxLineLength?: number; -} - -export function string(tokens: Token[], options: Options = {}): string { - const accumulator = new StringAccumulator(); - - let precedingNonWhitespaceToken: Token | undefined = undefined; - let parenthesisLevel = 0; // Tracks the current parenthesis nesting level - const indentationLevels: number[] = []; // Tracks the parenthesis nesting levels at which we've incremented the indentation - - function contentize(token: Token): void { - if (Array.isArray(token.content)) { - token.content.forEach(contentize); - return; - } - - if (token.type === 'LeftParenthesis') { - parenthesisLevel += 1; - accumulator.add('('); - - // If the previous legible token is a meaningful keyword that triggers a - // newline, increase the current indentation level and note the parenthesis level where this happened - if ( - typeof precedingNonWhitespaceToken?.content === 'string' && - PARENTHESIS_NEWLINE_KEYWORDS.has(precedingNonWhitespaceToken.content) - ) { - accumulator.break(); - - accumulator.indent(); - indentationLevels.push(parenthesisLevel); - } - } - - if (token.type === 'RightParenthesis') { - // If this right parenthesis closes a left parenthesis at a level where - // we incremented the indentation, decrement the indentation - if (indentationLevels.at(-1) === parenthesisLevel) { - accumulator.break(); - accumulator.unindent(); - indentationLevels.pop(); - } - - parenthesisLevel -= 1; - accumulator.add(')'); - } - - if (typeof token.content === 'string') { - if (token.type === 'Keyword' && NEWLINE_KEYWORDS.has(token.content)) { - if (!accumulator.lastLine.isEmpty) { - accumulator.break(); - } - - accumulator.add(token.content); - } else if (token.type === 'Whitespace') { - // Convert all whitespace to single spaces - accumulator.space(); - } else if (['LeftParenthesis', 'RightParenthesis'].includes(token.type)) { - // Parenthesis contents are appended above, so we can skip them here - } else { - accumulator.add(token.content); - } - } - - if (token.type !== 'Whitespace') { - precedingNonWhitespaceToken = token; - } - - return; - } - - tokens.forEach(contentize); - return accumulator.toString(options.maxLineLength); -} - -// Keywords that always trigger a newline -const NEWLINE_KEYWORDS = new Set([ - 'DELETE', - 'FROM', - 'GROUP', - 'INNER', - 'INSERT', - 'LEFT', - 'LIMIT', - 'OFFSET', - 'ORDER', - 'RETURNING', - 'RIGHT', - 'SELECT', - 'VALUES', - 'WHERE', -]); - -// Keywords that may or may not trigger a newline, but they always trigger a newlines if followed by a parenthesis -const PARENTHESIS_NEWLINE_KEYWORDS = new Set([...NEWLINE_KEYWORDS, ...['IN']]); diff --git a/static/app/utils/sqlish/formatters/stringAccumulator.ts b/static/app/utils/sqlish/formatters/stringAccumulator.ts deleted file mode 100644 index 63042a105dacde..00000000000000 --- a/static/app/utils/sqlish/formatters/stringAccumulator.ts +++ /dev/null @@ -1,122 +0,0 @@ -export class StringAccumulator { - lines: Line[]; - - constructor() { - this.lines = [new Line()]; - } - - get lastLine(): Line { - return this.lines.at(-1) as Line; - } - - add(token: string) { - if (!token) { - return; - } - - this.lastLine.add(token); - } - - space() { - this.lastLine.add(SPACE); - } - - break() { - const newLine = new Line(); - newLine.indentTo(this.lastLine.indentation); - - this.lines.push(newLine); - } - - indent() { - this.lastLine.indent(); - } - - unindent() { - this.lastLine.unindent(); - } - - indentTo(level = 1) { - this.lastLine.indentTo(level); - } - - toString(maxLineLength: number = DEFAULT_MAX_LINE_LENGTH) { - let output: Line[] = []; - - this.lines.forEach(line => { - if (line.textLength <= maxLineLength) { - output.push(line); - return; - } - - const splitLines: Line[] = [new Line([], line.indentation)]; - let tokenIndex = 0; - - while (tokenIndex < line.tokens.length) { - const incomingToken = line.tokens.at(tokenIndex) as string; - - const totalLength = (splitLines.at(-1) as Line).textLength + incomingToken.length; - - if (totalLength <= maxLineLength) { - splitLines.at(-1)?.add(incomingToken); - } else { - splitLines.push(new Line([incomingToken], line.indentation + 1)); - } - - tokenIndex += 1; - } - - output = [...output, ...splitLines.filter(splitLine => !splitLine.isEmpty)]; - }); - - return output.join(NEWLINE).trim(); - } -} - -const DEFAULT_MAX_LINE_LENGTH = 100; - -class Line { - tokens: string[]; - indentation: number; - - constructor(tokens: string[] = [], indentation = 0) { - this.tokens = tokens; - this.indentation = indentation; - } - - get isEmpty() { - return this.toString().trim() === ''; - } - - get length() { - return this.toString().length; - } - - get textLength() { - return this.toString().trim().length; - } - - add(token: string) { - this.tokens.push(token); - } - - indent() { - this.indentation += 1; - } - - unindent() { - this.indentation -= 1; - } - - indentTo(level: number) { - this.indentation = level; - } - - toString() { - return `${INDENTATION.repeat(this.indentation)}${this.tokens.join('').trimEnd()}`; - } -} - -const SPACE = ' '; -const INDENTATION = ' '; -const NEWLINE = '\n'; diff --git a/static/app/utils/sqlish/sqlish.pegjs b/static/app/utils/sqlish/sqlish.pegjs deleted file mode 100644 index 9c52e0107c7b50..00000000000000 --- a/static/app/utils/sqlish/sqlish.pegjs +++ /dev/null @@ -1,46 +0,0 @@ -Expression - = tokens:Token* - -Token - = LeftParenthesis / RightParenthesis / Whitespace / Keyword / Parameter / CollapsedColumns / GenericToken - -LeftParenthesis - = "(" { return { type: 'LeftParenthesis', content: '(' } } - -RightParenthesis - = ")" { return { type: 'RightParenthesis', content: ')' } } - -Keyword - = Keyword:("ADD"i / "ALL"i / "ALTER"i / "AND"i / "ANY"i / "AS"i / "ASC"i / "BACKUP"i / "BETWEEN"i / "BY"i / "CASE"i / "CHECK"i / "COLUMN"i / "CONSTRAINT"i / "COUNT"i / "CREATE"i / "DATABASE"i / "DEFAULT"i / "DELETE"i / "DESC"i / "DISTINCT"i / "DROP"i / "EXEC"i / "EXISTS"i / "FOREIGN"i / "FROM"i / "FROM"i / "FULL"i / "GROUP"i / "HAVING"i / "INNER"i / "INSERT"i / "JOIN"i / "KEY"i / "LEFT"i / "LIMIT"i / "OFFSET"i / "ON"i / "ORDER"i / "OUTER"i / "RETURNING"i / "RIGHT"i / "SELECT"i / "SELECT"i / "SET"i / "TABLE"i / "UPDATE"i / "VALUES"i / "WHERE"i / JoinKeyword) & (Whitespace / LeftParenthesis / RightParenthesis) { - return { type: 'Keyword', content: Keyword.toUpperCase() } -} - -JoinKeyword - = JoinDirection:JoinDirection? Whitespace? JoinType:JoinType? Whitespace "JOIN"i { - return (JoinDirection || '') + (JoinDirection ? " " : '') + JoinType + " " + "JOIN" -} - -JoinDirection - = "LEFT"i / "RIGHT"i / "FULL"i - -JoinType - = "OUTER"i / "INNER"i - -Parameter - = Parameter:("%s" / ":c" [0-9]) { return { type: 'Parameter', content: Array.isArray(Parameter) ? Parameter.join('') : Parameter } } - -CollapsedColumns - = ".." { return { type: 'CollapsedColumns', content: '..' } } - -Whitespace - = Whitespace:[\n\t\r ]+ { return { type: 'Whitespace', content: Whitespace.join("") } } - -// \u0000-\u001F are C0 Unicode control characters which sometimes sneak into -// strings, especially \u0000 which is Unicode null. I added support for a few -// just to prevent exceptions. -// \u00A0-\uFFFF is the entire Unicode BMP _including_ surrogate pairs and -// unassigned code points, which aren't parse-able naively. A more precise -// approach would be to define all valid Unicode ranges exactly but for -// permissive parsing we don't mind the lack of precision. -GenericToken - = GenericToken:[a-zA-Z0-9\u0000-\u0006\u00A0-\uFFFF"'`_\-.=><:,*;!\[\]?$%|/\\@#&~^+{}]+ { return { type: 'GenericToken', content: GenericToken.join('') } } diff --git a/static/app/utils/sqlish/types.ts b/static/app/utils/sqlish/types.ts deleted file mode 100644 index 6e4d7fd5e19d1f..00000000000000 --- a/static/app/utils/sqlish/types.ts +++ /dev/null @@ -1,11 +0,0 @@ -export interface Token { - type: - | 'LeftParenthesis' - | 'RightParenthesis' - | 'Whitespace' - | 'Keyword' - | 'Parameter' - | 'CollapsedColumns' - | 'GenericToken'; - content?: string | Token | Token[]; -} diff --git a/static/app/views/insights/common/components/fullSpanDescription.tsx b/static/app/views/insights/common/components/fullSpanDescription.tsx index 84c25a2de4c32d..cabc16b4548887 100644 --- a/static/app/views/insights/common/components/fullSpanDescription.tsx +++ b/static/app/views/insights/common/components/fullSpanDescription.tsx @@ -7,7 +7,7 @@ import {ClippedBox} from 'sentry/components/clippedBox'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; import {IconOpen} from 'sentry/icons'; import {t} from 'sentry/locale'; -import {SQLishFormatter} from 'sentry/utils/sqlish/SQLishFormatter'; +import {SQLishFormatter} from 'sentry/utils/sqlish'; import {MutableSearch} from 'sentry/utils/tokenizeSearch'; import {useLocation} from 'sentry/utils/useLocation'; import {useNavigate} from 'sentry/utils/useNavigate'; diff --git a/static/app/views/insights/common/components/spanDescription.tsx b/static/app/views/insights/common/components/spanDescription.tsx index 2212b9bac0a823..afa063e215054a 100644 --- a/static/app/views/insights/common/components/spanDescription.tsx +++ b/static/app/views/insights/common/components/spanDescription.tsx @@ -6,7 +6,7 @@ import {Flex} from '@sentry/scraps/layout'; import {ClippedBox} from 'sentry/components/clippedBox'; import {LoadingIndicator} from 'sentry/components/loadingIndicator'; -import {SQLishFormatter} from 'sentry/utils/sqlish/SQLishFormatter'; +import {SQLishFormatter} from 'sentry/utils/sqlish'; import {MutableSearch} from 'sentry/utils/tokenizeSearch'; import {useLocation} from 'sentry/utils/useLocation'; import {useNavigate} from 'sentry/utils/useNavigate'; diff --git a/static/app/views/insights/common/components/tableCells/spanDescriptionCell.spec.tsx b/static/app/views/insights/common/components/tableCells/spanDescriptionCell.spec.tsx new file mode 100644 index 00000000000000..fac858111be959 --- /dev/null +++ b/static/app/views/insights/common/components/tableCells/spanDescriptionCell.spec.tsx @@ -0,0 +1,41 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; + +import {render, screen} from 'sentry-test/reactTestingLibrary'; + +import {ModuleName} from 'sentry/views/insights/types'; + +import {SpanDescriptionCell} from './spanDescriptionCell'; + +describe('SpanDescriptionCell', () => { + const organization = OrganizationFixture(); + + it('formats SQL descriptions with bold keywords via toSimpleMarkup', () => { + render( + , + {organization} + ); + + // SQLishFormatter.toSimpleMarkup uppercases keywords and wraps them in + const boldElements = document.querySelectorAll('b'); + const boldText = Array.from(boldElements).map(el => el.textContent); + expect(boldText).toEqual(['SELECT', 'FROM', 'WHERE']); + }); + + it('renders raw description for non-DB modules', () => { + render( + , + {organization} + ); + + expect(screen.getByText('https://example.com/api/resource')).toBeInTheDocument(); + }); +}); diff --git a/static/app/views/insights/common/components/tableCells/spanDescriptionCell.tsx b/static/app/views/insights/common/components/tableCells/spanDescriptionCell.tsx index c7dec474f85a96..cc7758bdd15939 100644 --- a/static/app/views/insights/common/components/tableCells/spanDescriptionCell.tsx +++ b/static/app/views/insights/common/components/tableCells/spanDescriptionCell.tsx @@ -3,7 +3,7 @@ import styled from '@emotion/styled'; import {Hovercard} from 'sentry/components/hovercard'; import {t} from 'sentry/locale'; -import {SQLishFormatter} from 'sentry/utils/sqlish/SQLishFormatter'; +import {SQLishFormatter} from 'sentry/utils/sqlish'; import {FullSpanDescription} from 'sentry/views/insights/common/components/fullSpanDescription'; import {SpanGroupDetailsLink} from 'sentry/views/insights/common/components/spanGroupDetailsLink'; import {SupportedDatabaseSystem} from 'sentry/views/insights/database/utils/constants'; diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/span/eapSections/description.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/eapSections/description.tsx index 86ce9511e1d7ca..fc5fa50cdc8c8d 100644 --- a/static/app/views/performance/newTraceDetails/traceDrawer/details/span/eapSections/description.tsx +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/eapSections/description.tsx @@ -16,7 +16,7 @@ import {IconGraph} from 'sentry/icons/iconGraph'; import {t} from 'sentry/locale'; import type {Organization} from 'sentry/types/organization'; import type {Project} from 'sentry/types/project'; -import {SQLishFormatter} from 'sentry/utils/sqlish/SQLishFormatter'; +import {SQLishFormatter} from 'sentry/utils/sqlish'; import {useLocalStorageState} from 'sentry/utils/useLocalStorageState'; import type {TraceItemResponseAttribute} from 'sentry/views/explore/hooks/useTraceItemDetails'; import {ResourceSize} from 'sentry/views/insights/browser/resources/components/resourceSize'; diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/description.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/description.tsx index 4d6395d034ca7d..accc7e6e2a56b0 100644 --- a/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/description.tsx +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/description.tsx @@ -14,7 +14,7 @@ import {IconGraph} from 'sentry/icons/iconGraph'; import {t} from 'sentry/locale'; import type {Organization} from 'sentry/types/organization'; import type {Project} from 'sentry/types/project'; -import {SQLishFormatter} from 'sentry/utils/sqlish/SQLishFormatter'; +import {SQLishFormatter} from 'sentry/utils/sqlish'; import {useLocalStorageState} from 'sentry/utils/useLocalStorageState'; import {ResourceSize} from 'sentry/views/insights/browser/resources/components/resourceSize'; import { diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/generalInfo.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/generalInfo.tsx index 1d115364f2df5a..e56137c1dda3f5 100644 --- a/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/generalInfo.tsx +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/generalInfo.tsx @@ -10,7 +10,7 @@ import {t} from 'sentry/locale'; import type {Organization} from 'sentry/types/organization'; import {getDuration} from 'sentry/utils/duration/getDuration'; import {getDynamicText} from 'sentry/utils/getDynamicText'; -import {SQLishFormatter} from 'sentry/utils/sqlish/SQLishFormatter'; +import {SQLishFormatter} from 'sentry/utils/sqlish'; import {FullSpanDescription} from 'sentry/views/insights/common/components/fullSpanDescription'; import {WiderHovercard} from 'sentry/views/insights/common/components/tableCells/spanDescriptionCell'; import {resolveSpanModule} from 'sentry/views/insights/common/utils/resolveSpanModule'; From 0775ea1a10dfbd114e982ec801434b2d0889e6fa Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Thu, 2 Apr 2026 15:55:29 -0400 Subject: [PATCH 2/3] ref(insights): Use composition instead of inheritance for SQLishFormatter Wrap BaseSQLishFormatter via composition rather than extending it. Remove unused SQLishParser and Token re-exports. Co-Authored-By: Claude --- static/app/utils/sqlish.tsx | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/static/app/utils/sqlish.tsx b/static/app/utils/sqlish.tsx index 7c768e11720643..097f5598b30567 100644 --- a/static/app/utils/sqlish.tsx +++ b/static/app/utils/sqlish.tsx @@ -6,17 +6,22 @@ * `@sentry/sqlish/react` instead. */ import * as Sentry from '@sentry/react'; -import {SQLishFormatter as BaseSQLishFormatter} from '@sentry/sqlish'; +import {SQLishFormatter as BaseSQLishFormatter, SQLishParser} from '@sentry/sqlish'; import {simpleMarkup} from '@sentry/sqlish/react'; -export {SQLishParser} from '@sentry/sqlish'; -export type {Token} from '@sentry/sqlish'; - type StringFormatterOptions = Parameters[1]; -export class SQLishFormatter extends BaseSQLishFormatter { - override toString(sql: string, options?: StringFormatterOptions): string { - return this._withSentry('string', () => super.toString(sql, options), sql); +export class SQLishFormatter { + private formatter: BaseSQLishFormatter; + private parser: SQLishParser; + + constructor() { + this.formatter = new BaseSQLishFormatter(); + this.parser = new SQLishParser(); + } + + toString(sql: string, options?: StringFormatterOptions): string { + return this._withSentry('string', () => this.formatter.toString(sql, options), sql); } toSimpleMarkup(sql: string): React.ReactElement[] { From d7ca21e520e67f308816c5c1a418267293950437 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Tue, 7 Apr 2026 11:16:15 -0400 Subject: [PATCH 3/3] fix(insights): End Sentry span in finally block, improve jest config comment Move sentrySpan.end() to a finally block to prevent span leaks on parse errors. Clarify moduleNameMapper comment explaining why the manual paths are needed (ESM-only exports with no require condition). Co-Authored-By: Claude --- jest.config.ts | 3 ++- static/app/utils/sqlish.tsx | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/jest.config.ts b/jest.config.ts index c82e853ffa6f5d..006dc2c36b2269 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -308,7 +308,8 @@ const config: Config.InitialOptions = { '^echarts/(.*)': '/tests/js/sentry-test/mocks/echartsMock.js', '^zrender/(.*)': '/tests/js/sentry-test/mocks/echartsMock.js', - // @sentry/sqlish uses ESM exports which Jest can't resolve directly + // @sentry/sqlish is ESM-only with `exports` that only define `import` + // conditions. Jest's CJS resolver can't follow them without explicit mapping. '^@sentry/sqlish/react$': '/node_modules/@sentry/sqlish/dist/react.js', '^@sentry/sqlish$': '/node_modules/@sentry/sqlish/dist/index.js', diff --git a/static/app/utils/sqlish.tsx b/static/app/utils/sqlish.tsx index 097f5598b30567..247a949a1643f1 100644 --- a/static/app/utils/sqlish.tsx +++ b/static/app/utils/sqlish.tsx @@ -42,7 +42,6 @@ export class SQLishFormatter { try { const result = fn(); - sentrySpan?.end(); return result; } catch (error: any) { Sentry.withScope(scope => { @@ -63,6 +62,8 @@ export class SQLishFormatter { // If we fail to parse the SQL, return the original string return sql as unknown as T; + } finally { + sentrySpan?.end(); } } }