From 229345ec5089838307711fa6f664b462e85803d6 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 17 Feb 2023 19:38:14 -0500 Subject: [PATCH 1/4] feat(replay): Use SDK value for LCP 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 https://github.com/getsentry/sentry-javascript/pull/7225 --- static/app/components/replays/breadcrumbs/utils.tsx | 10 +++++----- .../replays/detail/breadcrumbs/breadcrumbItem.tsx | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/static/app/components/replays/breadcrumbs/utils.tsx b/static/app/components/replays/breadcrumbs/utils.tsx index fc4f7d6d769d9a..23b093987ac006 100644 --- a/static/app/components/replays/breadcrumbs/utils.tsx +++ b/static/app/components/replays/breadcrumbs/utils.tsx @@ -3,12 +3,12 @@ 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 `${crumb.data.value}ms`; } break; default: @@ -50,6 +50,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)}; } diff --git a/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx b/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx index 07d385a74f389d..ff6dc0f7d6e459 100644 --- a/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx @@ -32,7 +32,7 @@ function BreadcrumbItem({ onMouseLeave, onClick, }: Props) { - const {title, description} = getDetails(crumb, startTimestampMs); + const {title, description} = getDetails(crumb); const handleMouseEnter = useCallback( (e: React.MouseEvent) => onMouseEnter && onMouseEnter(crumb, e), From 4c833df27e988762a51d7563c2d631c44f4cd78b Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 24 Feb 2023 14:40:51 -0500 Subject: [PATCH 2/4] add warning when using outdated sdk --- .../app/components/replays/breadcrumbs/utils.tsx | 16 ++++++++++++++++ .../detail/breadcrumbs/breadcrumbItem.tsx | 4 +++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/static/app/components/replays/breadcrumbs/utils.tsx b/static/app/components/replays/breadcrumbs/utils.tsx index 23b093987ac006..9b6e010f508a03 100644 --- a/static/app/components/replays/breadcrumbs/utils.tsx +++ b/static/app/components/replays/breadcrumbs/utils.tsx @@ -1,3 +1,6 @@ +import {Tooltip} from 'sentry/components/tooltip'; +import {IconWarning} from 'sentry/icons'; +import {t} from 'sentry/locale'; import {BreadcrumbType, Crumb} from 'sentry/types/breadcrumbs'; /** @@ -10,6 +13,19 @@ export function getDescription(crumb: Crumb) { if (crumb.data?.value !== undefined) { return `${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 ( + + + + ); + } break; default: break; diff --git a/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx b/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx index ff6dc0f7d6e459..2623fa1644fde6 100644 --- a/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx @@ -74,7 +74,9 @@ function BreadcrumbItem({ ) : null} - {description} + + {description} + ); From c675dc518c8b583889c0b768404f3cc531215739 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 24 Feb 2023 14:41:07 -0500 Subject: [PATCH 3/4] just float things --- static/app/components/replays/breadcrumbs/utils.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/components/replays/breadcrumbs/utils.tsx b/static/app/components/replays/breadcrumbs/utils.tsx index 9b6e010f508a03..e6b6e54a7d6327 100644 --- a/static/app/components/replays/breadcrumbs/utils.tsx +++ b/static/app/components/replays/breadcrumbs/utils.tsx @@ -11,7 +11,7 @@ export function getDescription(crumb: Crumb) { switch (crumb.data.action) { case 'largest-contentful-paint': if (crumb.data?.value !== undefined) { - return `${crumb.data.value}ms`; + 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 From 6f71a8be696f78406de80e75725520ab663d6b69 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 24 Feb 2023 14:46:24 -0500 Subject: [PATCH 4/4] use Tooltip component --- .../views/replays/detail/breadcrumbs/breadcrumbItem.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx b/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx index 2623fa1644fde6..f7f4881182bbc6 100644 --- a/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx +++ b/static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx @@ -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'; @@ -74,9 +75,9 @@ function BreadcrumbItem({ ) : null} - - {description} - + + {description} + );