Browse Source

fix(perf): Fix database module sort parameter confusion (#55539)

The span table and the endpoints table were confused about what's using
which sort URL parameter, so the sorts weren't working properly. This
clarifies things.

- Allow configurable sort parameter in table header
- Pass in correct sort parameter names
- Also for the hook version
George Gritsouk 1 year ago
parent
commit
681da0bead

+ 2 - 1
static/app/views/performance/database/databaseLandingPage.tsx

@@ -12,6 +12,7 @@ import useOrganization from 'sentry/utils/useOrganization';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import {ModulePageProviders} from 'sentry/views/performance/database/modulePageProviders';
 import {ModulePageProviders} from 'sentry/views/performance/database/modulePageProviders';
 import {ModuleName, SpanMetricsField} from 'sentry/views/starfish/types';
 import {ModuleName, SpanMetricsField} from 'sentry/views/starfish/types';
+import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 import {ActionSelector} from 'sentry/views/starfish/views/spans/selectors/actionSelector';
 import {ActionSelector} from 'sentry/views/starfish/views/spans/selectors/actionSelector';
 import {DomainSelector} from 'sentry/views/starfish/views/spans/selectors/domainSelector';
 import {DomainSelector} from 'sentry/views/starfish/views/spans/selectors/domainSelector';
 import SpansTable from 'sentry/views/starfish/views/spans/spansTable';
 import SpansTable from 'sentry/views/starfish/views/spans/spansTable';
@@ -24,7 +25,7 @@ function DatabaseLandingPage() {
   const moduleName = ModuleName.DB;
   const moduleName = ModuleName.DB;
 
 
   const moduleFilters = useModuleFilters();
   const moduleFilters = useModuleFilters();
-  const sort = useModuleSort();
+  const sort = useModuleSort(QueryParameterNames.SPANS_SORT);
 
 
   return (
   return (
     <ModulePageProviders title={[t('Performance'), t('Database')].join(' — ')}>
     <ModulePageProviders title={[t('Performance'), t('Database')].join(' — ')}>

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

@@ -61,7 +61,7 @@ function SpanSummaryPage({params}: Props) {
     ? {transactionName: endpoint, 'transaction.method': endpointMethod}
     ? {transactionName: endpoint, 'transaction.method': endpointMethod}
     : {};
     : {};
 
 
-  const sort = useModuleSort(DEFAULT_SORT);
+  const sort = useModuleSort(QueryParameterNames.ENDPOINTS_SORT, DEFAULT_SORT);
 
 
   const {data: fullSpan} = useFullSpanFromTrace(groupId);
   const {data: fullSpan} = useFullSpanFromTrace(groupId);
 
 

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

@@ -15,8 +15,14 @@ type Options = {
   column: GridColumnHeader<string>;
   column: GridColumnHeader<string>;
   location?: Location;
   location?: Location;
   sort?: Sort;
   sort?: Sort;
+  sortParameterName?:
+    | QueryParameterNames.ENDPOINTS_SORT
+    | QueryParameterNames.SPANS_SORT
+    | typeof DEFAULT_SORT_PARAMETER_NAME;
 };
 };
 
 
+const DEFAULT_SORT_PARAMETER_NAME = 'sort';
+
 const {SPAN_SELF_TIME} = SpanMetricsField;
 const {SPAN_SELF_TIME} = SpanMetricsField;
 const {TIME_SPENT_PERCENTAGE, SPS, SPM, HTTP_ERROR_COUNT} = SpanFunction;
 const {TIME_SPENT_PERCENTAGE, SPS, SPM, HTTP_ERROR_COUNT} = SpanFunction;
 
 
@@ -30,7 +36,7 @@ export const SORTABLE_FIELDS = new Set([
   `${HTTP_ERROR_COUNT}()`,
   `${HTTP_ERROR_COUNT}()`,
 ]);
 ]);
 
 
-export const renderHeadCell = ({column, location, sort}: Options) => {
+export const renderHeadCell = ({column, location, sort, sortParameterName}: Options) => {
   const {key, name} = column;
   const {key, name} = column;
   const alignment = getAlignment(key);
   const alignment = getAlignment(key);
 
 
@@ -54,7 +60,7 @@ export const renderHeadCell = ({column, location, sort}: Options) => {
           ...location,
           ...location,
           query: {
           query: {
             ...location?.query,
             ...location?.query,
-            [QueryParameterNames.SPANS_SORT]: newSort,
+            [sortParameterName ?? DEFAULT_SORT_PARAMETER_NAME]: newSort,
           },
           },
         };
         };
       }}
       }}

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

@@ -2,4 +2,5 @@ export enum QueryParameterNames {
   SPANS_CURSOR = 'spansCursor',
   SPANS_CURSOR = 'spansCursor',
   SPANS_SORT = 'spansSort',
   SPANS_SORT = 'spansSort',
   ENDPOINTS_CURSOR = 'endpointsCursor',
   ENDPOINTS_CURSOR = 'endpointsCursor',
+  ENDPOINTS_SORT = 'endpointsSort',
 }
 }

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

