Browse Source

feat(trace): search using span. or transaction. entity prefix (#71152)

Enables search by narrowing down on the entity types via span. and
transaction. prefixes (errors, issues and autogroups still need to be
implemented). Previously if two objects would had the same key, the
value would be resolved from either of them and both were subject to
match. With this change, searching narrows down on the entity by the
prefix.

There is a slight annoyance with txns because they contain keys like
`transaction.op` as opposed to spans which only contain a key like `op`,
which is why we need to perform a direct lookup first, and only then
look at the entity.
Jonas 9 months ago
parent
commit
516c5a5ad3

+ 4 - 5
static/app/components/searchSyntax/evaluator.tsx

@@ -99,9 +99,7 @@ function processTokenResults(tokens: TokenResult<Token>[]): ProcessedTokenResult
 }
 
 function isBooleanOR(token: ProcessedTokenResult): boolean {
-  return (
-    token && token.type === Token.LOGIC_BOOLEAN && token.value === BooleanOperator.OR
-  );
+  return token?.type === Token.LOGIC_BOOLEAN && token?.value === BooleanOperator.OR;
 }
 
 // https://en.wikipedia.org/wiki/Shunting_yard_algorithm
@@ -109,20 +107,21 @@ export function toPostFix(tokens: TokenResult<Token>[]): ProcessedTokenResult[]
   const processed = processTokenResults(tokens);
 
   const result: ProcessedTokenResult[] = [];
