Browse Source

fix(perf): Improve `useSpanTransactionMetrics` (#57499)

Fixes a few issues with the API, and the logic.

- Better API
    - `sort` is its own parameter
    - `group` is no longer its own parameter, now part of `filters`
    - `filters` can be any valid filterable span string property
    - better parameter order
    - `enabled` is an explicit parameter
- Add transaction as a valid string field

Also, more importantly, makes sure that the endpoint method is part of
the filtering, which was not the case before!
George Gritsouk 1 year ago
parent
commit
6487967406

+ 22 - 21
static/app/views/starfish/queries/useSpanTransactionMetrics.tsx

@@ -1,14 +1,15 @@
 import {Location} from 'history';
 
+import {defined} from 'sentry/utils';
 import EventView from 'sentry/utils/discover/eventView';
 import {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 {SpanMetricsField} from 'sentry/views/starfish/types';
+import {MetricsFilters, SpanMetricsField} from 'sentry/views/starfish/types';
 import {useWrappedDiscoverQuery} from 'sentry/views/starfish/utils/useSpansQuery';
 
-const {SPAN_SELF_TIME, SPAN_GROUP} = SpanMetricsField;
+const {SPAN_SELF_TIME} = SpanMetricsField;
 
 export type SpanTransactionMetrics = {
   'avg(span.self_time)': number;
@@ -22,40 +23,40 @@ export type SpanTransactionMetrics = {
 };
 
 export const useSpanTransactionMetrics = (
-  group: string,
-  options: {sorts?: Sort[]; transactions?: string[]},
-  referrer = 'api.starfish.span-transaction-metrics',
-  cursor?: string
+  filters: MetricsFilters,
+  sorts?: Sort[],
+  cursor?: string,
+  enabled: boolean = true,
+  referrer = 'api.starfish.span-transaction-metrics'
 ) => {
   const location = useLocation();
 
-  const {transactions, sorts} = options;
-
-  const eventView = getEventView(group, location, transactions ?? [], sorts);
+  const eventView = getEventView(location, filters, sorts);
 
   return useWrappedDiscoverQuery<SpanTransactionMetrics[]>({
     eventView,
     initialData: [],
-    enabled: Boolean(group),
+    enabled,
     limit: 25,
     referrer,
     cursor,
   });
 };
 
-function getEventView(
-  group: string,
-  location: Location,
-  transactions: string[],
-  sorts?: Sort[]
-) {
+function getEventView(location: Location, filters: MetricsFilters = {}, sorts?: Sort[]) {
   const search = new MutableSearch('');
-  search.addFilterValues(SPAN_GROUP, [group]);
-  search.addFilterValues('transaction.op', ['http.server']);
 
-  if (transactions.length > 0) {
-    search.addFilterValues('transaction', transactions);
-  }
+  Object.entries(filters).forEach(([key, value]) => {
+    if (!defined(value)) {
+      return;
+    }
+
+    if (Array.isArray(value)) {
+      search.addFilterValues(key, value);
+    } else {
+      search.addFilterValue(key, value);
+    }
+  });
 
   const eventView = EventView.fromNewQueryWithLocation(
     {

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

@@ -36,7 +36,9 @@ export type SpanStringFields =
   | 'span.module'
   | 'span.action'
   | 'span.group'
-  | 'project.id';
+  | 'project.id'
+  | 'transaction'
+  | 'transaction.method';
 
 export type SpanStringArrayFields = 'span.domain';
 
@@ -61,6 +63,10 @@ export type MetricsResponse = {
   'time_spent_percentage(local)': number;
 };
 
+export type MetricsFilters = {
+  [Property in SpanStringFields as `${Property}`]?: string | string[];
+};
+
 export type MetricsProperty = keyof MetricsResponse;
 
 export enum SpanIndexedField {

+ 6 - 5
static/app/views/starfish/views/spanSummaryPage/spanTransactionsTable.tsx

@@ -71,13 +71,14 @@ export function SpanTransactionsTable({span, endpoint, endpointMethod, sort}: Pr
     isLoading,
     pageLinks,
   } = useSpanTransactionMetrics(
-    span[SpanMetricsField.SPAN_GROUP],
     {
-      transactions: endpoint ? [endpoint] : undefined,
-      sorts: [sort],
+      'span.group': span[SpanMetricsField.SPAN_GROUP],
+      transaction: endpoint,
+      'transaction.method': endpointMethod,
     },
-    undefined,
-    cursor
+    [sort],
+    cursor,
+    Boolean(span[SpanMetricsField.SPAN_GROUP])
   );
 
   const spanTransactionsWithMetrics = spanTransactionMetrics.map(row => {