Browse Source

fix(perf): Relax `http.server` requirement for database view (#57501)

The Database view only shows queries that appear in `http.server`
transactions. This is a legacy requirement, a holdover from early
Starfish work. This PR removes this requirement. Now, _all_ queries are
accounted for.

In short, I had to make sure that the flow still works when there's no
`transaction.method`, since most transactions don't have that. In order
to be safe, I added conditionals to omit that filter property, rather
than relaxing that requirement in the data fetching code.
George Gritsouk 1 year ago
parent
commit
689e9db0a8

+ 1 - 2
static/app/views/performance/landing/widgets/components/widgetContainer.spec.tsx

@@ -882,8 +882,7 @@ describe('Performance > Widgets > WidgetContainer', function () {
           ],
           per_page: QUERY_LIMIT_PARAM,
           project: ['-42'],
-          query:
-            'has:span.description span.module:db transaction.op:http.server transaction.op:pageload',
+          query: 'has:span.description span.module:db transaction.op:pageload',
           sort: '-time_spent_percentage()',
           statsPeriod: '7d',
         }),

+ 0 - 5
static/app/views/performance/landing/widgets/widgets/lineChartListWidget.tsx

@@ -171,7 +171,6 @@ export function LineChartListWidget(props: PerformanceWidgetProps) {
           eventView.additionalConditions.removeFilter('time_spent_percentage()');
           mutableSearch.addFilterValue('has', 'span.description');
           mutableSearch.addFilterValue('span.module', 'db');
-          mutableSearch.addFilterValue('transaction.op', 'http.server');
           eventView.query = mutableSearch.formatString();
         } else if (isSlowestType || isFramesType) {
           eventView.additionalConditions.setFilterValues('epm()', ['>0.01']);
@@ -292,10 +291,6 @@ export function LineChartListWidget(props: PerformanceWidgetProps) {
             // Update search query
             eventView.additionalConditions.removeFilter('event.type');
             eventView.additionalConditions.removeFilter('transaction');
-            eventView.additionalConditions.addFilterValue(
-              'transaction.op',
-              'http.server'
-            );
             eventView.additionalConditions.addFilterValue(
               SpanMetricsField.SPAN_GROUP,
               provided.widgetData.list.data[selectedListIndex][

+ 9 - 10
static/app/views/starfish/queries/useSpanList.tsx

@@ -67,16 +67,15 @@ function getEventView(
   spanCategory?: string,
   sorts?: Sort[]
 ) {
-  const query = [
-    ...buildEventViewQuery({
-      moduleName,
-      location,
-      transaction,
-      method,
-      spanCategory,
-    }),
-    'transaction.op:http.server',
-  ].join(' ');
+  const query = buildEventViewQuery({
+    moduleName,
+    location,
+    transaction,
+    method,
+    spanCategory,
+  })
+    .filter(Boolean)
+    .join(' ');
 
   const fields = [
     PROJECT_ID,

+ 14 - 3
static/app/views/starfish/queries/useSpanSamples.tsx

@@ -17,8 +17,8 @@ const {SPAN_SELF_TIME, SPAN_GROUP} = SpanIndexedField;
 
 type Options = {
   groupId: string;
-  transactionMethod: string;
   transactionName: string;
+  transactionMethod?: string;
 };
 
 export type SpanSample = Pick<
@@ -41,14 +41,25 @@ export const useSpanSamples = (options: Options) => {
   const query = new MutableSearch([
     `${SPAN_GROUP}:${groupId}`,
     `transaction:"${transactionName}"`,
-    `transaction.method:${transactionMethod}`,
   ]);
 
+  if (transactionMethod) {
+    query.addFilterValue('transaction.method', transactionMethod);
+  }
+
+  const filters = {
+    transactionName,
+  };
+
+  if (transactionMethod) {
+    filters['transaction.method'] = transactionMethod;
+  }
+
   const dateCondtions = getDateConditions(pageFilter.selection);
 
   const {isLoading: isLoadingSeries, data: spanMetricsSeriesData} = useSpanMetricsSeries(
     groupId,
-    {transactionName, 'transaction.method': transactionMethod},
+    filters,
     [`avg(${SPAN_SELF_TIME})`],
     'api.starfish.sidebar-span-metrics'
   );

+ 0 - 7
static/app/views/starfish/utils/useSpansQuery.tsx

@@ -13,7 +13,6 @@ import {
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
-import {StarfishType} from 'sentry/views/starfish/types';
 import {
   getRetryDelay,
   shouldRetryHandler,
@@ -29,7 +28,6 @@ export function useSpansQuery<T = any[]>({
   enabled,
   referrer = 'use-spans-query',
   cursor,
-  view = StarfishType.BACKEND,
 }: {
   cursor?: string;
   enabled?: boolean;
@@ -37,7 +35,6 @@ export function useSpansQuery<T = any[]>({
   initialData?: T;
   limit?: number;
   referrer?: string;
-  view?: StarfishType;
 }) {
   const isTimeseriesQuery = (eventView?.yAxis?.length ?? 0) > 0;
   const queryFunction = isTimeseriesQuery
@@ -48,10 +45,6 @@ export function useSpansQuery<T = any[]>({
 
   if (eventView) {
     const newEventView = eventView.clone();
-    // We can also add `if (view == 'mobile') -> 'transaction.op:ui.load'` here in the future
-    if (view === StarfishType.BACKEND) {
-      newEventView.query = `${eventView.query} transaction.op:http.server`;
-    }
     const response = queryFunction<T>({
       eventView: newEventView,
       initialData,

+ 10 - 2
static/app/views/starfish/views/spanSummaryPage/sampleList/durationChart/index.tsx

@@ -22,13 +22,13 @@ const {SPAN_SELF_TIME, SPAN_OP} = SpanMetricsField;
 
 type Props = {
   groupId: string;
-  transactionMethod: string;
   transactionName: string;
   highlightedSpanId?: string;
   onClickSample?: (sample: SpanSample) => void;
   onMouseLeaveSample?: () => void;
   onMouseOverSample?: (sample: SpanSample) => void;
   spanDescription?: string;
+  transactionMethod?: string;
 };
 
 function DurationChart({
@@ -66,13 +66,21 @@ function DurationChart({
         };
   };
 
+  const filters = {
+    transactionName,
+  };
+
+  if (transactionMethod) {
+    filters['transaction.method'] = transactionMethod;
+  }
+
   const {
     isLoading,
     data: spanMetricsSeriesData,
     error: spanMetricsSeriesError,
   } = useSpanMetricsSeries(
     groupId,
-    {transactionName, 'transaction.method': transactionMethod},
+    filters,
     [`avg(${SPAN_SELF_TIME})`],
     'api.starfish.sidebar-span-metrics-chart'
   );

+ 7 - 5
static/app/views/starfish/views/spanSummaryPage/sampleList/index.tsx

@@ -23,8 +23,8 @@ import SampleTable from 'sentry/views/starfish/views/spanSummaryPage/sampleList/
 
 type Props = {
   groupId: string;
-  transactionMethod: string;
   transactionName: string;
+  transactionMethod?: string;
 };
 
 export function SampleList({groupId, transactionName, transactionMethod}: Props) {
@@ -32,10 +32,12 @@ export function SampleList({groupId, transactionName, transactionMethod}: Props)
   const [highlightedSpanId, setHighlightedSpanId] = useState<string | undefined>(
     undefined
   );
-  const detailKey =
-    groupId && transactionName && transactionMethod
-      ? `${groupId}:${transactionName}:${transactionMethod}`
-      : undefined;
+
+  // A a transaction name is required to show the panel, but a transaction
+  // method is not
+  const detailKey = transactionName
+    ? [groupId, transactionName, transactionMethod].filter(Boolean).join(':')
+    : undefined;
 
   // eslint-disable-next-line react-hooks/exhaustive-deps
   const debounceSetHighlightedSpanId = useCallback(

+ 10 - 2
static/app/views/starfish/views/spanSummaryPage/sampleList/sampleInfo/index.tsx

@@ -14,17 +14,25 @@ const {SPAN_SELF_TIME, SPAN_OP} = SpanMetricsField;
 
 type Props = {
   groupId: string;
-  transactionMethod: string;
   transactionName: string;
+  transactionMethod?: string;
 };
 
 function SampleInfo(props: Props) {
   const {groupId, transactionName, transactionMethod} = props;
   const {setPageError} = usePageError();
 
+  const filters = {
+    transactionName,
+  };
+
+  if (transactionMethod) {
+    filters['transaction.method'] = transactionMethod;
+  }
+
   const {data: spanMetrics, error} = useSpanMetrics(
     groupId,
-    {transactionName, 'transaction.method': transactionMethod},
+    filters,
     [
       SPAN_OP,
       'spm()',

+ 10 - 2
static/app/views/starfish/views/spanSummaryPage/sampleList/sampleTable/sampleTable.tsx

@@ -23,11 +23,11 @@ const SpanSamplesTableContainer = styled('div')`
 
 type Props = {
   groupId: string;
-  transactionMethod: string;
   transactionName: string;
   highlightedSpanId?: string;
   onMouseLeaveSample?: () => void;
   onMouseOverSample?: (sample: SpanSample) => void;
+  transactionMethod?: string;
 };
 
 function SampleTable({
@@ -38,9 +38,17 @@ function SampleTable({
   onMouseOverSample,
   transactionMethod,
 }: Props) {
+  const filters = {
+    transactionName,
+  };
+
+  if (transactionMethod) {
+    filters['transaction.method'] = transactionMethod;
+  }
+
   const {data: spanMetrics, isFetching: isFetchingSpanMetrics} = useSpanMetrics(
     groupId,
-    {transactionName, 'transaction.method': transactionMethod},
+    filters,
     [`avg(${SPAN_SELF_TIME})`, SPAN_OP],
     'api.starfish.span-summary-panel-samples-table-avg'
   );

+ 7 - 4
static/app/views/starfish/views/spanSummaryPage/spanTransactionsTable.tsx

@@ -98,14 +98,17 @@ export function SpanTransactionsTable({span, endpoint, endpointMethod, sort}: Pr
       const pathname = `${routingContext.baseURL}/${
         extractRoute(location) ?? 'spans'
       }/span/${encodeURIComponent(span[SpanMetricsField.SPAN_GROUP])}`;
-      const query = {
+      const query: {[key: string]: string | undefined} = {
         ...location.query,
         endpoint,
         endpointMethod,
         transaction: row.transaction,
-        transactionMethod: row.transactionMethod,
       };
 
+      if (row.transactionMethod) {
+        query.transactionMethod = row.transactionMethod;
+      }
+
       return (
         <Link
           to={`${pathname}?${qs.stringify(query)}`}
@@ -174,7 +177,7 @@ export function SpanTransactionsTable({span, endpoint, endpointMethod, sort}: Pr
               query: omit(location.query, 'endpoint'),
             }}
           >
-            {t('View More Endpoints')}
+            {t('View More')}
           </Button>
         )}
         <StyledPagination pageLinks={pageLinks} onCursor={handleCursor} />
@@ -191,7 +194,7 @@ const getColumnOrder = (
 ): TableColumnHeader[] => [
   {
     key: 'transaction',
-    name: 'Found In Endpoints',
+    name: t('Found In'),
     width: COL_WIDTH_UNDEFINED,
   },
   {