Skip to content

Commit

Permalink
feat(replay): Use SDK value for LCP (#44868)
Browse files Browse the repository at this point in the history
Changes LCP breadcrumb rendering to use `data.value` instead of trying to calculate the value (which ends up resulting in incorrect values as it uses the replay start timestamp, so LCP from a refresh will be wrong).



Related getsentry/sentry-javascript#7225

Note that the above PR changes start + end timestamps to be equal since LCP is a single point in time rather than a range. The above PR also corrects the timestamp to be when the LCP occurs (rather than the previous start timestamp which is wrong and generally ends up around page load time).

Adds a warning for old SDK versions: 
![image](https://user-images.githubusercontent.com/79684/221275312-bbd5d365-4d33-483b-a6b7-658a5c325b9e.png)
  • Loading branch information
billyvg authored Feb 28, 2023
1 parent ede2640 commit bc92124
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 7 deletions.
26 changes: 21 additions & 5 deletions static/app/components/replays/breadcrumbs/utils.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,30 @@
import {Tooltip} from 'sentry/components/tooltip';
import {IconWarning} from 'sentry/icons';
import {t} from 'sentry/locale';
import {BreadcrumbType, Crumb} from 'sentry/types/breadcrumbs';

/**
* Generate breadcrumb descriptions based on type
*/
export function getDescription(crumb: Crumb, startTimestampMs?: number) {
export function getDescription(crumb: Crumb) {
if (typeof crumb.data === 'object' && crumb.data !== null && 'action' in crumb.data) {
switch (crumb.data.action) {
case 'largest-contentful-paint':
if (crumb.timestamp && startTimestampMs) {
return `${new Date(crumb.timestamp).getTime() - startTimestampMs}ms`;
if (crumb.data?.value !== undefined) {
return `${Math.round(crumb.data.value)}ms`;
}
if (crumb.data?.duration !== undefined) {
// this means user is using an old SDK where LCP values are not
// always correct. Prompt them to upgrade
return (
<Tooltip
title={t(
'This replay uses a SDK version that is subject to inaccurate LCP values. Please upgrade to the latest version for best results if you have not already done so.'
)}
>
<IconWarning />
</Tooltip>
);
}
break;
default:
Expand Down Expand Up @@ -50,6 +66,6 @@ export function getTitle(crumb: Crumb) {
/**
* Generate breadcrumb title + descriptions
*/
export function getDetails(crumb: Crumb, startTimestampMs?: number) {
return {title: getTitle(crumb), description: getDescription(crumb, startTimestampMs)};
export function getDetails(crumb: Crumb) {
return {title: getTitle(crumb), description: getDescription(crumb)};
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import styled from '@emotion/styled';
import BreadcrumbIcon from 'sentry/components/events/interfaces/breadcrumbs/breadcrumb/type/icon';
import {PanelItem} from 'sentry/components/panels';
import {getDetails} from 'sentry/components/replays/breadcrumbs/utils';
import {Tooltip} from 'sentry/components/tooltip';
import {SVGIconProps} from 'sentry/icons/svgIcon';
import {space} from 'sentry/styles/space';
import type {Crumb} from 'sentry/types/breadcrumbs';
Expand Down Expand Up @@ -32,7 +33,7 @@ function BreadcrumbItem({
onMouseLeave,
onClick,
}: Props) {
const {title, description} = getDetails(crumb, startTimestampMs);
const {title, description} = getDetails(crumb);

const handleMouseEnter = useCallback(
(e: React.MouseEvent<HTMLElement>) => onMouseEnter && onMouseEnter(crumb, e),
Expand Down Expand Up @@ -74,7 +75,9 @@ function BreadcrumbItem({
) : null}
</TitleContainer>

<Description title={description}>{description}</Description>
<Tooltip title={description} showOnlyOnOverflow>
<Description>{description}</Description>
</Tooltip>
</CrumbDetails>
</CrumbItem>
);
Expand Down

0 comments on commit bc92124

Please sign in to comment.