Browse Source

feat(insights): Switch to ratios for slow and frozen frames (#74523)

As described [here](https://github.com/getsentry/sentry/issues/73039),
we want to move to ratios for slow and frozen frames.

Utilizes [division_if](https://github.com/getsentry/sentry/pull/72995)
to calculate the ratios.

<img width="1240" alt="image"
src="https://github.com/user-attachments/assets/66d0b3ee-22ff-43eb-aee6-9242b9d3404a">
Markus Hintersteiner 7 months ago
parent
commit
6f127ef09c

+ 8 - 0
static/app/views/insights/mobile/appStarts/components/tables/screensTable.tsx

@@ -64,6 +64,13 @@ export function AppStartScreens({data, eventView, isLoading, pageLinks}: Props)
     'count_starts(measurements.app_start_warm)': t('Warm Start Count'),
   };
 
+  const columnTooltipMap = {
+    [`avg_compare(measurements.app_start_cold,release,${primaryRelease},${secondaryRelease})`]:
+      t('Average Cold Start difference'),
+    [`avg_compare(measurements.app_start_warm,release,${primaryRelease},${secondaryRelease})`]:
+      t('Average Warm Start difference'),
+  };
+
   function renderBodyCell(column, row): React.ReactNode {
     if (!data) {
       return null;
@@ -121,6 +128,7 @@ export function AppStartScreens({data, eventView, isLoading, pageLinks}: Props)
   return (
     <ScreensTable
       columnNameMap={columnNameMap}
+      columnTooltipMap={columnTooltipMap}
       data={data}
       eventView={eventView}
       isLoading={isLoading}

+ 34 - 1
static/app/views/insights/mobile/common/components/tables/screensTable.spec.tsx

@@ -1,4 +1,4 @@
-import {render, screen} from 'sentry-test/reactTestingLibrary';
+import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
 import EventView from 'sentry/utils/discover/eventView';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
@@ -34,6 +34,7 @@ describe('ScreensTable', () => {
         columnNameMap={{
           transaction: 'Screen',
         }}
+        columnTooltipMap={{}}
         columnOrder={['transaction']}
         data={{
           data: [{id: '1', transaction: 'Screen 1'}],
@@ -56,6 +57,7 @@ describe('ScreensTable', () => {
         columnNameMap={{
           transaction: 'Screen',
         }}
+        columnTooltipMap={{}}
         columnOrder={['transaction', 'non-custom']}
         data={{
           data: [
@@ -82,4 +84,35 @@ describe('ScreensTable', () => {
     expect(screen.getByText('Custom rendered Screen 1')).toBeInTheDocument();
     expect(screen.getByText('non customized value')).toBeInTheDocument();
   });
+
+  it('renders column header tooltips', async () => {
+    render(
+      <ScreensTable
+        columnNameMap={{
+          transaction: 'Screen Column',
+        }}
+        columnTooltipMap={{
+          transaction: 'Screen Column Tooltip',
+        }}
+        columnOrder={['transaction', 'non-custom']}
+        data={{
+          data: [
+            {id: '1', transaction: 'Screen 1', 'non-custom': 'non customized value'},
+          ],
+          meta: {fields: {transaction: 'string'}},
+        }}
+        defaultSort={[]}
+        eventView={getMockEventView({
+          fields: [{field: 'transaction'}, {field: 'non-custom'}],
+        })}
+        isLoading={false}
+        pageLinks={undefined}
+      />
+    );
+
+    const columnHeader = screen.getByText('Screen Column');
+    await userEvent.hover(columnHeader);
+
+    expect(await screen.findByText('Screen Column Tooltip')).toBeInTheDocument();
+  });
 });

+ 31 - 0
static/app/views/insights/mobile/common/components/tables/screensTable.tsx

@@ -1,4 +1,5 @@
 import {Fragment} from 'react';
+import styled from '@emotion/styled';
 
 import type {
   GridColumn,
@@ -8,6 +9,7 @@ import type {
 import GridEditable, {COL_WIDTH_UNDEFINED} from 'sentry/components/gridEditable';
 import SortLink from 'sentry/components/gridEditable/sortLink';
 import Pagination from 'sentry/components/pagination';
+import {Tooltip} from 'sentry/components/tooltip';
 import {defined} from 'sentry/utils';
 import {trackAnalytics} from 'sentry/utils/analytics';
 import type {TableData, TableDataRow} from 'sentry/utils/discover/discoverQuery';
@@ -23,6 +25,7 @@ import type {ModuleName} from 'sentry/views/insights/types';
 type Props = {
   columnNameMap: Record<string, string>;
   columnOrder: string[];
+  columnTooltipMap: Record<string, string> | undefined;
   data: TableData | undefined;
   defaultSort: GridColumnSortBy<string>[];
   eventView: EventView;
@@ -42,6 +45,7 @@ export function ScreensTable({
   pageLinks,
   columnNameMap,
   columnOrder,
+  columnTooltipMap,
   defaultSort,
   customBodyCellRenderer,
   moduleName,
@@ -116,6 +120,21 @@ export function ScreensTable({
         generateSortLink={generateSortLink}
       />
     );
+
+    function columnWithTooltip(tooltipTitle: string) {
+      return (
+        <Alignment align={alignment}>
+          <StyledTooltip isHoverable title={<span>{tooltipTitle}</span>}>
+            {sortLink}
+          </StyledTooltip>
+        </Alignment>
+      );
+    }
+
+    const tooltip = columnTooltipMap ? columnTooltipMap[column.key] : undefined;
+    if (tooltip) {
+      return columnWithTooltip(tooltip);
+    }
     return sortLink;
   }
 
@@ -152,3 +171,15 @@ export function ScreensTable({
     </Fragment>
   );
 }
+
+const Alignment = styled('span')<{align: string}>`
+  display: block;
+  margin: auto;
+  text-align: ${props => props.align};
+  width: 100%;
+`;
+
+const StyledTooltip = styled(Tooltip)`
+  top: 1px;
+  position: relative;
+`;

+ 4 - 4
static/app/views/insights/mobile/ui/components/tables/spanOperationTable.spec.tsx

@@ -63,10 +63,10 @@ describe('SpanOperationTable', () => {
             'span.op',
             'span.group',
             'span.description',
-            'avg_if(mobile.slow_frames,release,foo)',
-            'avg_if(mobile.slow_frames,release,bar)',
-            'avg_if(mobile.frozen_frames,release,foo)',
-            'avg_if(mobile.frozen_frames,release,bar)',
+            'division_if(mobile.slow_frames,mobile.total_frames,release,foo)',
+            'division_if(mobile.slow_frames,mobile.total_frames,release,bar)',
+            'division_if(mobile.frozen_frames,mobile.total_frames,release,foo)',
+            'division_if(mobile.frozen_frames,mobile.total_frames,release,bar)',
             'avg_if(mobile.frames_delay,release,foo)',
             'avg_if(mobile.frames_delay,release,bar)',
             'avg_compare(mobile.frames_delay,release,foo,bar)',

+ 38 - 21
static/app/views/insights/mobile/ui/components/tables/spanOperationTable.tsx

@@ -69,10 +69,10 @@ export function SpanOperationTable({
       SPAN_OP,
       SPAN_GROUP,
       SPAN_DESCRIPTION,
-      `avg_if(mobile.slow_frames,release,${primaryRelease})`,
-      `avg_if(mobile.slow_frames,release,${secondaryRelease})`,
-      `avg_if(mobile.frozen_frames,release,${primaryRelease})`,
-      `avg_if(mobile.frozen_frames,release,${secondaryRelease})`,
+      `division_if(mobile.slow_frames,mobile.total_frames,release,${primaryRelease})`,
+      `division_if(mobile.slow_frames,mobile.total_frames,release,${secondaryRelease})`,
+      `division_if(mobile.frozen_frames,mobile.total_frames,release,${primaryRelease})`,
+      `division_if(mobile.frozen_frames,mobile.total_frames,release,${secondaryRelease})`,
       `avg_if(mobile.frames_delay,release,${primaryRelease})`,
       `avg_if(mobile.frames_delay,release,${secondaryRelease})`,
       `avg_compare(mobile.frames_delay,release,${primaryRelease},${secondaryRelease})`,
@@ -97,22 +97,16 @@ export function SpanOperationTable({
   const columnNameMap = {
     [SPAN_OP]: t('Operation'),
     [SPAN_DESCRIPTION]: t('Span Description'),
-    [`avg_if(mobile.slow_frames,release,${primaryRelease})`]: t(
+    [`division_if(mobile.slow_frames,mobile.total_frames,release,${primaryRelease})`]: t(
       'Slow (%s)',
       PRIMARY_RELEASE_ALIAS
     ),
-    [`avg_if(mobile.slow_frames,release,${secondaryRelease})`]: t(
-      'Slow (%s)',
-      SECONDARY_RELEASE_ALIAS
-    ),
-    [`avg_if(mobile.frozen_frames,release,${primaryRelease})`]: t(
-      'Frozen (%s)',
-      PRIMARY_RELEASE_ALIAS
-    ),
-    [`avg_if(mobile.frozen_frames,release,${secondaryRelease})`]: t(
-      'Frozen (%s)',
-      SECONDARY_RELEASE_ALIAS
-    ),
+    [`division_if(mobile.slow_frames,mobile.total_frames,release,${secondaryRelease})`]:
+      t('Slow (%s)', SECONDARY_RELEASE_ALIAS),
+    [`division_if(mobile.frozen_frames,mobile.total_frames,release,${primaryRelease})`]:
+      t('Frozen (%s)', PRIMARY_RELEASE_ALIAS),
+    [`division_if(mobile.frozen_frames,mobile.total_frames,release,${secondaryRelease})`]:
+      t('Frozen (%s)', SECONDARY_RELEASE_ALIAS),
     [`avg_if(mobile.frames_delay,release,${primaryRelease})`]: t(
       'Delay (%s)',
       PRIMARY_RELEASE_ALIAS
@@ -125,6 +119,28 @@ export function SpanOperationTable({
       t('Change'),
   };
 
+  const columnTooltipMap = {
+    [`division_if(mobile.slow_frames,mobile.total_frames,release,${primaryRelease})`]: t(
+      'The number of slow frames divided by total frames (%s)',
+      PRIMARY_RELEASE_ALIAS
+    ),
+    [`division_if(mobile.slow_frames,mobile.total_frames,release,${secondaryRelease})`]:
+      t(
+        'The number of slow frames divided by total frames (%s)',
+        SECONDARY_RELEASE_ALIAS
+      ),
+    [`division_if(mobile.frozen_frames,mobile.total_frames,release,${primaryRelease})`]:
+      t(
+        'The number of frozen frames divided by total frames (%s)',
+        PRIMARY_RELEASE_ALIAS
+      ),
+    [`division_if(mobile.frozen_frames,mobile.total_frames,release,${secondaryRelease})`]:
+      t(
+        'The number of frozen frames divided by total frames (%s)',
+        SECONDARY_RELEASE_ALIAS
+      ),
+  };
+
   function renderBodyCell(column, row) {
     if (column.key === SPAN_DESCRIPTION) {
       const label = row[SpanMetricsField.SPAN_DESCRIPTION];
@@ -163,6 +179,7 @@ export function SpanOperationTable({
   return (
     <ScreensTable
       columnNameMap={columnNameMap}
+      columnTooltipMap={columnTooltipMap}
       data={data}
       eventView={eventView}
       isLoading={isLoading}
@@ -170,10 +187,10 @@ export function SpanOperationTable({
       columnOrder={[
         String(SPAN_OP),
         String(SPAN_DESCRIPTION),
-        `avg_if(mobile.slow_frames,release,${primaryRelease})`,
-        `avg_if(mobile.slow_frames,release,${secondaryRelease})`,
-        `avg_if(mobile.frozen_frames,release,${primaryRelease})`,
-        `avg_if(mobile.frozen_frames,release,${secondaryRelease})`,
+        `division_if(mobile.slow_frames,mobile.total_frames,release,${primaryRelease})`,
+        `division_if(mobile.slow_frames,mobile.total_frames,release,${secondaryRelease})`,
+        `division_if(mobile.frozen_frames,mobile.total_frames,release,${primaryRelease})`,
+        `division_if(mobile.frozen_frames,mobile.total_frames,release,${secondaryRelease})`,
         `avg_if(mobile.frames_delay,release,${primaryRelease})`,
         `avg_if(mobile.frames_delay,release,${secondaryRelease})`,
         `avg_compare(mobile.frames_delay,release,${primaryRelease},${secondaryRelease})`,

+ 42 - 17
static/app/views/insights/mobile/ui/components/tables/uiScreensTable.tsx

@@ -33,22 +33,16 @@ export function UIScreensTable({data, eventView, isLoading, pageLinks}: Props) {
 
   const columnNameMap = {
     transaction: t('Screen'),
-    [`avg_if(mobile.slow_frames,release,${primaryRelease})`]: t(
+    [`division_if(mobile.slow_frames,mobile.total_frames,release,${primaryRelease})`]: t(
       'Slow (%s)',
       PRIMARY_RELEASE_ALIAS
     ),
-    [`avg_if(mobile.slow_frames,release,${secondaryRelease})`]: t(
-      'Slow (%s)',
-      SECONDARY_RELEASE_ALIAS
-    ),
-    [`avg_if(mobile.frozen_frames,release,${primaryRelease})`]: t(
-      'Frozen (%s)',
-      PRIMARY_RELEASE_ALIAS
-    ),
-    [`avg_if(mobile.frozen_frames,release,${secondaryRelease})`]: t(
-      'Frozen (%s)',
-      SECONDARY_RELEASE_ALIAS
-    ),
+    [`division_if(mobile.slow_frames,mobile.total_frames,release,${secondaryRelease})`]:
+      t('Slow (%s)', SECONDARY_RELEASE_ALIAS),
+    [`division_if(mobile.frozen_frames,mobile.total_frames,release,${primaryRelease})`]:
+      t('Frozen (%s)', PRIMARY_RELEASE_ALIAS),
+    [`division_if(mobile.frozen_frames,mobile.total_frames,release,${secondaryRelease})`]:
+      t('Frozen (%s)', SECONDARY_RELEASE_ALIAS),
     [`avg_if(mobile.frames_delay,release,${primaryRelease})`]: t(
       'Delay (%s)',
       PRIMARY_RELEASE_ALIAS
@@ -66,6 +60,36 @@ export function UIScreensTable({data, eventView, isLoading, pageLinks}: Props) {
     // TODO: Counts
   };
 
+  const columnTooltipMap = {
+    [`division_if(mobile.slow_frames,mobile.total_frames,release,${primaryRelease})`]: t(
+      'The number of slow frames divided by total frames (%s)',
+      PRIMARY_RELEASE_ALIAS
+    ),
+    [`division_if(mobile.slow_frames,mobile.total_frames,release,${secondaryRelease})`]:
+      t(
+        'The number of slow frames divided by total frames (%s)',
+        SECONDARY_RELEASE_ALIAS
+      ),
+    [`division_if(mobile.frozen_frames,mobile.total_frames,release,${primaryRelease})`]:
+      t(
+        'The number of frozen frames divided by total frames (%s)',
+        PRIMARY_RELEASE_ALIAS
+      ),
+    [`division_if(mobile.frozen_frames,mobile.total_frames,release,${secondaryRelease})`]:
+      t(
+        'The number of frozen frames divided by total frames (%s)',
+        SECONDARY_RELEASE_ALIAS
+      ),
+    [`avg_if(mobile.frames_delay,release,${primaryRelease})`]: t(
+      'The average frame delay (%s)',
+      PRIMARY_RELEASE_ALIAS
+    ),
+    [`avg_if(mobile.frames_delay,release,${secondaryRelease})`]: t(
+      'The average frame delay (%s)',
+      SECONDARY_RELEASE_ALIAS
+    ),
+  };
+
   function renderBodyCell(column, row): React.ReactNode | null {
     if (!data) {
       return null;
@@ -113,16 +137,17 @@ export function UIScreensTable({data, eventView, isLoading, pageLinks}: Props) {
   return (
     <ScreensTable
       columnNameMap={columnNameMap}
+      columnTooltipMap={columnTooltipMap}
       data={data}
       eventView={eventView}
       isLoading={isLoading}
       pageLinks={pageLinks}
       columnOrder={[
         'transaction',
-        `avg_if(mobile.slow_frames,release,${primaryRelease})`,
-        `avg_if(mobile.slow_frames,release,${secondaryRelease})`,
-        `avg_if(mobile.frozen_frames,release,${primaryRelease})`,
-        `avg_if(mobile.frozen_frames,release,${secondaryRelease})`,
+        `division_if(mobile.slow_frames,mobile.total_frames,release,${primaryRelease})`,
+        `division_if(mobile.slow_frames,mobile.total_frames,release,${secondaryRelease})`,
+        `division_if(mobile.frozen_frames,mobile.total_frames,release,${primaryRelease})`,
+        `division_if(mobile.frozen_frames,mobile.total_frames,release,${secondaryRelease})`,
         `avg_if(mobile.frames_delay,release,${primaryRelease})`,
         `avg_if(mobile.frames_delay,release,${secondaryRelease})`,
         `avg_compare(mobile.frames_delay,release,${primaryRelease},${secondaryRelease})`,

+ 8 - 8
static/app/views/insights/mobile/ui/components/uiScreens.spec.tsx

@@ -28,10 +28,10 @@ const createMockTablePayload = ({
     null,
   'avg_if(mobile.frames_delay,release,com.example.vu.android@2.10.5)': 0,
   'avg_if(mobile.frames_delay,release,com.example.vu.android@2.10.3+42)': 0.259326119,
-  'avg_if(mobile.frozen_frames,release,com.example.vu.android@2.10.5)': 0,
-  'avg_if(mobile.frozen_frames,release,com.example.vu.android@2.10.3+42)': 0,
-  'avg_if(mobile.slow_frames,release,com.example.vu.android@2.10.5)': 0,
-  'avg_if(mobile.slow_frames,release,com.example.vu.android@2.10.3+42)': 2,
+  'division_if(mobile.frozen_frames,mobile.total_frames,release,com.example.vu.android@2.10.5)': 0,
+  'division_if(mobile.frozen_frames,mobile.total_frames,release,com.example.vu.android@2.10.3+42)': 0,
+  'division_if(mobile.slow_frames,mobile.total_frames,release,com.example.vu.android@2.10.5)': 0,
+  'division_if(mobile.slow_frames,mobile.total_frames,release,com.example.vu.android@2.10.3+42)': 2,
   'project.id': project.id,
   transaction,
 });
@@ -88,10 +88,10 @@ describe('Performance Mobile UI Screens', () => {
           field: [
             'project.id',
             'transaction',
-            'avg_if(mobile.slow_frames,release,com.example.vu.android@2.10.5)',
-            'avg_if(mobile.slow_frames,release,com.example.vu.android@2.10.3+42)',
-            'avg_if(mobile.frozen_frames,release,com.example.vu.android@2.10.5)',
-            'avg_if(mobile.frozen_frames,release,com.example.vu.android@2.10.3+42)',
+            'division_if(mobile.slow_frames,mobile.total_frames,release,com.example.vu.android@2.10.5)',
+            'division_if(mobile.slow_frames,mobile.total_frames,release,com.example.vu.android@2.10.3+42)',
+            'division_if(mobile.frozen_frames,mobile.total_frames,release,com.example.vu.android@2.10.5)',
+            'division_if(mobile.frozen_frames,mobile.total_frames,release,com.example.vu.android@2.10.3+42)',
             'avg_if(mobile.frames_delay,release,com.example.vu.android@2.10.5)',
             'avg_if(mobile.frames_delay,release,com.example.vu.android@2.10.3+42)',
             'avg_compare(mobile.frames_delay,release,com.example.vu.android@2.10.5,com.example.vu.android@2.10.3+42)',

+ 4 - 4
static/app/views/insights/mobile/ui/components/uiScreens.tsx

@@ -66,10 +66,10 @@ export function UIScreens() {
     fields: [
       SpanMetricsField.PROJECT_ID,
       'transaction',
-      `avg_if(mobile.slow_frames,release,${primaryRelease})`,
-      `avg_if(mobile.slow_frames,release,${secondaryRelease})`,
-      `avg_if(mobile.frozen_frames,release,${primaryRelease})`,
-      `avg_if(mobile.frozen_frames,release,${secondaryRelease})`,
+      `division_if(mobile.slow_frames,mobile.total_frames,release,${primaryRelease})`,
+      `division_if(mobile.slow_frames,mobile.total_frames,release,${secondaryRelease})`,
+      `division_if(mobile.frozen_frames,mobile.total_frames,release,${primaryRelease})`,
+      `division_if(mobile.frozen_frames,mobile.total_frames,release,${secondaryRelease})`,
       `avg_if(mobile.frames_delay,release,${primaryRelease})`,
       `avg_if(mobile.frames_delay,release,${secondaryRelease})`,
       `avg_compare(mobile.frames_delay,release,${primaryRelease},${secondaryRelease})`,