@@ -53,8 +53,9 @@ function SpanSummaryPage({params, location}: Props) {
     : {};
     : {};
 
 
   const sort =
   const sort =
-    fromSorts(location.query[QueryParameterNames.SPANS_SORT]).filter(isAValidSort)[0] ??
-    DEFAULT_SORT; // We only allow one sort on this table in this view
+    fromSorts(location.query[QueryParameterNames.ENDPOINTS_SORT]).filter(
+      isAValidSort
+    )[0] ?? DEFAULT_SORT; // We only allow one sort on this table in this view
 
 
   if (endpointMethod && queryFilter) {
   if (endpointMethod && queryFilter) {
     queryFilter['transaction.method'] = endpointMethod;
     queryFilter['transaction.method'] = endpointMethod;

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

@@ -153,7 +153,13 @@ export function SpanTransactionsTable({span, endpoint, endpointMethod, sort}: Pr
           columnOrder={getColumnOrder(span)}
           columnOrder={getColumnOrder(span)}
           columnSortBy={[]}
           columnSortBy={[]}
           grid={{
           grid={{
-            renderHeadCell: col => renderHeadCell({column: col, sort, location}),
+            renderHeadCell: col =>
+              renderHeadCell({
+                column: col,
+                sort,
+                location,
+                sortParameterName: QueryParameterNames.ENDPOINTS_SORT,
+              }),
             renderBodyCell,
             renderBodyCell,
           }}
           }}
           location={location}
           location={location}

+ 7 - 1
static/app/views/starfish/views/spans/spansTable.tsx

@@ -103,7 +103,13 @@ export default function SpansTable({
             },
             },
           ]}
           ]}
           grid={{
           grid={{
-            renderHeadCell: column => renderHeadCell({column, sort, location}),
+            renderHeadCell: column =>
+              renderHeadCell({
+                column,
+                sort,
+                location,
+                sortParameterName: QueryParameterNames.SPANS_SORT,
+              }),
             renderBodyCell: (column, row) =>
             renderBodyCell: (column, row) =>
               renderBodyCell(
               renderBodyCell(
                 column,
                 column,

+ 2 - 1
static/app/views/starfish/views/spans/spansView.tsx

@@ -3,6 +3,7 @@ import styled from '@emotion/styled';
 
 
 import {space} from 'sentry/styles/space';
 import {space} from 'sentry/styles/space';
 import {ModuleName, SpanMetricsField} from 'sentry/views/starfish/types';
 import {ModuleName, SpanMetricsField} from 'sentry/views/starfish/types';
+import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 import {ActionSelector} from 'sentry/views/starfish/views/spans/selectors/actionSelector';
 import {ActionSelector} from 'sentry/views/starfish/views/spans/selectors/actionSelector';
 import {DomainSelector} from 'sentry/views/starfish/views/spans/selectors/domainSelector';
 import {DomainSelector} from 'sentry/views/starfish/views/spans/selectors/domainSelector';
 import {SpanOperationSelector} from 'sentry/views/starfish/views/spans/selectors/spanOperationSelector';
 import {SpanOperationSelector} from 'sentry/views/starfish/views/spans/selectors/spanOperationSelector';
@@ -25,7 +26,7 @@ export default function SpansView(props: Props) {
   const moduleName = props.moduleName ?? ModuleName.ALL;
   const moduleName = props.moduleName ?? ModuleName.ALL;
 
 
   const moduleFilters = useModuleFilters();
   const moduleFilters = useModuleFilters();
-  const sort = useModuleSort();
+  const sort = useModuleSort(QueryParameterNames.SPANS_SORT);
 
 
   return (
   return (
     <Fragment>
     <Fragment>

+ 7 - 6
static/app/views/starfish/views/spans/useModuleSort.ts

@@ -5,7 +5,8 @@ import {SpanFunction, SpanMetricsField} from 'sentry/views/starfish/types';
 import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 
 
 type Query = {
 type Query = {
-  [QueryParameterNames.SPANS_SORT]: string;
+  [QueryParameterNames.SPANS_SORT]?: string;
+  [QueryParameterNames.ENDPOINTS_SORT]?: string;
 };
 };
 
 
 const SORTABLE_FIELDS = [
 const SORTABLE_FIELDS = [
@@ -24,13 +25,13 @@ export type ValidSort = Sort & {
  * Parses a `Sort` object from the URL. In case of multiple specified sorts
  * Parses a `Sort` object from the URL. In case of multiple specified sorts
  * picks the first one, since span module UIs only support one sort at a time.
  * picks the first one, since span module UIs only support one sort at a time.
  */
  */
-export function useModuleSort(fallback: Sort = DEFAULT_SORT) {
+export function useModuleSort(
+  sortParameterName: QueryParameterNames | 'sort' = 'sort',
+  fallback: Sort = DEFAULT_SORT
+) {
   const location = useLocation<Query>();
   const location = useLocation<Query>();
 
 
-  return (
-    fromSorts(location.query[QueryParameterNames.SPANS_SORT]).filter(isAValidSort)[0] ??
-    fallback
-  );
+  return fromSorts(location.query[sortParameterName]).filter(isAValidSort)[0] ?? fallback;
 }
 }
 
 
 const DEFAULT_SORT: Sort = {
 const DEFAULT_SORT: Sort = {