Browse Source

ref(perf): Accept `MutableQuery` in `useIndexedSpans` (#69043)

Follow-up to https://github.com/getsentry/sentry/pull/67080

Adds the same interface to `useIndexedSpans` as `useSpanMetrics` and
`useSpanMetricsSeries`. Luckily, it was already pretty close! Not a lot
of places use this, so it's a small change.

- accepts a parameter called `search` which is a `MutableSearch` instead
of a filters object (makes it possible to set weird compound queries,
and so on)
- obeys `enabled`
- typing for `fields`
George Gritsouk 10 months ago
parent
commit
6e807492df

+ 1 - 0
static/app/utils/tokenizeSearch.spec.tsx

@@ -8,6 +8,7 @@ describe('utils/tokenizeSearch', function () {
       [{transaction: '/index', 'span.domain': undefined}, 'transaction:/index'],
       [{'span.domain': '*hello*'}, 'span.domain:*hello*'],
       [{'span.description': '*hello*'}, 'span.description:*hello*'],
+      [{'span.duration': ['>0', '<100']}, 'span.duration:>0 span.duration:<100'],
       [{transaction: '(empty)'}, '!has:transaction'],
     ])('converts %s to search string', (query, result) => {
       expect(MutableSearch.fromQueryObject(query).formatString()).toEqual(result);

+ 3 - 1
static/app/utils/tokenizeSearch.tsx

@@ -51,7 +51,7 @@ export class MutableSearch {
    * @returns {MutableSearch}
    */
   static fromQueryObject(params: {
-    [key: string]: string | number | undefined;
+    [key: string]: string[] | string | number | undefined;
   }): MutableSearch {
     const query = new MutableSearch('');
 
@@ -62,6 +62,8 @@ export class MutableSearch {
 
       if (value === EMPTY_OPTION_VALUE) {
         query.addFilterValue('!has', key);
+      } else if (Array.isArray(value)) {
+        query.addFilterValues(key, value, !ALLOWED_WILDCARD_FIELDS.includes(key));
       } else {
         query.addFilterValue(
           key,

+ 5 - 7
static/app/views/performance/browser/webVitals/utils/queries/useInpSpanSamplesWebVitalsQuery.tsx

@@ -1,13 +1,11 @@
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {
   DEFAULT_INDEXED_INTERACTION_SORT,
   type InteractionSpanSampleRowWithScore,
   SORTABLE_INDEXED_INTERACTION_FIELDS,
 } from 'sentry/views/performance/browser/webVitals/utils/types';
 import {useWebVitalsSort} from 'sentry/views/performance/browser/webVitals/utils/useWebVitalsSort';
-import {
-  type Filters,
-  useIndexedSpans,
-} from 'sentry/views/starfish/queries/useIndexedSpans';
+import {useIndexedSpans} from 'sentry/views/starfish/queries/useIndexedSpans';
 import {SpanIndexedField} from 'sentry/views/starfish/types';
 
 export function useInpSpanSamplesWebVitalsQuery({
@@ -19,7 +17,7 @@ export function useInpSpanSamplesWebVitalsQuery({
 }: {
   limit: number;
   enabled?: boolean;
-  filters?: Filters;
+  filters?: {[key: string]: string[] | string | number | undefined};
   sortName?: string;
   transaction?: string;
 }) {
@@ -30,14 +28,14 @@ export function useInpSpanSamplesWebVitalsQuery({
     sortableFields: filteredSortableFields as unknown as string[],
   });
   const {data, isLoading, ...rest} = useIndexedSpans({
-    filters: {
+    search: MutableSearch.fromQueryObject({
       'span.op': 'ui.interaction.click',
       'measurements.score.weight.inp': '>0',
       ...(transaction !== undefined
         ? {[SpanIndexedField.ORIGIN_TRANSACTION]: transaction}
         : {}),
       ...filters,
-    },
+    }),
     sorts: [sort],
     fields: [
       SpanIndexedField.INP,

+ 1 - 1
static/app/views/performance/http/httpSamplesPanel.tsx

@@ -203,7 +203,7 @@ export function HTTPSamplesPanel() {
     error: responseCodeSamplesDataError,
     refetch: refetchResponseCodeSpanSamples,
   } = useIndexedSpans({
-    filters,
+    search: MutableSearch.fromQueryObject(filters),
     fields: [
       SpanIndexedField.PROJECT,
       SpanIndexedField.TRACE,

+ 2 - 1
static/app/views/starfish/components/spanDescription.tsx

@@ -4,6 +4,7 @@ import styled from '@emotion/styled';
 import {CodeSnippet} from 'sentry/components/codeSnippet';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import {space} from 'sentry/styles/space';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {
   MissingFrame,
   StackTraceMiniFrame,
@@ -37,7 +38,7 @@ export function DatabaseSpanDescription({
   preliminaryDescription,
 }: Omit<Props, 'op'>) {
   const {data: indexedSpans, isFetching: areIndexedSpansLoading} = useIndexedSpans({
-    filters: {'span.group': groupId},
+    search: MutableSearch.fromQueryObject({'span.group': groupId}),
     sorts: [INDEXED_SPAN_SORT],
     limit: 1,
     fields: [

+ 2 - 1
static/app/views/starfish/queries/useFullSpanFromTrace.tsx

@@ -1,6 +1,7 @@
 import type {Entry, EntrySpans} from 'sentry/types';
 import {EntryType} from 'sentry/types/event';
 import type {Sort} from 'sentry/utils/discover/fields';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {useEventDetails} from 'sentry/views/starfish/queries/useEventDetails';
 import {useIndexedSpans} from 'sentry/views/starfish/queries/useIndexedSpans';
 import {SpanIndexedField} from 'sentry/views/starfish/types';
@@ -27,7 +28,7 @@ export function useFullSpanFromTrace(
   const fields = Object.values(SpanIndexedField).slice(0, 20);
 
   const indexedSpansResponse = useIndexedSpans({
-    filters,
+    search: MutableSearch.fromQueryObject(filters),
     sorts: sorts || DEFAULT_SORT,
     limit: 1,
     enabled,

+ 159 - 0
static/app/views/starfish/queries/useIndexedSpans.spec.tsx

@@ -0,0 +1,159 @@
+import type {ReactNode} from 'react';
+import {LocationFixture} from 'sentry-fixture/locationFixture';
+import {OrganizationFixture} from 'sentry-fixture/organization';
+
+import {makeTestQueryClient} from 'sentry-test/queryClient';
+import {renderHook, waitFor} from 'sentry-test/reactTestingLibrary';
+
+import {QueryClientProvider} from 'sentry/utils/queryClient';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
+import {useLocation} from 'sentry/utils/useLocation';
+import useOrganization from 'sentry/utils/useOrganization';
+import usePageFilters from 'sentry/utils/usePageFilters';
+import {useIndexedSpans} from 'sentry/views/starfish/queries/useIndexedSpans';
+import type {IndexedProperty} from 'sentry/views/starfish/types';
+
+jest.mock('sentry/utils/useLocation');
+jest.mock('sentry/utils/usePageFilters');
+jest.mock('sentry/utils/useOrganization');
+
+function Wrapper({children}: {children?: ReactNode}) {
+  return (
+    <QueryClientProvider client={makeTestQueryClient()}>{children}</QueryClientProvider>
+  );
+}
+
+describe('useIndexedSpans', () => {
+  const organization = OrganizationFixture();
+
+  jest.mocked(usePageFilters).mockReturnValue({
+    isReady: true,
+    desyncedFilters: new Set(),
+    pinnedFilters: new Set(),
+    shouldPersist: true,
+    selection: {
+      datetime: {
+        period: '10d',
+        start: null,
+        end: null,
+        utc: false,
+      },
+      environments: [],
+      projects: [],
+    },
+  });
+
+  jest.mocked(useLocation).mockReturnValue(
+    LocationFixture({
+      query: {statsPeriod: '10d'},
+    })
+  );
+
+  jest.mocked(useOrganization).mockReturnValue(organization);
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('respects the `enabled` prop', () => {
+    const eventsRequest = MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events/`,
+      method: 'GET',
+      body: {data: []},
+    });
+
+    const {result} = renderHook(
+      ({fields, enabled}) => useIndexedSpans({fields, enabled}),
+      {
+        wrapper: Wrapper,
+        initialProps: {
+          fields: ['span.description'] as IndexedProperty[],
+          enabled: false,
+        },
+      }
+    );
+
+    expect(result.current.isFetching).toEqual(false);
+    expect(eventsRequest).not.toHaveBeenCalled();
+  });
+
+  it('queries for current selection', async () => {
+    const eventsRequest = MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events/`,
+      method: 'GET',
+      body: {
+        data: [
+          {
+            'span.group': '221aa7ebd216',
+            'span.op': 'db',
+            'span.description': 'SELECT * FROM users;',
+          },
+        ],
+        meta: {
+          fields: {
+            'span.description': 'string',
+            'span.op': 'string',
+            'span.group': 'string',
+          },
+        },
+      },
+    });
+
+    const {result} = renderHook(
+      ({filters, fields, sorts, limit, cursor, referrer}) =>
+        useIndexedSpans({
+          search: MutableSearch.fromQueryObject(filters),
+          fields,
+          sorts,
+          limit,
+          cursor,
+          referrer,
+        }),
+      {
+        wrapper: Wrapper,
+        initialProps: {
+          filters: {
+            'span.group': '221aa7ebd216',
+            'measurements.inp': ['<50', '>0'],
+            transaction: '/api/details',
+            release: '0.0.1',
+          },
+          fields: ['span.op', 'span.group', 'span.description'] as IndexedProperty[],
+          sorts: [{field: 'span.group', kind: 'desc' as const}],
+          limit: 10,
+          referrer: 'api-spec',
+          cursor: undefined,
+        },
+      }
+    );
+
+    expect(result.current.isLoading).toEqual(true);
+
+    expect(eventsRequest).toHaveBeenCalledWith(
+      '/organizations/org-slug/events/',
+      expect.objectContaining({
+        method: 'GET',
+        query: {
+          dataset: 'spansIndexed',
+          environment: [],
+          field: ['span.op', 'span.group', 'span.description'],
+          per_page: 10,
+          project: [],
+          sort: '-span.group',
+          query: `span.group:221aa7ebd216 measurements.inp:<50 measurements.inp:>0 transaction:/api/details release:0.0.1`,
+          referrer: 'api-spec',
+          statsPeriod: '10d',
+        },
+      })
+    );
+
+    await waitFor(() => expect(result.current.isLoading).toEqual(false));
+    expect(result.current.data).toEqual([
+      {
+        'span.group': '221aa7ebd216',
+        'span.op': 'db',
+        'span.description': 'SELECT * FROM users;',
+      },
+    ]);
+  });
+});

+ 36 - 53
static/app/views/starfish/queries/useIndexedSpans.tsx

@@ -1,40 +1,38 @@
-import type {Location} from 'history';
-
+import type {PageFilters} from 'sentry/types';
 import EventView from 'sentry/utils/discover/eventView';
 import type {Sort} from 'sentry/utils/discover/fields';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
-import {
-  ALLOWED_WILDCARD_FIELDS,
-  EMPTY_OPTION_VALUE,
-  MutableSearch,
-} from 'sentry/utils/tokenizeSearch';
-import {useLocation} from 'sentry/utils/useLocation';
-import type {SpanIndexedField, SpanIndexedFieldTypes} from 'sentry/views/starfish/types';
+import type {MutableSearch} from 'sentry/utils/tokenizeSearch';
+import usePageFilters from 'sentry/utils/usePageFilters';
+import type {IndexedProperty, SpanIndexedFieldTypes} from 'sentry/views/starfish/types';
 import {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';
 
-export interface Filters {
-  [key: string]: string | string[];
-}
-
-export const useIndexedSpans = ({
-  filters,
-  sorts,
-  cursor,
-  limit,
-  enabled = true,
-  referrer,
-  fields,
-}: {
-  fields: SpanIndexedField[];
-  filters: Filters;
-  limit: number;
-  referrer: string;
-  sorts: Sort[];
+interface UseIndexedSpansOptions<Fields> {
   cursor?: string;
   enabled?: boolean;
-}) => {
-  const location = useLocation();
-  const eventView = getEventView(filters, location, fields, sorts);
+  fields?: Fields;
+  limit?: number;
+  referrer?: string;
+  search?: MutableSearch;
+  sorts?: Sort[];
+}
+
+export const useIndexedSpans = <Fields extends IndexedProperty[]>(
+  options: UseIndexedSpansOptions<Fields> = {}
+) => {
+  const {
+    fields = [],
+    search = undefined,
+    sorts = [],
+    limit,
+    cursor,
+    referrer,
+    enabled,
+  } = options;
+
+  const pageFilters = usePageFilters();
+
+  const eventView = getEventView(search, fields, sorts, pageFilters.selection);
 
   return useSpansQuery<SpanIndexedFieldTypes[]>({
     eventView,
@@ -47,38 +45,23 @@ export const useIndexedSpans = ({
 };
 
 function getEventView(
-  filters: Filters,
-  location: Location,
-  fields: SpanIndexedField[],
-  sorts?: Sort[]
+  search: MutableSearch | undefined,
+  fields: string[] = [],
+  sorts: Sort[] = [],
+  pageFilters: PageFilters
 ) {
-  // TODO: Use `MutableSearch.fromQueryObject` instead
-  const search = new MutableSearch([]);
-
-  for (const filterName in filters) {
-    const shouldEscape = !ALLOWED_WILDCARD_FIELDS.includes(filterName);
-    const filter = filters[filterName];
-    if (filter === EMPTY_OPTION_VALUE) {
-      search.addStringFilter(`!has:${filterName}`);
-    } else if (Array.isArray(filter)) {
-      search.addFilterValues(filterName, filter, shouldEscape);
-    } else {
-      search.addFilterValue(filterName, filter, shouldEscape);
-    }
-  }
-
-  const eventView = EventView.fromNewQueryWithLocation(
+  const eventView = EventView.fromNewQueryWithPageFilters(
     {
       name: '',
-      query: search.formatString(),
+      query: search?.formatString() ?? '',
       fields,
       dataset: DiscoverDatasets.SPANS_INDEXED,
       version: 2,
     },
-    location
+    pageFilters
   );
 
-  if (sorts) {
+  if (sorts.length > 0) {
     eventView.sorts = sorts;
   }
 

+ 1 - 0
static/app/views/starfish/types.tsx

@@ -174,6 +174,7 @@ export type IndexedResponse = {
   [SpanIndexedField.TRANSACTION_METHOD]: string;
   [SpanIndexedField.TRANSACTION_OP]: string;
   [SpanIndexedField.SPAN_DOMAIN]: string[];
+  [SpanIndexedField.RAW_DOMAIN]: string;
   [SpanIndexedField.TIMESTAMP]: string;
   [SpanIndexedField.PROJECT]: string;
   [SpanIndexedField.PROJECT_ID]: number;