Browse Source

feat(performance): queues module table sorting (#70817)

Updates Queues module tables to allow sorting and updates the Time Spent
column
- Updates Queues module landing page and destination summary tables to
allow sorting by clicking column headers
- Update Time Spent columns to display a tooltip. Updates and utilizes
the `time_spent_percentage` function.
-  Also fixes minor loading state bug for metrics ribbons
edwardgou-sentry 10 months ago
parent
commit
47e08b88ab

+ 4 - 1
static/app/utils/discover/fieldRenderers.tsx

@@ -35,6 +35,7 @@ import {
   getSpanOperationName,
   isEquation,
   isRelativeSpanOperationBreakdownField,
+  parseFunction,
   SPAN_OP_BREAKDOWN_FIELDS,
   SPAN_OP_RELATIVE_BREAKDOWN_FIELD,
 } from 'sentry/utils/discover/fields';
@@ -782,10 +783,12 @@ const SPECIAL_FUNCTIONS: SpecialFunctions = {
     );
   },
   time_spent_percentage: fieldName => data => {
+    const parsedFunction = parseFunction(fieldName);
+    const column = parsedFunction?.arguments?.[1] ?? SpanMetricsField.SPAN_SELF_TIME;
     return (
       <TimeSpentCell
         percentage={data[fieldName]}
-        total={data[`sum(${SpanMetricsField.SPAN_SELF_TIME})`]}
+        total={data[`sum(${column})`]}
         op={data[`span.op`]}
       />
     );

+ 11 - 7
static/app/views/performance/queues/destinationSummary/destinationSummaryPage.tsx

@@ -33,6 +33,7 @@ import {
   MODULE_TITLE,
   RELEASE_LEVEL,
 } from 'sentry/views/performance/queues/settings';
+import {getTimeSpentExplanation} from 'sentry/views/starfish/components/tableCells/timeSpentCell';
 
 function DestinationSummaryPage() {
   const organization = useOrganization();
@@ -41,7 +42,7 @@ function DestinationSummaryPage() {
   const {query} = useLocation();
   const destination = decodeScalar(query.destination);
 
-  const {data} = useQueuesMetricsQuery({destination});
+  const {data, isLoading} = useQueuesMetricsQuery({destination});
   return (
     <Fragment>
       <Layout.Header>
@@ -95,37 +96,40 @@ function DestinationSummaryPage() {
                       title={t('Avg Time In Queue')}
                       value={data[0]?.['avg(messaging.message.receive.latency)']}
                       unit={DurationUnit.MILLISECOND}
-                      isLoading={false}
+                      isLoading={isLoading}
                     />
                     <MetricReadout
                       title={t('Avg Processing Time')}
                       value={data[0]?.['avg_if(span.duration,span.op,queue.process)']}
                       unit={DurationUnit.MILLISECOND}
-                      isLoading={false}
+                      isLoading={isLoading}
                     />
                     <MetricReadout
                       title={t('Error Rate')}
                       value={undefined}
                       unit={'percentage'}
-                      isLoading={false}
+                      isLoading={isLoading}
                     />
                     <MetricReadout
                       title={t('Published')}
                       value={data[0]?.['count_op(queue.publish)']}
                       unit={'count'}
-                      isLoading={false}
+                      isLoading={isLoading}
                     />
                     <MetricReadout
                       title={t('Processed')}
                       value={data[0]?.['count_op(queue.process)']}
                       unit={'count'}
-                      isLoading={false}
+                      isLoading={isLoading}
                     />
                     <MetricReadout
                       title={t('Time Spent')}
                       value={data[0]?.['sum(span.duration)']}
                       unit={DurationUnit.MILLISECOND}
-                      isLoading={false}
+                      tooltip={getTimeSpentExplanation(
+                        data[0]?.['time_spent_percentage(app,span.duration)']
+                      )}
+                      isLoading={isLoading}
                     />
                   </MetricsRibbon>
                 )}

+ 59 - 0
static/app/views/performance/queues/destinationSummary/transactionsTable.spec.tsx

@@ -2,10 +2,13 @@ import {OrganizationFixture} from 'sentry-fixture/organization';
 
 import {render, screen} from 'sentry-test/reactTestingLibrary';
 
+import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import {TransactionsTable} from 'sentry/views/performance/queues/destinationSummary/transactionsTable';
+import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 
 jest.mock('sentry/utils/useOrganization');
+jest.mock('sentry/utils/useLocation');
 
 describe('transactionsTable', () => {
   const organization = OrganizationFixture();
@@ -18,6 +21,16 @@ describe('transactionsTable', () => {
     '<https://sentry.io/fake/next>; rel="next"; results="true"; cursor="0:20:0"';
 
   beforeEach(() => {
+    jest.mocked(useLocation).mockReturnValue({
+      pathname: '',
+      search: '',
+      query: {statsPeriod: '10d', project: '1'},
+      hash: '',
+      state: undefined,
+      action: 'PUSH',
+      key: '',
+    });
+
     eventsMock = MockApiClient.addMockResponse({
       url: `/organizations/${organization.slug}/events/`,
       headers: {Link: pageLinks},
@@ -35,6 +48,7 @@ describe('transactionsTable', () => {
             'avg_if(span.duration,span.op,queue.publish)': 0,
             'avg_if(span.duration,span.op,queue.process)': 3,
             'avg(messaging.message.receive.latency)': 20,
+            'time_spent_percentage(app,span.duration)': 0.5,
           },
         ],
         meta: {
@@ -47,6 +61,7 @@ describe('transactionsTable', () => {
             'avg_if(span.duration,span.op,queue.publish)': 'duration',
             'avg_if(span.duration,span.op,queue.process)': 'duration',
             'avg(messaging.message.receive.latency)': 'duration',
+            'time_spent_percentage(app,span.duration)': 'percentage',
           },
         },
       },
@@ -84,6 +99,7 @@ describe('transactionsTable', () => {
             'avg_if(span.duration,span.op,queue.publish)',
             'avg_if(span.duration,span.op,queue.process)',
             'avg(messaging.message.receive.latency)',
+            'time_spent_percentage(app,span.duration)',
           ],
           dataset: 'spansMetrics',
         }),
@@ -97,4 +113,47 @@ describe('transactionsTable', () => {
     expect(screen.getByRole('cell', {name: 'Consumer'})).toBeInTheDocument();
     expect(screen.getByRole('button', {name: 'Next'})).toBeInTheDocument();
   });
+
+  it('sorts by processing time', async () => {
+    jest.mocked(useLocation).mockReturnValue({
+      pathname: '',
+      search: '',
+      query: {
+        statsPeriod: '10d',
+        project: '1',
+        [QueryParameterNames.DESTINATIONS_SORT]:
+          '-avg_if(span.duration,span.op,queue.process)',
+      },
+      hash: '',
+      state: undefined,
+      action: 'PUSH',
+      key: '',
+    });
+
+    render(<TransactionsTable />);
+
+    expect(eventsMock).toHaveBeenCalledWith(
+      '/organizations/org-slug/events/',
+      expect.objectContaining({
+        query: expect.objectContaining({
+          field: [
+            'transaction',
+            'span.op',
+            'count()',
+            'count_op(queue.publish)',
+            'count_op(queue.process)',
+            'sum(span.duration)',
+            'avg(span.duration)',
+            'avg_if(span.duration,span.op,queue.publish)',
+            'avg_if(span.duration,span.op,queue.process)',
+            'avg(messaging.message.receive.latency)',
+            'time_spent_percentage(app,span.duration)',
+          ],
+          dataset: 'spansMetrics',
+          sort: '-avg_if(span.duration,span.op,queue.process)',
+        }),
+      })
+    );
+    await screen.findByText('celery.backend_cleanup');
+  });
 });

+ 50 - 8
static/app/views/performance/queues/destinationSummary/transactionsTable.tsx

@@ -15,13 +15,15 @@ import type {Organization} from 'sentry/types';
 import {browserHistory} from 'sentry/utils/browserHistory';
 import type {EventsMetaType} from 'sentry/utils/discover/eventView';
 import {FIELD_FORMATTERS, getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
-import {decodeScalar} from 'sentry/utils/queryString';
+import type {Sort} from 'sentry/utils/discover/fields';
+import {decodeScalar, decodeSorts} from 'sentry/utils/queryString';
+import useLocationQuery from 'sentry/utils/url/useLocationQuery';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import {useQueuesByTransactionQuery} from 'sentry/views/performance/queues/queries/useQueuesByTransactionQuery';
 import {renderHeadCell} from 'sentry/views/starfish/components/tableCells/renderHeadCell';
-import type {SpanMetricsResponse} from 'sentry/views/starfish/types';
+import {SpanFunction, type SpanMetricsResponse} from 'sentry/views/starfish/types';
 import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 
 type Row = Pick<
@@ -71,19 +73,52 @@ const COLUMN_ORDER: Column[] = [
     width: COL_WIDTH_UNDEFINED,
   },
   {
-    key: 'sum(span.duration)',
+    key: 'time_spent_percentage(app,span.duration)',
     name: t('Time Spent'),
     width: COL_WIDTH_UNDEFINED,
   },
 ];
 
+const SORTABLE_FIELDS = [
+  'transaction',
+  'count_op(queue.publish)',
+  'count_op(queue.process)',
+  'avg_if(span.duration,span.op,queue.process)',
+  'avg(messaging.message.receive.latency)',
+  `${SpanFunction.TIME_SPENT_PERCENTAGE}(app,span.duration)`,
+] as const;
+
+type ValidSort = Sort & {
+  field: (typeof SORTABLE_FIELDS)[number];
+};
+
+export function isAValidSort(sort: Sort): sort is ValidSort {
+  return (SORTABLE_FIELDS as unknown as string[]).includes(sort.field);
+}
+
+const DEFAULT_SORT = {
+  field: 'time_spent_percentage(app,span.duration)' as const,
+  kind: 'desc' as const,
+};
+
 export function TransactionsTable() {
   const organization = useOrganization();
   const location = useLocation();
-  const destination = decodeScalar(location.query.destination);
+
+  const locationQuery = useLocationQuery({
+    fields: {
+      destination: decodeScalar,
+      [QueryParameterNames.DESTINATIONS_SORT]: decodeScalar,
+    },
+  });
+  const sort =
+    decodeSorts(locationQuery[QueryParameterNames.DESTINATIONS_SORT])
+      .filter(isAValidSort)
+      .at(0) ?? DEFAULT_SORT;
 
   const {data, isLoading, meta, pageLinks, error} = useQueuesByTransactionQuery({
-    destination,
+    destination: locationQuery.destination,
+    sort,
   });
 
   const handleCursor: CursorHandler = (newCursor, pathname, query) => {
@@ -101,12 +136,19 @@ export function TransactionsTable() {
         error={error}
         data={data}
         columnOrder={COLUMN_ORDER}
-        columnSortBy={[]}
+        columnSortBy={[
+          {
+            key: sort.field,
+            order: sort.kind,
+          },
+        ]}
         grid={{
-          renderHeadCell: col =>
+          renderHeadCell: column =>
             renderHeadCell({
-              column: col,
+              column,
+              sort,
               location,
+              sortParameterName: QueryParameterNames.DESTINATIONS_SORT,
             }),
           renderBodyCell: (column, row) =>
             renderBodyCell(column, row, meta, location, organization),

+ 2 - 0
static/app/views/performance/queues/messageSamplesPanel.spec.tsx

@@ -118,6 +118,7 @@ describe('messageSamplesPanel', () => {
             'avg_if(span.duration,span.op,queue.publish)',
             'avg_if(span.duration,span.op,queue.process)',
             'avg(messaging.message.receive.latency)',
+            'time_spent_percentage(app,span.duration)',
           ],
           per_page: 10,
           project: [],
@@ -190,6 +191,7 @@ describe('messageSamplesPanel', () => {
             'avg_if(span.duration,span.op,queue.publish)',
             'avg_if(span.duration,span.op,queue.process)',
             'avg(messaging.message.receive.latency)',
+            'time_spent_percentage(app,span.duration)',
           ],
           per_page: 10,
           project: [],

+ 1 - 1
static/app/views/performance/queues/queries/useQueuesByDestinationQuery.tsx

@@ -33,7 +33,7 @@ export function useQueuesByDestinationQuery({enabled, destination, sort}: Props)
         'avg_if(span.duration,span.op,queue.publish)',
         'avg_if(span.duration,span.op,queue.process)',
         'avg(messaging.message.receive.latency)',
-        'time_spent_percentage()',
+        'time_spent_percentage(app,span.duration)',
       ],
       enabled,
       sorts: sort ? [sort] : [],

+ 5 - 2
static/app/views/performance/queues/queries/useQueuesByTransactionQuery.tsx

@@ -1,3 +1,4 @@
+import type {Sort} from 'sentry/utils/discover/fields';
 import {decodeScalar} from 'sentry/utils/queryString';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {useLocation} from 'sentry/utils/useLocation';
@@ -8,9 +9,10 @@ import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 type Props = {
   destination?: string;
   enabled?: boolean;
+  sort?: Sort;
 };
 
-export function useQueuesByTransactionQuery({destination, enabled}: Props) {
+export function useQueuesByTransactionQuery({destination, enabled, sort}: Props) {
   const location = useLocation();
   const cursor = decodeScalar(location.query?.[QueryParameterNames.TRANSACTIONS_CURSOR]);
 
@@ -32,9 +34,10 @@ export function useQueuesByTransactionQuery({destination, enabled}: Props) {
         'avg_if(span.duration,span.op,queue.publish)',
         'avg_if(span.duration,span.op,queue.process)',
         'avg(messaging.message.receive.latency)',
+        'time_spent_percentage(app,span.duration)',
       ],
       enabled,
-      sorts: [],
+      sorts: sort ? [sort] : [],
       limit: 10,
       cursor,
     },

+ 1 - 0
static/app/views/performance/queues/queries/useQueuesMetricsQuery.tsx

@@ -28,6 +28,7 @@ export function useQueuesMetricsQuery({destination, transaction, enabled}: Props
         'avg_if(span.duration,span.op,queue.publish)',
         'avg_if(span.duration,span.op,queue.process)',
         'avg(messaging.message.receive.latency)',
+        'time_spent_percentage(app,span.duration)',
       ],
       enabled,
       sorts: [],

+ 5 - 4
static/app/views/performance/queues/queuesLandingPage.tsx

@@ -35,7 +35,7 @@ import {
 import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 
 const DEFAULT_SORT = {
-  field: 'time_spent_percentage()' as const,
+  field: 'time_spent_percentage(app,span.duration)' as const,
   kind: 'desc' as const,
 };
 
@@ -47,13 +47,14 @@ function QueuesLandingPage() {
   const query = useLocationQuery({
     fields: {
       destination: decodeScalar,
-      [QueryParameterNames.DOMAINS_SORT]: decodeScalar,
+      [QueryParameterNames.DESTINATIONS_SORT]: decodeScalar,
     },
   });
 
   const sort =
-    decodeSorts(query[QueryParameterNames.DOMAINS_SORT]).filter(isAValidSort).at(0) ??
-    DEFAULT_SORT;
+    decodeSorts(query[QueryParameterNames.DESTINATIONS_SORT])
+      .filter(isAValidSort)
+      .at(0) ?? DEFAULT_SORT;
 
   const handleSearch = (newDestination: string) => {
     browserHistory.push({

+ 11 - 4
static/app/views/performance/queues/queuesTable.spec.tsx

@@ -4,6 +4,7 @@ import {render, screen} from 'sentry-test/reactTestingLibrary';
 
 import useOrganization from 'sentry/utils/useOrganization';
 import {QueuesTable} from 'sentry/views/performance/queues/queuesTable';
+import {SpanIndexedField} from 'sentry/views/starfish/types';
 
 jest.mock('sentry/utils/useOrganization');
 
@@ -34,6 +35,7 @@ describe('queuesTable', () => {
             'avg_if(span.duration,span.op,queue.publish)': 0,
             'avg_if(span.duration,span.op,queue.process)': 3,
             'avg(messaging.message.receive.latency)': 20,
+            'time_spent_percentage(app,span.duration)': 0.5,
           },
         ],
         meta: {
@@ -46,13 +48,18 @@ describe('queuesTable', () => {
             'avg_if(span.duration,span.op,queue.publish)': 'duration',
             'avg_if(span.duration,span.op,queue.process)': 'duration',
             'avg(messaging.message.receive.latency)': 'duration',
+            'time_spent_percentage(app,span.duration)': 'percentage',
           },
         },
       },
     });
   });
   it('renders', async () => {
-    render(<QueuesTable />);
+    render(
+      <QueuesTable
+        sort={{field: 'time_spent_percentage(app,span.duration)', kind: 'desc'}}
+      />
+    );
     expect(screen.getByRole('table', {name: 'Queues'})).toBeInTheDocument();
     expect(screen.getByRole('columnheader', {name: 'Destination'})).toBeInTheDocument();
     expect(
@@ -79,7 +86,7 @@ describe('queuesTable', () => {
             'avg_if(span.duration,span.op,queue.publish)',
             'avg_if(span.duration,span.op,queue.process)',
             'avg(messaging.message.receive.latency)',
-            'time_spent_percentage()',
+            'time_spent_percentage(app,span.duration)',
           ],
           dataset: 'spansMetrics',
         }),
@@ -96,7 +103,7 @@ describe('queuesTable', () => {
     render(
       <QueuesTable
         destination="*events*"
-        sort={{field: 'messaging.destination.name', kind: 'desc'}}
+        sort={{field: SpanIndexedField.MESSAGING_MESSAGE_DESTINATION_NAME, kind: 'desc'}}
       />
     );
     expect(eventsMock).toHaveBeenCalledWith(
@@ -113,7 +120,7 @@ describe('queuesTable', () => {
             'avg_if(span.duration,span.op,queue.publish)',
             'avg_if(span.duration,span.op,queue.process)',
             'avg(messaging.message.receive.latency)',
-            'time_spent_percentage()',
+            'time_spent_percentage(app,span.duration)',
           ],
           dataset: 'spansMetrics',
           sort: '-messaging.destination.name',

Some files were not shown because too many files changed in this diff