Browse Source

feat(starfish): Gently highlight SQL in span table (#53782)

We are convinced that links in tables should look like links, so we do
_not_ want to fully syntax highlight these. We _do_ want to _lightly_
highlight them, though! This is a nice little compromise.

## Changes

- Break up the files a bit
- Introduce simple markup formatter
- Bold keywords
- Fix rendering of collapsed columns
- Capitalize keywords
- Fix whitespace between tokens and join commands
- Render descriptions with simple markup
George Gritsouk 1 year ago
parent
commit
fd2ff178a6

+ 25 - 0
static/app/views/starfish/utils/sqlish/SQLishFormatter.spec.tsx

@@ -1,3 +1,7 @@
+import React from 'react';
+
+import {render} from 'sentry-test/reactTestingLibrary';
+
 import {SQLishFormatter} from 'sentry/views/starfish/utils/sqlish/SQLishFormatter';
 
 describe('SQLishFormatter', function () {
@@ -14,4 +18,25 @@ describe('SQLishFormatter', function () {
       ).toEqual('SELECT hello \nFROM users \nORDER BY name DESC \nLIMIT 1;');
     });
   });
+
+  describe('SQLishFormatter.toSimpleMarkup()', () => {
+    const formatter = new SQLishFormatter();
+    const getMarkup = (markup: any): string => {
+      const {container} = render(<React.Fragment>{markup}</React.Fragment>);
+
+      return container.innerHTML;
+    };
+
+    it('Capitalizes keywords', () => {
+      expect(getMarkup(formatter.toSimpleMarkup('select hello'))).toEqual(
+        '<b>SELECT</b><span> </span><span>hello</span>'
+      );
+    });
+
+    it('Wraps every token in a `<span>` element', () => {
+      expect(getMarkup(formatter.toSimpleMarkup('SELECT hello;'))).toEqual(
+        '<b>SELECT</b><span> </span><span>hello;</span>'
+      );
+    });
+  });
 });

+ 31 - 40
static/app/views/starfish/utils/sqlish/SQLishFormatter.tsx

@@ -1,56 +1,47 @@
 import * as Sentry from '@sentry/react';
 
+import {simpleMarkup} from 'sentry/views/starfish/utils/sqlish/formatters/simpleMarkup';
+import {string} from 'sentry/views/starfish/utils/sqlish/formatters/string';
 import {SQLishParser} from 'sentry/views/starfish/utils/sqlish/SQLishParser';
-import type {Token} from 'sentry/views/starfish/utils/sqlish/types';
+
+enum Format {
+  STRING = 'string',
+  SIMPLE_MARKUP = 'simpleMarkup',
+}
+
+const FORMATTERS = {
+  [Format.STRING]: string,
+  [Format.SIMPLE_MARKUP]: simpleMarkup,
+};
 
 export class SQLishFormatter {
   parser: SQLishParser;
-  tokens?: Token[];
 
   constructor() {
     this.parser = new SQLishParser();
   }
 
-  toString(sql: string): string;
-  toString(tokens: Token[]): string;
-  toString(input: string | Token[]): string {
-    if (typeof input === 'string') {
-      try {
-        const tokens = this.parser.parse(input);
-        return this.toString(tokens);
-      } catch (error) {
-        Sentry.captureException(error);
-        // If we fail to parse the SQL, return the original string, so there is always output
-        return input;
-      }
-    }
+  toString(sql: string) {
+    return this.toFormat(sql, Format.STRING);
+  }
+
+  toSimpleMarkup(sql: string) {
+    return this.toFormat(sql, Format.SIMPLE_MARKUP);
+  }
 
-    const tokens = input;
-    let ret = '';
-
-    function contentize(content: Token): void {
-      if (content.type === 'Keyword') {
-        ret += '\n';
-      }
-
-      if (Array.isArray(content.content)) {
-        content.content.forEach(contentize);
-        return;
-      }
-
-      if (typeof content.content === 'string') {
-        if (content.type === 'Whitespace') {
-          ret += ' ';
-        } else {
-          ret += content.content;
-        }
-        return;
-      }
-
-      return;
+  toFormat(sql: string, format: Format.STRING): string;
+  toFormat(sql: string, format: Format.SIMPLE_MARKUP): React.ReactElement[];
+  toFormat(sql: string, format: Format) {
+    let tokens;
+
+    try {
+      tokens = this.parser.parse(sql);
+    } catch (error) {
+      Sentry.captureException(error);
+      // If we fail to parse the SQL, return the original string
+      return sql;
     }
 
-    tokens.forEach(contentize);
-    return ret.trim();
+    return FORMATTERS[format](tokens);
   }
 }

+ 36 - 0
static/app/views/starfish/utils/sqlish/SQLishParser.spec.tsx

@@ -25,4 +25,40 @@ describe('SQLishParser', function () {
       }).not.toThrow();
     });
   });
+
+  describe('SQLishParser.parse', () => {
+    const parser = new SQLishParser();
+    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('table1 INNER JOIN table2')).toEqual([
+        {
+          type: 'GenericToken',
+          content: 'table1',
+        },
+        {type: 'Whitespace', content: ' '},
+        {type: 'Keyword', content: 'INNER JOIN'},
+        {type: 'Whitespace', content: ' '},
+        {
+          type: 'GenericToken',
+          content: 'table2',
+        },
+      ]);
+    });
+  });
 });

+ 33 - 0
static/app/views/starfish/utils/sqlish/formatters/simpleMarkup.tsx