-  // Operator stack
   const stack: ProcessedTokenResult[] = [];
 
   for (const token of processed) {
     switch (token.type) {
       case Token.LOGIC_BOOLEAN: {
         while (
+          // Establishes higher precedence for OR operators.
+          // Whenever the current stack top operator is an OR,
           stack.length > 0 &&
           token.value === BooleanOperator.AND &&
           stack[stack.length - 1].type === Token.LOGIC_BOOLEAN &&
           stack[stack.length - 1].type !== 'L_PAREN' &&
           isBooleanOR(stack[stack.length - 1])
         ) {
-          result.push(stack.pop() as ProcessedTokenResult);
+          result.push(stack.pop()!);
         }
         stack.push(token);
         break;

+ 24 - 1
static/app/views/performance/newTraceDetails/traceSearch/traceSearchEvaluator.spec.tsx

@@ -141,8 +141,9 @@ describe('TraceSearchEvaluator', () => {
     ]);
 
     const cb = jest.fn();
-    // (transaction.op:operation AND transaction:something) OR transaction.op:other
+    // @TODO check if this makes sense with some users. We might only want to do this only if we have a set of parens.
     search(
+      // (transaction.op:operation OR transaction.op:other) AND transaction:something
       'transaction.op:operation AND transaction:something OR transaction.op:other',
       tree,
       cb
@@ -170,6 +171,17 @@ describe('TraceSearchEvaluator', () => {
       expect(cb.mock.calls[0][0][2]).toBe(null);
     });
 
+    it('text filter with prefix', async () => {
+      const tree = makeTree([makeTransaction({transaction: 'operation'})]);
+
+      const cb = jest.fn();
+      search('transaction.transaction:operation', tree, cb);
+      await waitFor(() => expect(cb).toHaveBeenCalled());
+      expect(cb.mock.calls[0][0][1].size).toBe(1);
+      expect(cb.mock.calls[0][0][0]).toEqual([{index: 0, value: tree.list[0]}]);
+      expect(cb.mock.calls[0][0][2]).toBe(null);
+    });
+
     it('transaction.duration (milliseconds)', async () => {
       const tree = makeTree([
         makeTransaction({'transaction.duration': 1000}),
@@ -228,6 +240,17 @@ describe('TraceSearchEvaluator', () => {
       expect(cb.mock.calls[0][0][2]).toBe(null);
     });
 
+    it('text filter with prefix', async () => {
+      const tree = makeTree([makeSpan({op: 'db'}), makeSpan({op: 'http'})]);
+
+      const cb = jest.fn();
+      search('span.op:db', tree, cb);
+      await waitFor(() => expect(cb).toHaveBeenCalled());
+      expect(cb.mock.calls[0][0][1].size).toBe(1);
+      expect(cb.mock.calls[0][0][0]).toEqual([{index: 0, value: tree.list[0]}]);
+      expect(cb.mock.calls[0][0][2]).toBe(null);
+    });
+
     it('span.duration (milliseconds)', async () => {
       const tree = makeTree([
         makeSpan({start_timestamp: 0, timestamp: 1}),

+ 24 - 0
static/app/views/performance/newTraceDetails/traceSearch/traceSearchEvaluator.tsx

@@ -472,10 +472,34 @@ function resolveValueFromKey(
     }
 
     if (key !== null) {
+      // If the value can be accessed directly, do so,
+      // else check if the key is an entity key, sanitize it and try direct access again.
+      // @TODO support deep nested keys with dot notation
+
       // Check for direct key access.
       if (value[key] !== undefined) {
         return value[key];
       }
+
+      // @TODO perf optimization opportunity
+      // Entity check should be preprocessed per token, not once per token per node we are evaluating, however since
+      // we are searching <10k nodes in p99 percent of the time and the search is non blocking, we are likely fine
+      // and can be optimized later.
+      const [maybeEntity, ...rest] = key.split('.');
+      switch (maybeEntity) {
+        case 'span':
+          if (isSpanNode(node)) {
+            return value[rest.join('.')];
+          }
+          break;
+        case 'transaction':
+          if (isTransactionNode(node)) {
+            return value[rest.join('.')];
+          }
+          break;
+        default:
+          break;
+      }
     }
 
     return key ? value[key] ?? null : null;

+ 65 - 32
static/app/views/performance/newTraceDetails/traceSearch/traceTokenConverter.tsx

@@ -5,8 +5,15 @@ import {
 } from 'sentry/components/searchSyntax/parser';
 import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree';
 
+// Span keys
+type TransactionPrefix = 'Transaction';
+// The keys can be prefixed by the entity type they belong to, this ensures that
+// conflicting keys on different entities are resolved to the correct entity.
+type TransactionKey =
+  | `${TransactionPrefix}.${keyof TraceTree.Transaction}`
+  | keyof TraceTree.Transaction;
 // Transaction keys
-const TRANSACTION_TEXT_KEYS: (keyof TraceTree.Transaction)[] = [
+const TRANSACTION_TEXT_KEYS: TransactionKey[] = [
   'event_id',
   'project_slug',
   'parent_event_id',
@@ -16,31 +23,35 @@ const TRANSACTION_TEXT_KEYS: (keyof TraceTree.Transaction)[] = [
   'transaction.op',
   'transaction.status',
 ];
-const TRANSACTION_NUMERIC_KEYS: (keyof TraceTree.Transaction)[] = [
+const TRANSACTION_NUMERIC_KEYS: TransactionKey[] = [
   'project_id',
   'start_timestamp',
   'timestamp',
 ];
-const TRANSACTION_DURATION_KEYS: (keyof TraceTree.Transaction)[] = [
-  'transaction.duration',
-  // The keys below are not real keys returned by the API, but are instead
-  // mapped by the frontend to the correct keys for convenience and UX reasons
+
+const TRANSACTION_DURATION_KEYS: TransactionKey[] = ['transaction.duration'];
+
+const TRANSACTION_DURATION_SYNTHETIC_KEYS: TransactionKey[] = [
   // @ts-expect-error
-  'transaction.total_time',
-  // @TODO for consistency with spans, this should be implemented
-  // 'transaction.self_time'
+  'duration',
+  // @ts-expect-error
+  'total_time',
 ];
 
 // @TODO the current date parsing does not support timestamps, so we
 // exclude these keys for now and parse them as numeric keys
-const TRANSACTION_DATE_KEYS: (keyof TraceTree.Transaction)[] = [
+const TRANSACTION_DATE_KEYS: TransactionKey[] = [
   //   'start_timestamp',
   //   'timestamp',
 ];
-const TRANSACTION_BOOLEAN_KEYS: (keyof TraceTree.Transaction)[] = [];
+const TRANSACTION_BOOLEAN_KEYS: TransactionKey[] = [];
 
 // Span keys
-const SPAN_TEXT_KEYS: (keyof TraceTree.Span)[] = [
+type SpanPrefix = 'span';
+// The keys can be prefixed by the entity type they belong to, this ensures that
+// conflicting keys on different entities are resolved to the correct entity.
+type SpanKey = `${SpanPrefix}.${keyof TraceTree.Span}` | keyof TraceTree.Span;
+const SPAN_TEXT_KEYS: SpanKey[] = [
   'hash',
   'description',
   'op',
@@ -50,36 +61,58 @@ const SPAN_TEXT_KEYS: (keyof TraceTree.Span)[] = [
   'trace_id',
   'status',
 ];
-const SPAN_NUMERIC_KEYS: (keyof TraceTree.Span)[] = ['timestamp', 'start_timestamp'];
-const SPAN_DURATION_KEYS: (keyof TraceTree.Span)[] = [
-  // @TODO create aliases for self_time total_time and duration.
-  'exclusive_time',
-  // The keys below are not real keys returned by the API, but are instead
-  // mapped by the frontend to the correct keys for convenience and UX reasons
-  // @ts-expect-error
-  'span.duration',
+
+const SPAN_NUMERIC_KEYS: SpanKey[] = ['timestamp', 'start_timestamp'];
+
+const SPAN_DURATION_KEYS: SpanKey[] = ['exclusive_time'];
+
+// The keys below are not real keys returned by the API, but are instead
+// mapped by the frontend to the correct keys for convenience and UX reasons
+const SPAN_DURATION_SYNTHETIC_KEYS: SpanKey[] = [
   // @ts-expect-error
-  'span.total_time',
+  'duration',
   // @ts-expect-error
-  'span.self_time',
+  'total_time',
   // @ts-expect-error
-  'span.exclusive_time',
+  'self_time',
 ];
 
 // @TODO the current date parsing does not support timestamps, so we
 // exclude these keys for now and parse them as numeric keys
-const SPAN_DATE_KEYS: (keyof TraceTree.Span)[] = [
+const SPAN_DATE_KEYS: SpanKey[] = [
   // 'timestamp', 'start_timestamp'
 ];
-const SPAN_BOOLEAN_KEYS: (keyof TraceTree.Span)[] = ['same_process_as_parent'];
+const SPAN_BOOLEAN_KEYS: SpanKey[] = ['same_process_as_parent'];
 
-// @TODO Issue keys
-
-const TEXT_KEYS = new Set([...TRANSACTION_TEXT_KEYS, ...SPAN_TEXT_KEYS]);
-const NUMERIC_KEYS = new Set([...TRANSACTION_NUMERIC_KEYS, ...SPAN_NUMERIC_KEYS]);
-const DURATION_KEYS = new Set([...TRANSACTION_DURATION_KEYS, ...SPAN_DURATION_KEYS]);
-const DATE_KEYS = new Set([...TRANSACTION_DATE_KEYS, ...SPAN_DATE_KEYS]);
-const BOOLEAN_KEYS = new Set([...TRANSACTION_BOOLEAN_KEYS, ...SPAN_BOOLEAN_KEYS]);
+function withPrefixedPermutation(
+  prefix: 'span' | 'transaction',
+  keys: string[]
+): string[] {
+  return [...keys, ...keys.map(key => `${prefix}.${key}`)];
+}
+// @TODO Add issue keys
+const TEXT_KEYS = new Set([
+  ...withPrefixedPermutation('transaction', TRANSACTION_TEXT_KEYS),
+  ...withPrefixedPermutation('span', SPAN_TEXT_KEYS),
+]);
+const NUMERIC_KEYS = new Set([
+  ...withPrefixedPermutation('transaction', TRANSACTION_NUMERIC_KEYS),
+  ...withPrefixedPermutation('span', SPAN_NUMERIC_KEYS),
+]);
+const DURATION_KEYS = new Set([
+  ...withPrefixedPermutation('transaction', TRANSACTION_DURATION_KEYS),
+  ...withPrefixedPermutation('transaction', TRANSACTION_DURATION_SYNTHETIC_KEYS),
+  ...withPrefixedPermutation('span', SPAN_DURATION_KEYS),
+  ...withPrefixedPermutation('span', SPAN_DURATION_SYNTHETIC_KEYS),
+]);
+const DATE_KEYS = new Set([
+  ...withPrefixedPermutation('transaction', TRANSACTION_DATE_KEYS),
+  ...withPrefixedPermutation('span', SPAN_DATE_KEYS),
+]);
+const BOOLEAN_KEYS = new Set([
+  ...withPrefixedPermutation('transaction', TRANSACTION_BOOLEAN_KEYS),
+  ...withPrefixedPermutation('span', SPAN_BOOLEAN_KEYS),
+]);
 
 export const TRACE_SEARCH_CONFIG: SearchConfig = {
   ...defaultConfig,