Browse Source

fix(perf): Isolate endpoint table cursor (#55387)

Paginating the endpoints table breaks _everything_ because all our
Discover queries automatically pick up the `cursor=` URL parameter,
which the endpoint pagination changes. Since we _only_ want to paginate
the table cursor, I'm isolating it. We do the same thing for the spans
table! Paginating through the endpoints only changes the
`endpointsCursor` parameter, so the `cursor=` parameter is never filled,
so all the queries work correctly because they don't specify a cursor.

- Rename spans cursor parameter name
- Rename span sort parameter name
- Fix incorrect argument name
- Clarify cursor variable names
- Allow span transactions hook to accept cursor
- Isolate endpoint list cursor
George Gritsouk 1 year ago
parent
commit
4f771b9e6f

+ 1 - 1
static/app/views/performance/database/databaseSpanSummaryPage.tsx

@@ -43,7 +43,7 @@ type Query = {
   endpointMethod: string;
   transaction: string;
   transactionMethod: string;
-  [QueryParameterNames.SORT]: string;
+  [QueryParameterNames.SPANS_SORT]: string;
 };
 
 type Props = {

+ 1 - 1
static/app/views/starfish/components/tableCells/renderHeadCell.tsx

@@ -54,7 +54,7 @@ export const renderHeadCell = ({column, location, sort}: Options) => {
           ...location,
           query: {
             ...location?.query,
-            [QueryParameterNames.SORT]: newSort,
+            [QueryParameterNames.SPANS_SORT]: newSort,
           },
         };
       }}

+ 2 - 2
static/app/views/starfish/components/tableCells/spanDescriptionCell.tsx

