Skip to content

Commit

Permalink
feat(insights): Switch to ratios for slow and frozen frames (#74523)
Browse files Browse the repository at this point in the history
As described [here](#73039),
we want to move to ratios for slow and frozen frames.

Utilizes [division_if](#72995)
to calculate the ratios.

<img width="1240" alt="image"
src="https://github.com/user-attachments/assets/66d0b3ee-22ff-43eb-aee6-9242b9d3404a">
  • Loading branch information
markushi authored Jul 26, 2024
1 parent a1ec779 commit 6f127ef
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -121,6 +128,7 @@ export function AppStartScreens({data, eventView, isLoading, pageLinks}: Props)
return (
<ScreensTable
columnNameMap={columnNameMap}
columnTooltipMap={columnTooltipMap}
data={data}
eventView={eventView}
isLoading={isLoading}
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -34,6 +34,7 @@ describe('ScreensTable', () => {
columnNameMap={{
transaction: 'Screen',
}}
columnTooltipMap={{}}
columnOrder={['transaction']}
data={{
data: [{id: '1', transaction: 'Screen 1'}],
Expand All @@ -56,6 +57,7 @@ describe('ScreensTable', () => {
columnNameMap={{
transaction: 'Screen',
}}
columnTooltipMap={{}}
columnOrder={['transaction', 'non-custom']}
data={{
data: [
Expand All @@ -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();
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Fragment} from 'react';
import styled from '@emotion/styled';

import type {
GridColumn,
Expand All @@ -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';
Expand All @@ -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;
Expand All @@ -42,6 +45,7 @@ export function ScreensTable({
pageLinks,
columnNameMap,
columnOrder,
columnTooltipMap,
defaultSort,
customBodyCellRenderer,
moduleName,
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
`;
Original file line number Diff line number Diff line change
Expand Up @@ -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)',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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})`,
Expand All @@ -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
Expand All @@ -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];
Expand Down Expand Up @@ -163,17 +179,18 @@ export function SpanOperationTable({
return (
<ScreensTable
columnNameMap={columnNameMap}
columnTooltipMap={columnTooltipMap}
data={data}
eventView={eventView}
isLoading={isLoading}
pageLinks={pageLinks}
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})`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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})`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down Expand Up @@ -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)',
Expand Down
8 changes: 4 additions & 4 deletions static/app/views/insights/mobile/ui/components/uiScreens.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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})`,
Expand Down

0 comments on commit 6f127ef

Please sign in to comment.