Browse Source

feat(performance): Updates useIndexedSpans to accept an optional fields array (#66275)

Updates useIndexedSpans to accept an optional fields array to allow
selecting specific fields.
Refactors existing call to use new function signature.
edwardgou-sentry 1 year ago
parent
commit
8594fa780e

+ 13 - 6
static/app/views/starfish/components/spanDescription.tsx

@@ -10,7 +10,8 @@ import {
 } from 'sentry/views/starfish/components/stackTraceMiniFrame';
 import {useFullSpanFromTrace} from 'sentry/views/starfish/queries/useFullSpanFromTrace';
 import {useIndexedSpans} from 'sentry/views/starfish/queries/useIndexedSpans';
-import type {SpanIndexedField, SpanIndexedFieldTypes} from 'sentry/views/starfish/types';
+import type {SpanIndexedFieldTypes} from 'sentry/views/starfish/types';
+import {SpanIndexedField} from 'sentry/views/starfish/types';
 import {SQLishFormatter} from 'sentry/views/starfish/utils/sqlish/SQLishFormatter';
 
 interface Props {
@@ -35,11 +36,17 @@ export function DatabaseSpanDescription({
   groupId,
   preliminaryDescription,
 }: Omit<Props, 'op'>) {
-  const {data: indexedSpans, isFetching: areIndexedSpansLoading} = useIndexedSpans(
-    {'span.group': groupId},
-    [INDEXED_SPAN_SORT],
-    1
-  );
+  // TODO: we're using all SpanIndexedFields here, but maybe we should only use what we need?
+  // Truncate to 20 fields otherwise discover will complain.
+  const fields = Object.values(SpanIndexedField);
+
+  const {data: indexedSpans, isFetching: areIndexedSpansLoading} = useIndexedSpans({
+    filters: {'span.group': groupId},
+    sorts: [INDEXED_SPAN_SORT],
+    limit: 1,
+    fields,
+    referrer: 'api.starfish.span-description',
+  });
   const indexedSpan = indexedSpans?.[0];
 
   // NOTE: We only need this for `span.data`! If this info existed in indexed spans, we could skip it

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

@@ -5,6 +5,8 @@ import {useEventDetails} from 'sentry/views/starfish/queries/useEventDetails';
 import {useIndexedSpans} from 'sentry/views/starfish/queries/useIndexedSpans';
 import {SpanIndexedField} from 'sentry/views/starfish/types';
 
+const DEFAULT_SORT: Sort[] = [{field: 'timestamp', kind: 'desc'}];
+
 // NOTE: Fetching the top one is a bit naive, but works for now. A better
 // approach might be to fetch several at a time, and let the hook consumer
 // decide how to display them
@@ -20,7 +22,18 @@ export function useFullSpanFromTrace(
     filters[SpanIndexedField.SPAN_GROUP] = group;
   }
 
-  const indexedSpansResponse = useIndexedSpans(filters, sorts, 1, enabled);
+  // TODO: we're using all SpanIndexedFields here, but maybe we should only use what we need?
+  // Truncate to 20 fields otherwise discover will complain.
+  const fields = Object.values(SpanIndexedField).slice(0, 20);
+
+  const indexedSpansResponse = useIndexedSpans({
+    filters,
+    sorts: sorts || DEFAULT_SORT,
+    limit: 1,
+    enabled,
+    fields,
+    referrer: 'api.starfish.full-span-from-trace',
+  });
 
   const firstIndexedSpan = indexedSpansResponse.data?.[0];
 

+ 24 - 14
static/app/views/starfish/queries/useIndexedSpans.tsx

@@ -5,25 +5,30 @@ import type {Sort} from 'sentry/utils/discover/fields';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {useLocation} from 'sentry/utils/useLocation';
-import type {SpanIndexedFieldTypes} from 'sentry/views/starfish/types';
-import {SpanIndexedField} from 'sentry/views/starfish/types';
+import type {SpanIndexedField, SpanIndexedFieldTypes} from 'sentry/views/starfish/types';
 import {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';
 
-const DEFAULT_LIMIT = 10;
-
 interface Filters {
   [key: string]: string;
 }
 
-export const useIndexedSpans = (
-  filters: Filters,
-  sorts: Sort[] = [{field: 'timestamp', kind: 'desc'}],
-  limit: number = DEFAULT_LIMIT,
-  enabled: boolean = true,
-  referrer: string = 'use-indexed-spans'
-) => {
+export const useIndexedSpans = ({
+  filters,
+  sorts,
+  limit,
+  enabled = true,
+  referrer,
+  fields,
+}: {
+  fields: SpanIndexedField[];
+  filters: Filters;
+  limit: number;
+  referrer: string;
+  sorts: Sort[];
+  enabled?: boolean;
+}) => {
   const location = useLocation();
-  const eventView = getEventView(filters, location, sorts);
+  const eventView = getEventView(filters, location, fields, sorts);
 
   return useSpansQuery<SpanIndexedFieldTypes[]>({
     eventView,
@@ -34,7 +39,12 @@ export const useIndexedSpans = (
   });
 };
 
-function getEventView(filters: Filters, location: Location, sorts?: Sort[]) {
+function getEventView(
+  filters: Filters,
+  location: Location,
+  fields: SpanIndexedField[],
+  sorts?: Sort[]
+) {
   // TODO: Add a `MutableSearch` constructor that accept a key-value mapping
   const search = new MutableSearch([]);
 
@@ -46,7 +56,7 @@ function getEventView(filters: Filters, location: Location, sorts?: Sort[]) {
     {
       name: '',
       query: search.formatString(),
-      fields: Object.values(SpanIndexedField),
+      fields,
       dataset: DiscoverDatasets.SPANS_INDEXED,
       version: 2,
     },