@@ -54,11 +54,11 @@ export function SpanDescriptionCell({
     endpointMethod,
   };
 
-  const sort: string | undefined = queryString[QueryParameterNames.SORT];
+  const sort: string | undefined = queryString[QueryParameterNames.SPANS_SORT];
 
   // the spans page uses time_spent_percentage(local), so to persist the sort upon navigation we need to replace
   if (sort?.includes(`${StarfishFunctions.TIME_SPENT_PERCENTAGE}()`)) {
-    queryString[QueryParameterNames.SORT] = sort.replace(
+    queryString[QueryParameterNames.SPANS_SORT] = sort.replace(
       `${StarfishFunctions.TIME_SPENT_PERCENTAGE}()`,
       `${StarfishFunctions.TIME_SPENT_PERCENTAGE}(local)`
     );

+ 4 - 2
static/app/views/starfish/queries/useSpanTransactionMetrics.tsx

@@ -24,7 +24,8 @@ export type SpanTransactionMetrics = {
 export const useSpanTransactionMetrics = (
   group: string,
   options: {sorts?: Sort[]; transactions?: string[]},
-  _referrer = 'api.starfish.span-transaction-metrics'
+  referrer = 'api.starfish.span-transaction-metrics',
+  cursor?: string
 ) => {
   const location = useLocation();
 
@@ -37,7 +38,8 @@ export const useSpanTransactionMetrics = (
     initialData: [],
     enabled: Boolean(group),
     limit: 25,
-    referrer: _referrer,
+    referrer,
+    cursor,
   });
 };
 

+ 3 - 2
static/app/views/starfish/views/queryParameters.tsx

@@ -1,4 +1,5 @@
 export enum QueryParameterNames {
-  CURSOR = 'spansCursor',
-  SORT = 'spansSort',
+  SPANS_CURSOR = 'spansCursor',
+  SPANS_SORT = 'spansSort',
+  ENDPOINTS_CURSOR = 'endpointsCursor',
 }

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

@@ -36,7 +36,7 @@ type Query = {
   endpointMethod: string;
   transaction: string;
   transactionMethod: string;
-  [QueryParameterNames.SORT]: string;
+  [QueryParameterNames.SPANS_SORT]: string;
 };
 
 type Props = {
@@ -54,7 +54,7 @@ function SpanSummaryPage({params, location}: Props) {
     : {};
 
   const sort =
-    fromSorts(location.query[QueryParameterNames.SORT]).filter(isAValidSort)[0] ??
+    fromSorts(location.query[QueryParameterNames.SPANS_SORT]).filter(isAValidSort)[0] ??
     DEFAULT_SORT; // We only allow one sort on this table in this view
 
   if (endpointMethod && queryFilter) {

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

@@ -1,4 +1,5 @@
 import {Fragment} from 'react';
+import {browserHistory} from 'react-router';
 import styled from '@emotion/styled';
 import omit from 'lodash/omit';
 import * as qs from 'query-string';
@@ -9,12 +10,13 @@ import GridEditable, {
   GridColumnHeader,
 } from 'sentry/components/gridEditable';
 import Link from 'sentry/components/links/link';
-import Pagination from 'sentry/components/pagination';
+import Pagination, {CursorHandler} from 'sentry/components/pagination';
 import Truncate from 'sentry/components/truncate';
 import {t} from 'sentry/locale';
 import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
 import {Sort} from 'sentry/utils/discover/fields';
 import {VisuallyCompleteWithData} from 'sentry/utils/performanceForSentry';
+import {decodeScalar} from 'sentry/utils/queryString';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import useRouter from 'sentry/utils/useRouter';
@@ -34,6 +36,7 @@ import {
 } from 'sentry/views/starfish/types';
 import {extractRoute} from 'sentry/views/starfish/utils/extractRoute';
 import {useRoutingContext} from 'sentry/views/starfish/utils/routingContext';
+import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 import {DataTitles, getThroughputTitle} from 'sentry/views/starfish/views/spans/types';
 import type {ValidSort} from 'sentry/views/starfish/views/spans/useModuleSort';
 
@@ -63,15 +66,22 @@ export function SpanTransactionsTable({span, endpoint, endpointMethod, sort}: Pr
   const organization = useOrganization();
   const router = useRouter();
 
+  const cursor = decodeScalar(location.query?.[QueryParameterNames.ENDPOINTS_CURSOR]);
+
   const {
     data: spanTransactionMetrics = [],
     meta,
     isLoading,
     pageLinks,
-  } = useSpanTransactionMetrics(span[SpanMetricsFields.SPAN_GROUP], {
-    transactions: endpoint ? [endpoint] : undefined,
-    sorts: [sort],
-  });
+  } = useSpanTransactionMetrics(
+    span[SpanMetricsFields.SPAN_GROUP],
+    {
+      transactions: endpoint ? [endpoint] : undefined,
+      sorts: [sort],
+    },
+    undefined,
+    cursor
+  );
 
   const spanTransactionsWithMetrics = spanTransactionMetrics.map(row => {
     return {
@@ -127,6 +137,13 @@ export function SpanTransactionsTable({span, endpoint, endpointMethod, sort}: Pr
     return rendered;
   };
 
+  const handleCursor: CursorHandler = (newCursor, pathname, query) => {
+    browserHistory.push({
+      pathname,
+      query: {...query, [QueryParameterNames.ENDPOINTS_CURSOR]: newCursor},
+    });
+  };
+
   return (
     <Fragment>
       <VisuallyCompleteWithData
@@ -156,7 +173,7 @@ export function SpanTransactionsTable({span, endpoint, endpointMethod, sort}: Pr
             {t('View More Endpoints')}
           </Button>
         )}
-        <StyledPagination pageLinks={pageLinks} />
+        <StyledPagination pageLinks={pageLinks} onCursor={handleCursor} />
       </Footer>
     </Fragment>
   );

+ 4 - 4
static/app/views/starfish/views/spans/spansTable.tsx

@@ -62,7 +62,7 @@ export default function SpansTable({
   const location = useLocation();
   const organization = useOrganization();
 
-  const spansCursor = decodeScalar(location.query?.[QueryParameterNames.CURSOR]);
+  const cursor = decodeScalar(location.query?.[QueryParameterNames.SPANS_CURSOR]);
 
   const {isLoading, data, meta, pageLinks} = useSpanList(
     moduleName ?? ModuleName.ALL,
@@ -72,13 +72,13 @@ export default function SpansTable({
     [sort],
     limit,
     'api.starfish.use-span-list',
-    spansCursor
+    cursor
   );
 
-  const handleCursor: CursorHandler = (cursor, pathname, query) => {
+  const handleCursor: CursorHandler = (newCursor, pathname, query) => {
     browserHistory.push({
       pathname,
-      query: {...query, [QueryParameterNames.CURSOR]: cursor},
+      query: {...query, [QueryParameterNames.SPANS_CURSOR]: newCursor},
     });
   };
 

+ 2 - 2
static/app/views/starfish/views/spans/useModuleSort.ts

@@ -5,7 +5,7 @@ import {SpanMetricsFields, StarfishFunctions} from 'sentry/views/starfish/types'
 import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 
 type Query = {
-  [QueryParameterNames.SORT]: string;
+  [QueryParameterNames.SPANS_SORT]: string;
 };
 
 const SORTABLE_FIELDS = [
@@ -28,7 +28,7 @@ export function useModuleSort(fallback: Sort = DEFAULT_SORT) {
   const location = useLocation<Query>();
 
   return (
-    fromSorts(location.query[QueryParameterNames.SORT]).filter(isAValidSort)[0] ??
+    fromSorts(location.query[QueryParameterNames.SPANS_SORT]).filter(isAValidSort)[0] ??
     fallback
   );
 }