@@ -0,0 +1,33 @@
+import type {Token} from 'sentry/views/starfish/utils/sqlish/types';
+
+export function simpleMarkup(tokens: Token[]): React.ReactElement[] {
+  const accumulator: React.ReactElement[] = [];
+
+  function contentize(content: Token): void {
+    if (Array.isArray(content.content)) {
+      content.content.forEach(contentize);
+      return;
+    }
+
+    if (typeof content.content === 'string') {
+      if (content.type === 'Keyword') {
+        accumulator.push(
+          <b key={toKey(content.content)}>{content.content.toUpperCase()}</b>
+        );
+      } else if (content.type === 'Whitespace') {
+        accumulator.push(<span key={toKey(content.content)}> </span>);
+      } else {
+        accumulator.push(<span key={toKey(content.content)}>{content.content}</span>);
+      }
+    }
+
+    return;
+  }
+
+  tokens.forEach(contentize);
+  return accumulator;
+}
+
+function toKey(content: string): string {
+  return content.toLowerCase();
+}

+ 32 - 0
static/app/views/starfish/utils/sqlish/formatters/string.ts

@@ -0,0 +1,32 @@
+import type {Token} from 'sentry/views/starfish/utils/sqlish/types';
+
+export function string(tokens: Token[]): string {
+  let accumulator = '';
+
+  function contentize(content: Token): void {
+    if (Array.isArray(content.content)) {
+      content.content.forEach(contentize);
+      return;
+    }
+
+    if (typeof content.content === 'string') {
+      if (content.type === 'Keyword') {
+        // Break up the string on newlines
+        accumulator += '\n';
+        accumulator += content.content;
+      } else if (content.type === 'Whitespace') {
+        // Convert all whitespace to single spaces
+        accumulator += ' ';
+      } else {
+        accumulator += content.content;
+      }
+
+      return;
+    }
+
+    return;
+  }
+
+  tokens.forEach(contentize);
+  return accumulator.trim();
+}

+ 3 - 3
static/app/views/starfish/utils/sqlish/sqlish.pegjs

@@ -2,7 +2,7 @@ Expression
    = tokens:Token*
 
 Token
-   = Keyword / Parameter / CollapsedColumns / Whitespace / GenericToken
+   = Whitespace / Keyword / Parameter / CollapsedColumns / GenericToken
 
 Keyword
   = Keyword:("SELECT"i / "INSERT"i / "DELETE"i / "FROM"i / "ON"i / "WHERE"i / "AND"i / "ORDER BY"i / "LIMIT"i / "GROUP BY"i / "OFFSET"i / JoinKeyword) {
@@ -10,7 +10,7 @@ Keyword
 }
 
 JoinKeyword
-  = JoinDirection:JoinDirection? Whitespace JoinType:JoinType? Whitespace "JOIN"i {
+  = JoinDirection:JoinDirection? Whitespace? JoinType:JoinType? Whitespace "JOIN"i {
   return (JoinDirection || '') + (JoinDirection ? " " : '') + JoinType + " " + "JOIN"
 }
 
@@ -24,7 +24,7 @@ Parameter
   = Parameter:("%s" / ":c" [0-9]) { return { type: 'Parameter', content: Array.isArray(Parameter) ? Parameter.join('') : Parameter } }
 
 CollapsedColumns
-  = ".." { return { type: 'CollapsedColumns' } }
+  = ".." { return { type: 'CollapsedColumns', content: '..' } }
 
 Whitespace
   = Whitespace:[\n\t ]+ { return { type: 'Whitespace', content: Whitespace.join("") } }

+ 21 - 3
static/app/views/starfish/views/spans/spansTable.tsx

@@ -27,9 +27,12 @@ import {
   StarfishFunctions,
 } from 'sentry/views/starfish/types';
 import {extractRoute} from 'sentry/views/starfish/utils/extractRoute';
+import {SQLishFormatter} from 'sentry/views/starfish/utils/sqlish/SQLishFormatter';
 import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 import {DataTitles, getThroughputTitle} from 'sentry/views/starfish/views/spans/types';
 
+const formatter = new SQLishFormatter();
+
 type Row = {
   'avg(span.self_time)': number;
   'http_error_count()': number;
@@ -126,7 +129,16 @@ export default function SpansTable({
           grid={{
             renderHeadCell: column => renderHeadCell({column, sort, location}),
             renderBodyCell: (column, row) =>
-              renderBodyCell(column, row, meta, location, organization, endpoint, method),
+              renderBodyCell(
+                column,
+                row,
+                moduleName,
+                meta,
+                location,
+                organization,
+                endpoint,
+                method
+              ),
           }}
           location={location}
         />
@@ -139,6 +151,7 @@ export default function SpansTable({
 function renderBodyCell(
   column: Column,
   row: Row,
+  moduleName: ModuleName,
   meta: EventsMetaType | undefined,
   location: Location,
   organization: Organization,
@@ -161,6 +174,11 @@ function renderBodyCell(
       );
     }
 
+    const description =
+      moduleName === ModuleName.DB
+        ? formatter.toSimpleMarkup(row[SPAN_DESCRIPTION])
+        : row[SPAN_DESCRIPTION];
+
     return (
       <OverflowEllipsisTextContainer>
         {row[SPAN_GROUP] ? (
@@ -169,10 +187,10 @@ function renderBodyCell(
               queryString ? `?${qs.stringify(queryString)}` : ''
             }`}
           >
-            {row[SPAN_DESCRIPTION] || '<null>'}
+            {description || '<null>'}
           </Link>
         ) : (
-          row[SPAN_DESCRIPTION] || '<null>'
+          description || '<null>'
         )}
       </OverflowEllipsisTextContainer>
     );