From b6ce85dad84219e921166996c09af73605d2dc00 Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Fri, 5 Dec 2025 16:31:39 -0500 Subject: [PATCH 1/3] feat(trace-tree-node): Mitigating SpanNode type guard usage --- .../details/missingInstrumentation.tsx | 144 ++++-------------- .../details/span/sections/keys.tsx | 12 +- .../newTraceDetails/traceModels/traceTree.tsx | 4 +- .../traceModels/traceTreeNode/baseNode.tsx | 7 + .../traceModels/traceTreeNode/eapSpanNode.tsx | 4 +- .../traceTreeNode/uptimeCheckNode.tsx | 4 +- .../traceTreeNode/uptimeCheckTimingNode.tsx | 4 +- .../traceRow/traceEAPSpanRow.tsx | 115 ++++++++++++++ .../newTraceDetails/traceRow/traceSpanRow.tsx | 40 +---- .../traceRow/traceUptimeCheckNode.tsx | 105 +++++++++++++ .../traceRow/traceUptimeCheckTimingNode.tsx | 105 +++++++++++++ 11 files changed, 383 insertions(+), 161 deletions(-) create mode 100644 static/app/views/performance/newTraceDetails/traceRow/traceEAPSpanRow.tsx create mode 100644 static/app/views/performance/newTraceDetails/traceRow/traceUptimeCheckNode.tsx create mode 100644 static/app/views/performance/newTraceDetails/traceRow/traceUptimeCheckTimingNode.tsx diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/missingInstrumentation.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/missingInstrumentation.tsx index d6c4ad7bd3a3c8..14a6cfaed8cfe9 100644 --- a/static/app/views/performance/newTraceDetails/traceDrawer/details/missingInstrumentation.tsx +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/missingInstrumentation.tsx @@ -1,21 +1,12 @@ import {useMemo} from 'react'; import {ExternalLink} from 'sentry/components/core/link'; -import LoadingError from 'sentry/components/loadingError'; -import LoadingIndicator from 'sentry/components/loadingIndicator'; import {t, tct} from 'sentry/locale'; -import type {EventTransaction} from 'sentry/types/event'; -import type {Project} from 'sentry/types/project'; import useProjects from 'sentry/utils/useProjects'; import {useTransaction} from 'sentry/views/performance/newTraceDetails/traceApi/useTransaction'; import {getCustomInstrumentationLink} from 'sentry/views/performance/newTraceDetails/traceConfigurations'; import {ProfilePreview} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/profiling/profilePreview'; import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails'; -import { - isEAPSpanNode, - isSpanNode, -} from 'sentry/views/performance/newTraceDetails/traceGuards'; -import type {EapSpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode'; import type {NoInstrumentationNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/noInstrumentationNode'; import {ProfileGroupProvider} from 'sentry/views/profiling/profileGroupProvider'; import {ProfileContext, ProfilesProvider} from 'sentry/views/profiling/profilesProvider'; @@ -23,62 +14,17 @@ import {ProfileContext, ProfilesProvider} from 'sentry/views/profiling/profilesP import {TraceDrawerComponents} from './styles'; import {getProfileMeta} from './utils'; -interface BaseProps extends TraceTreeNodeDetailsProps { - event: EventTransaction | null; - profileId: string | undefined; - profileMeta: ReturnType; - profilerId: string | undefined; - project: Project | undefined; -} - export function MissingInstrumentationNodeDetails({ ...props }: TraceTreeNodeDetailsProps) { const {node} = props; const {projects} = useProjects(); + const previous = node.previous; - if (isEAPSpanNode(node.previous)) { - return ; - } - - if (isSpanNode(node.previous)) { - const event = node.previous.event; - const project = projects.find(proj => proj.slug === event?.projectSlug); - const profileMeta = getProfileMeta(event) || ''; - const profileContext = event?.contexts?.profile ?? {}; - - return ( - - ); - } - - return null; -} - -function EAPMissingInstrumentationNodeDetails({ - projects, - ...props -}: TraceTreeNodeDetailsProps & { - projects: Project[]; -}) { - const {node} = props; - const previous = node.previous as EapSpanNode; - - const { - data: eventTransaction = null, - isLoading: isEventTransactionLoading, - isError: isEventTransactionError, - } = useTransaction({ - event_id: previous.value.transaction_id, + const {data: eventTransaction = null} = useTransaction({ + event_id: previous.transactionId ?? '', organization: props.organization, - project_slug: previous.value.project_slug, + project_slug: previous.projectSlug ?? '', }); const profileMeta = useMemo( @@ -86,39 +32,9 @@ function EAPMissingInstrumentationNodeDetails({ [eventTransaction] ); - if (isEventTransactionLoading) { - return ; - } - - if (isEventTransactionError) { - return ; - } - const project = projects.find(proj => proj.slug === eventTransaction?.projectSlug); const profileContext = eventTransaction?.contexts?.profile ?? {}; - return ( - - ); -} - -function BaseMissingInstrumentationNodeDetails({ - node, - organization, - onTabScrollToNode, - profileMeta, - project, - event, - profileId, - profilerId, -}: BaseProps) { return ( @@ -135,8 +51,8 @@ function BaseMissingInstrumentationNodeDetails({ @@ -150,29 +66,31 @@ function BaseMissingInstrumentationNodeDetails({ } )}

- - - {profiles => ( - - - - )} - - + {eventTransaction ? ( + + + {profiles => ( + + + + )} + + + ) : null}

{t("If you'd prefer, you can also turn the feature off in the settings above.")}

diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/keys.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/keys.tsx index 1d675587a4bae3..692c27bd29ef1e 100644 --- a/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/keys.tsx +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/sections/keys.tsx @@ -23,7 +23,6 @@ import { type SectionCardKeyValueList, } from 'sentry/views/performance/newTraceDetails/traceDrawer/details/styles'; import {TraceDrawerActionValueKind} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/utils'; -import {isSpanNode} from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {SpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/spanNode'; import {getPerformanceDuration} from 'sentry/views/performance/utils/getPerformanceDuration'; @@ -77,11 +76,12 @@ function getSpanAggregateMeasurements(node: SpanNode) { let sum = 0; node.forEachChild(n => { - if ( - isSpanNode(n) && - typeof n?.value?.measurements?.ai_total_tokens_used?.value === 'number' - ) { - sum += n.value.measurements.ai_total_tokens_used.value; + const tokens = + typeof n.measurements?.ai_total_tokens_used === 'number' + ? n.measurements?.ai_total_tokens_used + : (n.measurements?.ai_total_tokens_used?.value ?? 0); + if (tokens) { + sum += tokens; } }); return [ diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx index 9c0d0ee3f5bd4a..5e8c3bf77d53d4 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx @@ -523,7 +523,7 @@ export class TraceTree extends TraceTreeEventDispatcher { tree, c, c.space[0], - c.value.measurements, + c.measurements, tree.vitals, tree.vital_types ) @@ -678,7 +678,7 @@ export class TraceTree extends TraceTreeEventDispatcher { tree, node, baseTraceNode.space[0], - node.value.measurements, + node.measurements, this.vitals, this.vital_types ) diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx index 02c085f6038e18..93cbe40e4b1b5d 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx @@ -2,6 +2,7 @@ import type {Theme} from '@emotion/react'; import type {Client} from 'sentry/api'; import {pickBarColor} from 'sentry/components/performance/waterfall/utils'; +import type {Measurement} from 'sentry/types/event'; import type {Organization} from 'sentry/types/organization'; import type {TraceMetaQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceMeta'; import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails'; @@ -326,6 +327,12 @@ export abstract class BaseNode | Record | undefined { + return this.value && 'measurements' in this.value + ? this.value.measurements + : undefined; + } + get attributes(): Record | undefined { return this.value && 'additional_attributes' in this.value ? this.value.additional_attributes diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx index 5dc4ce0cf921cd..ff1629e3b2d18a 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx @@ -11,8 +11,8 @@ import { isEAPTransaction, } from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; +import {TraceEAPSpanRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceEAPSpanRow'; import type {TraceRowProps} from 'sentry/views/performance/newTraceDetails/traceRow/traceRow'; -import {TraceSpanRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceSpanRow'; import {BaseNode, type TraceTreeNodeExtra} from './baseNode'; import {traceChronologicalSort} from './utils'; @@ -282,7 +282,7 @@ export class EapSpanNode extends BaseNode { renderWaterfallRow( props: TraceRowProps ): React.ReactNode { - return ; + return ; } renderDetails( diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode.tsx index d1d676fe8b6883..9d6904640ac45b 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode.tsx @@ -7,7 +7,7 @@ import {UptimeNodeDetails} from 'sentry/views/performance/newTraceDetails/traceD import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import type {TraceRowProps} from 'sentry/views/performance/newTraceDetails/traceRow/traceRow'; -import {TraceSpanRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceSpanRow'; +import {TraceUptimeCheckNodeRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceUptimeCheckNode'; import {BaseNode, type TraceTreeNodeExtra} from './baseNode'; import {UptimeCheckTimingNode} from './uptimeCheckTimingNode'; @@ -140,7 +140,7 @@ export class UptimeCheckNode extends BaseNode { renderWaterfallRow( props: TraceRowProps ): React.ReactNode { - return ; + return ; } renderDetails( diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckTimingNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckTimingNode.tsx index e4acda5f6c6e18..cf819b572a3b60 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckTimingNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckTimingNode.tsx @@ -5,7 +5,7 @@ import {UptimeTimingDetails} from 'sentry/views/performance/newTraceDetails/trac import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import type {TraceRowProps} from 'sentry/views/performance/newTraceDetails/traceRow/traceRow'; -import {TraceSpanRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceSpanRow'; +import {TraceUptimeCheckTimingNodeRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceUptimeCheckTimingNode'; import {BaseNode} from './baseNode'; @@ -33,7 +33,7 @@ export class UptimeCheckTimingNode extends BaseNode props: TraceRowProps ): React.ReactNode { return ( - ) { + const spanId = props.node.id; + + const childrenCount = getChildrenCount(props.node); + + const icon = ( + + ); + + return ( +
+ props.tabIndex === 0 + ? maybeFocusTraceRow(r, props.node, props.previouslyFocusedNodeRef) + : undefined + } + tabIndex={props.tabIndex} + className={`TraceRow ${props.rowSearchClassName} ${props.node.hasErrors ? props.node.maxIssueSeverity : ''}`} + onPointerDown={props.onRowClick} + onKeyDown={props.onRowKeyDown} + style={props.style} + > +
+
+
+ + {props.node.children.length > 0 || props.node.canFetchChildren ? ( + + ) + } + status={props.node.fetchStatus} + expanded={props.node.expanded || props.node.hasFetchedChildren} + onDoubleClick={props.onExpandDoubleClick} + onClick={e => + props.node.canFetchChildren ? props.onZoomIn(e) : props.onExpand(e) + } + > + {childrenCount > 0 ? TRACE_COUNT_FORMATTER.format(childrenCount) : null} + + ) : null} +
+ {icon} + + {props.node.value.op && props.node.value.op !== 'default' && ( + + {props.node.value.op} + — + + )} + + {props.node.description + ? ellipsize(props.node.description, 100) + : (spanId ?? 'unknown')} + + +
+
+
+ + +
+
+ ); +} + +function getChildrenCount(node: EapSpanNode) { + if (node.value.is_transaction && !node.expanded) { + return node.children.length - node.directVisibleChildren.length; + } + + return node.children.length; +} diff --git a/static/app/views/performance/newTraceDetails/traceRow/traceSpanRow.tsx b/static/app/views/performance/newTraceDetails/traceRow/traceSpanRow.tsx index 5cfda05c21b425..9ad0a507229505 100644 --- a/static/app/views/performance/newTraceDetails/traceRow/traceSpanRow.tsx +++ b/static/app/views/performance/newTraceDetails/traceRow/traceSpanRow.tsx @@ -1,19 +1,9 @@ import React from 'react'; import {PlatformIcon} from 'platformicons'; -import {IconSentry, IconTimer} from 'sentry/icons'; import {ellipsize} from 'sentry/utils/string/ellipsize'; -import { - isEAPTransactionNode, - isSpanNode, - isUptimeCheckNode, - isUptimeCheckTimingNode, -} from 'sentry/views/performance/newTraceDetails/traceGuards'; import {TraceIcons} from 'sentry/views/performance/newTraceDetails/traceIcons'; -import type {EapSpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode'; import type {SpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/spanNode'; -import type {UptimeCheckNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode'; -import type {UptimeCheckTimingNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckTimingNode'; import {TraceBar} from 'sentry/views/performance/newTraceDetails/traceRow/traceBar'; import { maybeFocusTraceRow, @@ -23,25 +13,15 @@ import { type TraceRowProps, } from 'sentry/views/performance/newTraceDetails/traceRow/traceRow'; -export function TraceSpanRow( - props: TraceRowProps -) { +export function TraceSpanRow(props: TraceRowProps) { const spanId = props.node.id; - const childrenCount = getChildrenCount(props.node); - - const icon = isUptimeCheckNode(props.node) ? ( - - ) : isUptimeCheckTimingNode(props.node) ? ( - - ) : ( + const icon = ( ); const isPrefetch = - isSpanNode(props.node) && - props.node.value.data && - !!props.node.value.data['http.request.prefetch']; + props.node.value.data && !!props.node.value.data['http.request.prefetch']; return (
- {childrenCount > 0 ? TRACE_COUNT_FORMATTER.format(childrenCount) : null} + {props.node.children.length > 0 + ? TRACE_COUNT_FORMATTER.format(props.node.children.length) + : null} ) : null}
@@ -127,13 +109,3 @@ export function TraceSpanRow( ); } - -function getChildrenCount( - node: SpanNode | EapSpanNode | UptimeCheckNode | UptimeCheckTimingNode -) { - if (isEAPTransactionNode(node) && !node.expanded) { - return node.children.length - node.directVisibleChildren.length; - } - - return node.children.length; -} diff --git a/static/app/views/performance/newTraceDetails/traceRow/traceUptimeCheckNode.tsx b/static/app/views/performance/newTraceDetails/traceRow/traceUptimeCheckNode.tsx new file mode 100644 index 00000000000000..1346dbb89e4dad --- /dev/null +++ b/static/app/views/performance/newTraceDetails/traceRow/traceUptimeCheckNode.tsx @@ -0,0 +1,105 @@ +import React from 'react'; + +import {IconSentry} from 'sentry/icons'; +import {ellipsize} from 'sentry/utils/string/ellipsize'; +import {TraceIcons} from 'sentry/views/performance/newTraceDetails/traceIcons'; +import type {UptimeCheckNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode'; +import {TraceBar} from 'sentry/views/performance/newTraceDetails/traceRow/traceBar'; +import { + maybeFocusTraceRow, + TRACE_COUNT_FORMATTER, + TraceChildrenButton, + TraceRowConnectors, + type TraceRowProps, +} from 'sentry/views/performance/newTraceDetails/traceRow/traceRow'; + +export function TraceUptimeCheckNodeRow(props: TraceRowProps) { + const spanId = props.node.id; + + const icon = ; + + return ( +
+ props.tabIndex === 0 + ? maybeFocusTraceRow(r, props.node, props.previouslyFocusedNodeRef) + : undefined + } + tabIndex={props.tabIndex} + className={`TraceRow ${props.rowSearchClassName} ${props.node.hasErrors ? props.node.maxIssueSeverity : ''}`} + onPointerDown={props.onRowClick} + onKeyDown={props.onRowKeyDown} + style={props.style} + > +
+
+
+ + {props.node.children.length > 0 || props.node.canFetchChildren ? ( + + ) + } + status={props.node.fetchStatus} + expanded={props.node.expanded || props.node.hasFetchedChildren} + onDoubleClick={props.onExpandDoubleClick} + onClick={e => + props.node.canFetchChildren ? props.onZoomIn(e) : props.onExpand(e) + } + > + {props.node.children.length > 0 + ? TRACE_COUNT_FORMATTER.format(props.node.children.length) + : null} + + ) : null} +
+ {icon} + + {props.node.value.op && props.node.value.op !== 'default' && ( + + {props.node.value.op} + — + + )} + + {props.node.description + ? ellipsize(props.node.description, 100) + : (spanId ?? 'unknown')} + + +
+
+
+ + +
+
+ ); +} diff --git a/static/app/views/performance/newTraceDetails/traceRow/traceUptimeCheckTimingNode.tsx b/static/app/views/performance/newTraceDetails/traceRow/traceUptimeCheckTimingNode.tsx new file mode 100644 index 00000000000000..569eead20447c3 --- /dev/null +++ b/static/app/views/performance/newTraceDetails/traceRow/traceUptimeCheckTimingNode.tsx @@ -0,0 +1,105 @@ +import React from 'react'; + +import {IconTimer} from 'sentry/icons'; +import {ellipsize} from 'sentry/utils/string/ellipsize'; +import {TraceIcons} from 'sentry/views/performance/newTraceDetails/traceIcons'; +import type {UptimeCheckTimingNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckTimingNode'; +import {TraceBar} from 'sentry/views/performance/newTraceDetails/traceRow/traceBar'; +import { + maybeFocusTraceRow, + TRACE_COUNT_FORMATTER, + TraceChildrenButton, + TraceRowConnectors, + type TraceRowProps, +} from 'sentry/views/performance/newTraceDetails/traceRow/traceRow'; + +export function TraceUptimeCheckTimingNodeRow( + props: TraceRowProps +) { + const icon = ; + + return ( +
+ props.tabIndex === 0 + ? maybeFocusTraceRow(r, props.node, props.previouslyFocusedNodeRef) + : undefined + } + tabIndex={props.tabIndex} + className={`TraceRow ${props.rowSearchClassName} ${props.node.hasErrors ? props.node.maxIssueSeverity : ''}`} + onPointerDown={props.onRowClick} + onKeyDown={props.onRowKeyDown} + style={props.style} + > +
+
+
+ + {props.node.children.length > 0 || props.node.canFetchChildren ? ( + + ) + } + status={props.node.fetchStatus} + expanded={props.node.expanded || props.node.hasFetchedChildren} + onDoubleClick={props.onExpandDoubleClick} + onClick={e => + props.node.canFetchChildren ? props.onZoomIn(e) : props.onExpand(e) + } + > + {props.node.children.length > 0 + ? TRACE_COUNT_FORMATTER.format(props.node.children.length) + : null} + + ) : null} +
+ {icon} + + {props.node.value.op && props.node.value.op !== 'default' && ( + + {props.node.value.op} + — + + )} + + {props.node.description + ? ellipsize(props.node.description, 100) + : (props.node.id ?? 'unknown')} + + +
+
+
+ + +
+
+ ); +} From dfe2925b422a086c0015bcaa879cc45ccae5871a Mon Sep 17 00:00:00 2001 From: Abdullah Khan Date: Sat, 6 Dec 2025 13:36:16 -0500 Subject: [PATCH 2/3] feat(trace-tree-node): Fixing knip errors --- .../views/performance/newTraceDetails/traceGuards.tsx | 9 --------- .../newTraceDetails/traceModels/traceTree.spec.tsx | 11 ++++++++++- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/static/app/views/performance/newTraceDetails/traceGuards.tsx b/static/app/views/performance/newTraceDetails/traceGuards.tsx index af3de4cef3a7d9..7154903cf9728e 100644 --- a/static/app/views/performance/newTraceDetails/traceGuards.tsx +++ b/static/app/views/performance/newTraceDetails/traceGuards.tsx @@ -11,7 +11,6 @@ import type {SiblingAutogroupNode} from './traceModels/traceTreeNode/siblingAuto import type {SpanNode} from './traceModels/traceTreeNode/spanNode'; import type {TransactionNode} from './traceModels/traceTreeNode/transactionNode'; import type {UptimeCheckNode} from './traceModels/traceTreeNode/uptimeCheckNode'; -import type {UptimeCheckTimingNode} from './traceModels/traceTreeNode/uptimeCheckTimingNode'; export function isMissingInstrumentationNode( node: BaseNode @@ -47,14 +46,6 @@ export function isUptimeCheckNode(node: BaseNode): node is UptimeCheckNode { return isUptimeCheck(node.value); } -export function isUptimeCheckTimingNode(node: BaseNode): node is UptimeCheckTimingNode { - return !!( - node.value && - 'event_type' in node.value && - node.value.event_type === 'uptime_check_timing' - ); -} - export function isTransactionNode(node: BaseNode): node is TransactionNode { return !!(node.value && 'transaction.op' in node.value); } diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx index a4c8eaf92ea463..2d2a77ba1befd0 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx @@ -10,11 +10,12 @@ import { isSpanNode, isTransactionNode, isUptimeCheckNode, - isUptimeCheckTimingNode, } from './../traceGuards'; +import type {BaseNode} from './traceTreeNode/baseNode'; import type {EapSpanNode} from './traceTreeNode/eapSpanNode'; import type {ParentAutogroupNode} from './traceTreeNode/parentAutogroupNode'; import type {SiblingAutogroupNode} from './traceTreeNode/siblingAutogroupNode'; +import type {UptimeCheckTimingNode} from './traceTreeNode/uptimeCheckTimingNode'; import {TraceShape, TraceTree} from './traceTree'; import { assertEAPSpanNode, @@ -2043,6 +2044,14 @@ describe('TraceTree', () => { }); describe('uptime check integration', () => { + function isUptimeCheckTimingNode(node: BaseNode): node is UptimeCheckTimingNode { + return !!( + node.value && + 'event_type' in node.value && + node.value.event_type === 'uptime_check_timing' + ); + } + it('automatically creates timing nodes when uptime check node is created', () => { const uptimeCheck = makeUptimeCheck({ additional_attributes: { From d37e10f5d12949153e4a627dc6e1ef4838f139e1 Mon Sep 17 00:00:00 2001 From: Abdullah Khan <60121741+Abdkhan14@users.noreply.github.com> Date: Fri, 12 Dec 2025 12:23:53 -0500 Subject: [PATCH 3/3] feat(trace-tree-node): Mitigating eap span node type guards usage (#104497) - Added an attributes getter to the baseNode class, used it to get rid of some guards - Removed isEapTransaction, reduces guards count. guards, we can check for an eapSpanNode with `value.is_transaction=true` --------- Co-authored-by: Abdullah Khan Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com> --- .../interfaces/performance/anrRootCause.tsx | 10 +- .../insights/agents/components/aiSpanList.tsx | 49 ++---- .../newTraceDetails/traceApi/useTrace.tsx | 7 +- .../traceApi/useTraceRootEvent.tsx | 47 ++--- .../newTraceDetails/traceApi/utils.tsx | 120 ++----------- .../newTraceDetails/traceContextVitals.tsx | 4 +- .../details/autogroup/index.spec.tsx | 74 ++++++++ .../traceDrawer/details/error.spec.tsx | 66 +++++++ .../traceDrawer/details/issues/issues.tsx | 5 +- .../details/missingInstrumentation.spec.tsx | 105 +++++++++++ .../traceDrawer/details/span/index.spec.tsx | 133 ++++++++++++++ .../details/transaction/index.spec.tsx | 93 ++++++++++ .../traceDrawer/details/uptime/index.spec.tsx | 76 ++++++++ .../newTraceDetails/traceGuards.tsx | 134 -------------- .../newTraceDetails/traceHeader/index.tsx | 7 +- .../newTraceDetails/traceHeader/meta.tsx | 43 +++-- .../newTraceDetails/traceHeader/title.tsx | 50 +----- .../traceModels/issuesTraceTree.spec.tsx | 27 ++- .../traceModels/traceTree.measurements.tsx | 73 ++++---- .../traceModels/traceTree.spec.tsx | 52 ++++-- .../newTraceDetails/traceModels/traceTree.tsx | 163 +++++++++++++++--- .../traceModels/traceTreeNode/baseNode.tsx | 40 +++-- .../traceModels/traceTreeNode/eapSpanNode.tsx | 60 +++++-- .../traceModels/traceTreeNode/traceNode.tsx | 4 +- .../traceTreeNode/transactionNode.tsx | 6 +- .../traceTreeNode/uptimeCheckNode.tsx | 2 + .../traceRenderers/virtualizedViewManager.tsx | 3 +- .../traceRow/traceCollapsedRow.tsx | 5 +- .../newTraceDetails/traceRow/traceIcons.tsx | 4 +- 29 files changed, 948 insertions(+), 514 deletions(-) create mode 100644 static/app/views/performance/newTraceDetails/traceDrawer/details/autogroup/index.spec.tsx create mode 100644 static/app/views/performance/newTraceDetails/traceDrawer/details/error.spec.tsx create mode 100644 static/app/views/performance/newTraceDetails/traceDrawer/details/missingInstrumentation.spec.tsx create mode 100644 static/app/views/performance/newTraceDetails/traceDrawer/details/span/index.spec.tsx create mode 100644 static/app/views/performance/newTraceDetails/traceDrawer/details/transaction/index.spec.tsx create mode 100644 static/app/views/performance/newTraceDetails/traceDrawer/details/uptime/index.spec.tsx diff --git a/static/app/components/events/interfaces/performance/anrRootCause.tsx b/static/app/components/events/interfaces/performance/anrRootCause.tsx index fac035c83a4b34..f28f803114e3cc 100644 --- a/static/app/components/events/interfaces/performance/anrRootCause.tsx +++ b/static/app/components/events/interfaces/performance/anrRootCause.tsx @@ -25,7 +25,6 @@ import {SectionKey} from 'sentry/views/issueDetails/streamline/context'; import {InterimSection} from 'sentry/views/issueDetails/streamline/interimSection'; import {useIssuesTraceTree} from 'sentry/views/performance/newTraceDetails/traceApi/useIssuesTraceTree'; import {useTrace} from 'sentry/views/performance/newTraceDetails/traceApi/useTrace'; -import {isEAPTraceOccurrence} from 'sentry/views/performance/newTraceDetails/traceGuards'; import useTraceStateAnalytics from 'sentry/views/performance/newTraceDetails/useTraceStateAnalytics'; enum AnrRootCauseAllowlist { @@ -143,12 +142,9 @@ export function AnrRootCause({event, organization}: Props) { > {potentialAnrRootCause?.map(occurence => { const project = projects.find(p => p.id === occurence.project_id.toString()); - const title = isEAPTraceOccurrence(occurence) - ? occurence.description - : occurence.title; - const shortId = isEAPTraceOccurrence(occurence) - ? occurence.short_id - : occurence.issue_short_id; + const isEAPOccurence = 'description' in occurence; + const title = isEAPOccurence ? occurence.description : occurence.title; + const shortId = isEAPOccurence ? occurence.short_id : occurence.issue_short_id; return ( diff --git a/static/app/views/insights/agents/components/aiSpanList.tsx b/static/app/views/insights/agents/components/aiSpanList.tsx index b1611981728aa3..015a150bbba31d 100644 --- a/static/app/views/insights/agents/components/aiSpanList.tsx +++ b/static/app/views/insights/agents/components/aiSpanList.tsx @@ -22,10 +22,6 @@ import { } from 'sentry/views/insights/agents/utils/query'; import type {AITraceSpanNode} from 'sentry/views/insights/agents/utils/types'; import {SpanFields} from 'sentry/views/insights/types'; -import { - isEAPSpanNode, - isTransactionNode, -} from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {EapSpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode'; import type {TransactionNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/transactionNode'; @@ -269,32 +265,18 @@ function getNodeInfo(node: AITraceSpanNode, colors: readonly string[]) { color: colors[1], }; - if (isTransactionNode(node)) { - nodeInfo.title = node.value.transaction || 'Transaction'; - nodeInfo.subtitle = node.value['transaction.op'] || ''; - return nodeInfo; - } - - const getNodeAttribute = (key: string) => { - if (isEAPSpanNode(node)) { - return node.value.additional_attributes?.[key]; - } - - return node.value?.data?.[key]; - }; - const op = node.op ?? 'default'; const truncatedOp = op.startsWith('gen_ai.') ? op.slice(7) : op; nodeInfo.title = truncatedOp; if (getIsAiRunSpan({op}) || getIsAiCreateAgentSpan({op})) { const agentName = - getNodeAttribute(SpanFields.GEN_AI_AGENT_NAME) || - getNodeAttribute(SpanFields.GEN_AI_FUNCTION_ID) || + node.attributes?.[SpanFields.GEN_AI_AGENT_NAME] || + node.attributes?.[SpanFields.GEN_AI_FUNCTION_ID] || ''; const model = - getNodeAttribute(SpanFields.GEN_AI_REQUEST_MODEL) || - getNodeAttribute(SpanFields.GEN_AI_RESPONSE_MODEL) || + node.attributes?.[SpanFields.GEN_AI_REQUEST_MODEL] || + node.attributes?.[SpanFields.GEN_AI_RESPONSE_MODEL] || ''; nodeInfo.icon = <IconBot size="md" />; nodeInfo.title = agentName || truncatedOp; @@ -308,8 +290,12 @@ function getNodeInfo(node: AITraceSpanNode, colors: readonly string[]) { } nodeInfo.color = colors[0]; } else if (getIsAiGenerationSpan({op})) { - const tokens = getNodeAttribute(SpanFields.GEN_AI_USAGE_TOTAL_TOKENS); - const cost = getNodeAttribute(SpanFields.GEN_AI_USAGE_TOTAL_COST); + const tokens = node.attributes?.[SpanFields.GEN_AI_USAGE_TOTAL_TOKENS] as + | number + | undefined; + const cost = node.attributes?.[SpanFields.GEN_AI_USAGE_TOTAL_COST] as + | number + | undefined; nodeInfo.icon = <IconSpeechBubble size="md" />; nodeInfo.subtitle = tokens ? ( <Fragment> @@ -328,17 +314,17 @@ function getNodeInfo(node: AITraceSpanNode, colors: readonly string[]) { } nodeInfo.color = colors[2]; } else if (getIsExecuteToolSpan({op})) { - const toolName = getNodeAttribute(SpanFields.GEN_AI_TOOL_NAME); + const toolName = node.attributes?.[SpanFields.GEN_AI_TOOL_NAME] as string | undefined; nodeInfo.icon = <IconTool size="md" />; nodeInfo.title = toolName || truncatedOp; nodeInfo.subtitle = toolName ? truncatedOp : ''; nodeInfo.color = colors[5]; } else if (getIsHandoffSpan({op})) { nodeInfo.icon = <IconChevron size="md" isDouble direction="right" />; - nodeInfo.subtitle = node.value.description || ''; + nodeInfo.subtitle = node.description || ''; nodeInfo.color = colors[4]; } else { - nodeInfo.subtitle = node.value.description || ''; + nodeInfo.subtitle = node.description || ''; } // Override the color and icon if the node has errors @@ -355,12 +341,9 @@ function hasError(node: AITraceSpanNode) { return true; } - if (isEAPSpanNode(node)) { - const status = node.value.additional_attributes?.[SpanFields.SPAN_STATUS]; - if (typeof status === 'string') { - return status.includes('error'); - } - return false; + const status = node.attributes?.[SpanFields.SPAN_STATUS] as string | undefined; + if (typeof status === 'string') { + return status.includes('error'); } return false; diff --git a/static/app/views/performance/newTraceDetails/traceApi/useTrace.tsx b/static/app/views/performance/newTraceDetails/traceApi/useTrace.tsx index 102f8548babaea..97d5798cf38a81 100644 --- a/static/app/views/performance/newTraceDetails/traceApi/useTrace.tsx +++ b/static/app/views/performance/newTraceDetails/traceApi/useTrace.tsx @@ -10,7 +10,6 @@ import {decodeScalar} from 'sentry/utils/queryString'; import type RequestError from 'sentry/utils/requestError/requestError'; import useOrganization from 'sentry/utils/useOrganization'; import usePageFilters from 'sentry/utils/usePageFilters'; -import {isValidEventUUID} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import {useIsEAPTraceEnabled} from 'sentry/views/performance/newTraceDetails/useIsEAPTraceEnabled'; @@ -291,3 +290,9 @@ export function useTrace( return isDemoMode ? demoTrace : isEAPEnabled ? eapTraceQuery : traceQuery; } + +const isValidEventUUID = (id: string): boolean => { + const uuidRegex = + /^[0-9a-f]{8}[0-9a-f]{4}[1-5][0-9a-f]{3}[89ab][0-9a-f]{3}[0-9a-f]{12}$/i; + return uuidRegex.test(id); +}; diff --git a/static/app/views/performance/newTraceDetails/traceApi/useTraceRootEvent.tsx b/static/app/views/performance/newTraceDetails/traceApi/useTraceRootEvent.tsx index cba79b9897c5ba..d1d89d242851ab 100644 --- a/static/app/views/performance/newTraceDetails/traceApi/useTraceRootEvent.tsx +++ b/static/app/views/performance/newTraceDetails/traceApi/useTraceRootEvent.tsx @@ -11,13 +11,7 @@ import { type OurLogsResponseItem, } from 'sentry/views/explore/logs/types'; import {TraceItemDataset} from 'sentry/views/explore/types'; -import {getRepresentativeTraceEvent} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; -import { - isEAPError, - isTraceError, -} from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; -import {useIsEAPTraceEnabled} from 'sentry/views/performance/newTraceDetails/useIsEAPTraceEnabled'; type Params = { logs: OurLogsResponseItem[] | undefined; @@ -34,28 +28,20 @@ export function useTraceRootEvent({ logs, traceId, }: Params): TraceRootEventQueryResults { - const rep = getRepresentativeTraceEvent(tree, logs); + const rep = tree.findRepresentativeTraceNode({logs}); const organization = useOrganization(); - // TODO: This is a bit of a mess, we won't need all of this once we switch to EAP only const treeIsLoading = tree.type === 'loading'; - const hasOnlyLogs = !!(tree.type === 'empty' && logs && logs.length > 0); - const enabledBase = - !treeIsLoading && (tree.type === 'trace' || hasOnlyLogs) && !!rep?.event && !!traceId; - const isRepEventError = - rep.event && OurLogKnownFieldKey.PROJECT_ID in rep.event - ? false - : isTraceError(rep.event) || isEAPError(rep.event); + const enabledBase = !treeIsLoading && !!rep?.event; - const isEAPTraceEnabled = useIsEAPTraceEnabled(); - const isEAPQueryEnabled = - !isRepEventError && // Errors are not supported in EAP yet - (isEAPTraceEnabled || (!treeIsLoading && hasOnlyLogs)); + const isRepLog = rep?.dataset === TraceItemDataset.LOGS; + const isEAPQueryEnabled = !!(isRepLog || rep?.event?.isEAPEvent); + const projectSlug = rep?.event?.projectSlug; const legacyRootEvent = useApiQuery<EventTransaction>( [ - `/organizations/${organization.slug}/events/${rep?.event?.project_slug}:${rep?.event?.event_id}/`, + `/organizations/${organization.slug}/events/${projectSlug}:${rep?.event?.id}/`, { query: { referrer: 'trace-details-summary', @@ -65,32 +51,27 @@ export function useTraceRootEvent({ { // 10 minutes staleTime: 1000 * 60 * 10, - enabled: enabledBase && !isEAPQueryEnabled, + enabled: enabledBase && !isEAPQueryEnabled && !!projectSlug && !!rep?.event?.id, } ); - const projectId = rep.event + const projectId = rep?.event ? OurLogKnownFieldKey.PROJECT_ID in rep.event ? rep.event[OurLogKnownFieldKey.PROJECT_ID] - : rep.event.project_id + : rep.event.projectId : ''; - const eventId = rep.event - ? OurLogKnownFieldKey.ID in rep.event + const eventId = rep?.event + ? OurLogKnownFieldKey.PROJECT_ID in rep.event ? rep.event[OurLogKnownFieldKey.ID] - : rep.event.event_id + : rep.event.id : ''; - - const itemTypes = { - log: TraceItemDataset.LOGS, - span: TraceItemDataset.SPANS, - uptime_check: TraceItemDataset.UPTIME_RESULTS, - }; + const dataset = rep?.dataset ?? TraceItemDataset.SPANS; const rootEvent = useTraceItemDetails({ traceItemId: String(eventId), projectId: String(projectId), traceId, - traceItemType: itemTypes[rep.type], + traceItemType: dataset, referrer: 'api.explore.log-item-details', enabled: enabledBase && isEAPQueryEnabled, }); diff --git a/static/app/views/performance/newTraceDetails/traceApi/utils.tsx b/static/app/views/performance/newTraceDetails/traceApi/utils.tsx index ad7c2ee57b471b..146d9e00c72c0d 100644 --- a/static/app/views/performance/newTraceDetails/traceApi/utils.tsx +++ b/static/app/views/performance/newTraceDetails/traceApi/utils.tsx @@ -1,15 +1,22 @@ import type {TraceItemDetailsResponse} from 'sentry/views/explore/hooks/useTraceItemDetails'; -import type {OurLogsResponseItem} from 'sentry/views/explore/logs/types'; +import type {TraceSplitResults} from 'sentry/views/performance/newTraceDetails/traceApi/types'; import type {TraceRootEventQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceRootEvent'; -import { - isEAPTraceNode, - isEAPTransaction, - isRootEvent, - isTraceNode, - isTraceSplitResult, - isUptimeCheckNode, -} from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; +import type {BaseNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; + +export function isBrowserRequestNode(node: BaseNode): boolean { + return ( + // Adjust for SDK changes in https://github.com/getsentry/sentry-javascript/pull/13527 + node.op === 'browser.request' || + (node.op === 'browser' && node.description === 'request') + ); +} + +export function isTraceSplitResult( + result: TraceTree.Trace +): result is TraceSplitResults<TraceTree.Transaction> { + return 'transactions' in result && 'orphan_errors' in result; +} export function isEmptyTrace(trace: TraceTree.Trace): boolean { if (isTraceSplitResult(trace)) { @@ -19,103 +26,8 @@ export function isEmptyTrace(trace: TraceTree.Trace): boolean { return trace.length === 0; } -const CANDIDATE_TRACE_TITLE_OPS = ['pageload', 'navigation', 'ui.load']; - -export type RepresentativeTraceEvent = { - event: TraceTree.TraceEvent | OurLogsResponseItem | null; - type: 'span' | 'log' | 'uptime_check'; -}; - -export const getRepresentativeTraceEvent = ( - tree: TraceTree, - logs: OurLogsResponseItem[] | undefined -): RepresentativeTraceEvent => { - const hasLogs = logs && logs.length > 0; - if (tree.type === 'empty' && hasLogs) { - return { - event: logs[0]!, - type: 'log', - }; - } - - const traceNode = tree.root.children[0]; - - if (!traceNode) { - return { - event: null, - type: 'span', - }; - } - - if (!isTraceNode(traceNode) && !isEAPTraceNode(traceNode)) { - throw new TypeError('Not trace node'); - } - - const traceChild = traceNode.children[0]; - - if (traceChild && isUptimeCheckNode(traceChild)) { - return {type: 'uptime_check', event: traceChild.value}; - } - - let rootEvent: TraceTree.TraceEvent | null = null; - let candidateEvent: TraceTree.TraceEvent | null = null; - let firstEvent: TraceTree.TraceEvent | null = null; - - const isEAP = isEAPTraceNode(traceNode); - const events = isEAP - ? traceNode.value - : [...traceNode.value.transactions, ...traceNode.value.orphan_errors]; - for (const event of events) { - if (isRootEvent(event)) { - rootEvent = event; - - if (!isEAP) { - // For non-EAP traces, we return the first root event. - break; - } - - if (isEAPTransaction(event)) { - // If we find a root EAP transaction, we can stop looking and use it for the title. - break; - } - // Otherwise we keep looking for a root eap transaction. If we don't find one, we use other roots, like standalone spans. - continue; - } else if ( - // If we haven't found a root transaction, but we found a candidate transaction - // with an op that we care about, we can use it for the title. We keep looking for - // a root. - !candidateEvent && - CANDIDATE_TRACE_TITLE_OPS.includes( - 'transaction.op' in event - ? event['transaction.op'] - : 'op' in event - ? event.op - : '' - ) - ) { - candidateEvent = event; - continue; - } else if (!firstEvent) { - // If we haven't found a root or candidate transaction, we can use the first transaction - // in the trace for the title. - firstEvent = event; - } - } - - return { - event: rootEvent ?? candidateEvent ?? firstEvent, - type: 'span', - }; -}; - export const isTraceItemDetailsResponse = ( data: TraceRootEventQueryResults['data'] ): data is TraceItemDetailsResponse => { return data !== undefined && 'attributes' in data; }; - -export const isValidEventUUID = (id: string): boolean => { - const uuidRegex = - /^[0-9a-f]{8}[0-9a-f]{4}[1-5][0-9a-f]{3}[89ab][0-9a-f]{3}[0-9a-f]{12}$/i; - return uuidRegex.test(id); -}; diff --git a/static/app/views/performance/newTraceDetails/traceContextVitals.tsx b/static/app/views/performance/newTraceDetails/traceContextVitals.tsx index 2ebb63e9c30318..3522458bd2721f 100644 --- a/static/app/views/performance/newTraceDetails/traceContextVitals.tsx +++ b/static/app/views/performance/newTraceDetails/traceContextVitals.tsx @@ -24,7 +24,6 @@ import { import {SectionDivider} from 'sentry/views/issueDetails/streamline/foldSection'; import type {TraceRootEventQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceRootEvent'; import {isTraceItemDetailsResponse} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; -import {isEAPTraceNode} from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import { TRACE_VIEW_MOBILE_VITALS, @@ -52,9 +51,8 @@ export function TraceContextVitals({rootEventResults, tree, containerWidth}: Pro ? TRACE_VIEW_WEB_VITALS : TRACE_VIEW_MOBILE_VITALS; - const isEAPTrace = isEAPTraceNode(traceNode); const collectedVitals = - isEAPTrace && tree.vital_types.has('mobile') + traceNode.isEAPEvent && tree.vital_types.has('mobile') ? getMobileVitalsFromRootEventResults(rootEventResults.data) : Array.from(tree.vitals.values()).flat(); diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/autogroup/index.spec.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/autogroup/index.spec.tsx new file mode 100644 index 00000000000000..d8f4806370ef36 --- /dev/null +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/autogroup/index.spec.tsx @@ -0,0 +1,74 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; + +import {render, screen} from 'sentry-test/reactTestingLibrary'; + +import type {TraceTreeNodeExtra} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; +import {EapSpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode'; +import {SiblingAutogroupNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/siblingAutogroupNode'; +import { + makeEAPSpan, + makeSiblingAutogroup, +} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeTestUtils'; +import {DEFAULT_TRACE_VIEW_PREFERENCES} from 'sentry/views/performance/newTraceDetails/traceState/tracePreferences'; +import {TraceStateProvider} from 'sentry/views/performance/newTraceDetails/traceState/traceStateProvider'; + +import {AutogroupNodeDetails} from './index'; + +const createMockExtra = ( + overrides: Partial<TraceTreeNodeExtra> = {} +): TraceTreeNodeExtra => ({ + organization: OrganizationFixture(), + ...overrides, +}); + +describe('AutogroupNodeDetails', () => { + it('renders autogroup details with title and description', () => { + const organization = OrganizationFixture(); + const extra = createMockExtra({organization}); + const autogroupValue = makeSiblingAutogroup({ + span_id: 'test-span-id', + autogrouped_by: { + op: 'db.query', + description: 'SELECT * FROM users', + }, + }); + + const childSpanValue = makeEAPSpan({event_id: 'child-span-1'}); + const childNode = new EapSpanNode(null, childSpanValue, extra); + const node = new SiblingAutogroupNode(null, autogroupValue, extra); + node.children = [childNode]; + + render( + <TraceStateProvider initialPreferences={DEFAULT_TRACE_VIEW_PREFERENCES}> + <AutogroupNodeDetails + node={node as any} + organization={organization} + onTabScrollToNode={jest.fn()} + onParentClick={jest.fn()} + manager={null} + replay={null} + traceId="test-trace-id" + /> + </TraceStateProvider> + ); + + expect(screen.getByText('Autogroup')).toBeInTheDocument(); + + expect(screen.getByText(/ID: test-span-id/)).toBeInTheDocument(); + + expect( + screen.getByText(/This block represents autogrouped spans/) + ).toBeInTheDocument(); + + expect( + screen.getByText('5 or more siblings with the same operation and description') + ).toBeInTheDocument(); + expect( + screen.getByText('2 or more descendants with the same operation') + ).toBeInTheDocument(); + + expect( + screen.getByText(/You can either open this autogroup using the chevron/) + ).toBeInTheDocument(); + }); +}); diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/error.spec.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/error.spec.tsx new file mode 100644 index 00000000000000..d08b2c29c00dd5 --- /dev/null +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/error.spec.tsx @@ -0,0 +1,66 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; +import {ProjectFixture} from 'sentry-fixture/project'; + +import {act, render, screen} from 'sentry-test/reactTestingLibrary'; + +import ProjectsStore from 'sentry/stores/projectsStore'; +import type {TraceTreeNodeExtra} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; +import {ErrorNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/errorNode'; +import {makeTraceError} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeTestUtils'; +import {DEFAULT_TRACE_VIEW_PREFERENCES} from 'sentry/views/performance/newTraceDetails/traceState/tracePreferences'; +import {TraceStateProvider} from 'sentry/views/performance/newTraceDetails/traceState/traceStateProvider'; + +import {ErrorNodeDetails} from './error'; + +const createMockExtra = ( + overrides: Partial<TraceTreeNodeExtra> = {} +): TraceTreeNodeExtra => ({ + organization: OrganizationFixture(), + ...overrides, +}); + +describe('ErrorNodeDetails', () => { + beforeEach(() => { + MockApiClient.clearMockResponses(); + }); + + it('renders error details with title and ID', () => { + const organization = OrganizationFixture(); + const project = ProjectFixture({id: '1', slug: 'project_slug'}); + + act(() => ProjectsStore.loadInitialData([project])); + + const errorValue = makeTraceError({ + event_id: 'test-error-id', + title: 'TypeError: Cannot read property of undefined', + level: 'error', + message: 'Something went wrong', + }); + + const extra = createMockExtra({organization}); + const node = new ErrorNode(null, errorValue, extra); + + render( + <TraceStateProvider initialPreferences={DEFAULT_TRACE_VIEW_PREFERENCES}> + <ErrorNodeDetails + node={node as any} + organization={organization} + onTabScrollToNode={jest.fn()} + onParentClick={jest.fn()} + manager={null} + replay={null} + traceId="test-trace-id" + tree={null as any} + /> + </TraceStateProvider> + ); + + expect(screen.getByText('Error')).toBeInTheDocument(); + + expect(screen.getByText(/ID: test-error-id/)).toBeInTheDocument(); + + expect( + screen.getByText(/This error is related to an ongoing issue/) + ).toBeInTheDocument(); + }); +}); diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/issues/issues.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/issues/issues.tsx index 676a3595cc7e43..b45b37d3219a35 100644 --- a/static/app/views/performance/newTraceDetails/traceDrawer/details/issues/issues.tsx +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/issues/issues.tsx @@ -14,7 +14,6 @@ import type {Group} from 'sentry/types/group'; import type {Organization} from 'sentry/types/organization'; import {useApiQuery} from 'sentry/utils/queryClient'; import {TraceDrawerComponents} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/styles'; -import {isTraceOccurence} from 'sentry/views/performance/newTraceDetails/traceGuards'; import {TraceIcons} from 'sentry/views/performance/newTraceDetails/traceIcons'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import type {BaseNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; @@ -69,8 +68,8 @@ function Issue(props: IssueProps) { } ); - const isOccurence: boolean = isTraceOccurence(props.issue); - const iconClassName: string = isOccurence ? 'occurence' : props.issue.level; + const iconClassName: string = + props.issue.event_type === 'error' ? props.issue.level : 'occurence'; return isPending ? ( <StyledLoadingIndicatorWrapper> diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/missingInstrumentation.spec.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/missingInstrumentation.spec.tsx new file mode 100644 index 00000000000000..3e7658064cda34 --- /dev/null +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/missingInstrumentation.spec.tsx @@ -0,0 +1,105 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; +import {ProjectFixture} from 'sentry-fixture/project'; + +import {act, render, screen} from 'sentry-test/reactTestingLibrary'; + +import ProjectsStore from 'sentry/stores/projectsStore'; +import type {TraceTreeNodeExtra} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; +import {NoInstrumentationNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/noInstrumentationNode'; +import {SpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/spanNode'; +import {makeSpan} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeTestUtils'; +import {DEFAULT_TRACE_VIEW_PREFERENCES} from 'sentry/views/performance/newTraceDetails/traceState/tracePreferences'; +import {TraceStateProvider} from 'sentry/views/performance/newTraceDetails/traceState/traceStateProvider'; + +import {MissingInstrumentationNodeDetails} from './missingInstrumentation'; + +const createMockExtra = ( + overrides: Partial<TraceTreeNodeExtra> = {} +): TraceTreeNodeExtra => ({ + organization: OrganizationFixture(), + ...overrides, +}); + +describe('MissingInstrumentationNodeDetails', () => { + beforeEach(() => { + MockApiClient.clearMockResponses(); + }); + + it('renders missing instrumentation details with title and subtitle', () => { + const organization = OrganizationFixture(); + const project = ProjectFixture({id: '1', slug: 'project_slug'}); + + act(() => ProjectsStore.loadInitialData([project])); + + const extra = createMockExtra({organization}); + + // Create previous and next span nodes + const previousSpanValue = makeSpan({ + span_id: 'previous-span-id', + op: 'db.query', + description: 'SELECT * FROM users', + start_timestamp: 1000, + timestamp: 1001, + }); + const previousNode = new SpanNode(null, previousSpanValue, extra); + + const nextSpanValue = makeSpan({ + span_id: 'next-span-id', + op: 'http.client', + description: 'GET /api/data', + start_timestamp: 1002, + timestamp: 1003, + }); + const nextNode = new SpanNode(null, nextSpanValue, extra); + + // Create the missing instrumentation span value + const missingInstrumentationValue = { + type: 'missing_instrumentation' as const, + start_timestamp: 1001, + timestamp: 1002, + }; + + const node = new NoInstrumentationNode( + previousNode, + nextNode, + null, + missingInstrumentationValue, + extra + ); + + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/events/:/`, + method: 'GET', + body: null, + }); + + render( + <TraceStateProvider initialPreferences={DEFAULT_TRACE_VIEW_PREFERENCES}> + <MissingInstrumentationNodeDetails + node={node as any} + organization={organization} + onTabScrollToNode={jest.fn()} + onParentClick={jest.fn()} + manager={null} + replay={null} + traceId="test-trace-id" + tree={null as any} + /> + </TraceStateProvider> + ); + + expect(screen.getByText('No Instrumentation')).toBeInTheDocument(); + + expect(screen.getByText('How Awkward')).toBeInTheDocument(); + + expect( + screen.getByText(/It looks like there's more than 100ms unaccounted for/) + ).toBeInTheDocument(); + + expect( + screen.getByText( + "If you'd prefer, you can also turn the feature off in the settings above." + ) + ).toBeInTheDocument(); + }); +}); diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/span/index.spec.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/index.spec.tsx new file mode 100644 index 00000000000000..60c930d108b8ee --- /dev/null +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/index.spec.tsx @@ -0,0 +1,133 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; +import {ProjectFixture} from 'sentry-fixture/project'; + +import {act, render, screen} from 'sentry-test/reactTestingLibrary'; + +import ProjectsStore from 'sentry/stores/projectsStore'; +import type {TraceTreeNodeExtra} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; +import {EapSpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode'; +import {SpanNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/spanNode'; +import { + makeEAPSpan, + makeSpan, +} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeTestUtils'; +import {DEFAULT_TRACE_VIEW_PREFERENCES} from 'sentry/views/performance/newTraceDetails/traceState/tracePreferences'; +import {TraceStateProvider} from 'sentry/views/performance/newTraceDetails/traceState/traceStateProvider'; + +import {EAPSpanNodeDetails, SpanNodeDetails} from './index'; + +const createMockExtra = ( + overrides: Partial<TraceTreeNodeExtra> = {} +): TraceTreeNodeExtra => ({ + organization: OrganizationFixture(), + ...overrides, +}); + +describe('SpanNodeDetails', () => { + beforeEach(() => { + MockApiClient.clearMockResponses(); + }); + + it('renders EAP span details with title, ID, op, and description', async () => { + const organization = OrganizationFixture(); + const project = ProjectFixture({id: '1', slug: 'project_slug'}); + + act(() => ProjectsStore.loadInitialData([project])); + + const spanValue = makeEAPSpan({ + event_id: 'test-span-id', + op: 'db.query', + description: 'SELECT * FROM users', + project_id: 1, + project_slug: 'project_slug', + }); + + const extra = createMockExtra({organization}); + const node = new EapSpanNode(null, spanValue, extra); + + MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${project.slug}/trace-items/${spanValue.event_id}/`, + method: 'GET', + body: { + itemId: spanValue.event_id, + timestamp: new Date().toISOString(), + attributes: [], + meta: {}, + }, + }); + + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/events/`, + method: 'GET', + body: {data: []}, + }); + + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/logs/`, + method: 'GET', + body: {data: []}, + }); + + render( + <TraceStateProvider initialPreferences={DEFAULT_TRACE_VIEW_PREFERENCES}> + <EAPSpanNodeDetails + node={node as any} + organization={organization} + onTabScrollToNode={jest.fn()} + onParentClick={jest.fn()} + manager={null} + replay={null} + traceId="test-trace-id" + tree={null as any} + /> + </TraceStateProvider> + ); + + expect(await screen.findByText('Span')).toBeInTheDocument(); + + expect(screen.getByText(/ID: test-span-id/)).toBeInTheDocument(); + + expect(screen.getByText('db.query')).toBeInTheDocument(); + + expect(screen.getByText(/SELECT \* FROM users/)).toBeInTheDocument(); + }); + + it('renders non-EAP span details with title, ID, op, and description', () => { + const organization = OrganizationFixture(); + const project = ProjectFixture({id: '1', slug: 'project_slug'}); + + act(() => ProjectsStore.loadInitialData([project])); + + const spanValue = makeSpan({ + span_id: 'legacy-span-id', + op: 'http.client', + description: 'GET /api/users', + }); + + const extra = createMockExtra({organization}); + const node = new SpanNode(null, spanValue, extra); + + render( + <TraceStateProvider initialPreferences={DEFAULT_TRACE_VIEW_PREFERENCES}> + <SpanNodeDetails + node={node as any} + organization={organization} + onTabScrollToNode={jest.fn()} + onParentClick={jest.fn()} + manager={null} + replay={null} + traceId="test-trace-id" + tree={null as any} + /> + </TraceStateProvider> + ); + + expect(screen.getByText('Span')).toBeInTheDocument(); + + expect(screen.getByText(/ID: legacy-span-id/)).toBeInTheDocument(); + + expect(screen.getAllByText('http.client').length).toBeGreaterThan(0); + + expect(screen.getAllByText(/GET \/api\/users/).length).toBeGreaterThan(0); + }); +}); diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/transaction/index.spec.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/transaction/index.spec.tsx new file mode 100644 index 00000000000000..552462da0c298c --- /dev/null +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/transaction/index.spec.tsx @@ -0,0 +1,93 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; +import {ProjectFixture} from 'sentry-fixture/project'; + +import {act, render, screen} from 'sentry-test/reactTestingLibrary'; + +import ProjectsStore from 'sentry/stores/projectsStore'; +import type {TraceTreeNodeExtra} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; +import {TransactionNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/transactionNode'; +import { + makeEventTransaction, + makeTransaction, +} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeTestUtils'; +import {DEFAULT_TRACE_VIEW_PREFERENCES} from 'sentry/views/performance/newTraceDetails/traceState/tracePreferences'; +import {TraceStateProvider} from 'sentry/views/performance/newTraceDetails/traceState/traceStateProvider'; + +import {TransactionNodeDetails} from './index'; + +const createMockExtra = ( + overrides: Partial<TraceTreeNodeExtra> = {} +): TraceTreeNodeExtra => ({ + organization: OrganizationFixture(), + ...overrides, +}); + +describe('TransactionNodeDetails', () => { + beforeEach(() => { + MockApiClient.clearMockResponses(); + }); + + it('renders transaction details with title, ID, and op', async () => { + const organization = OrganizationFixture(); + const project = ProjectFixture({id: '1', slug: 'project-slug'}); + + act(() => ProjectsStore.loadInitialData([project])); + + const transactionValue = makeTransaction({ + event_id: 'test-transaction-id', + 'transaction.op': 'http.server', + transaction: 'GET /api/users', + project_slug: 'project-slug', + start_timestamp: 1000, + timestamp: 1001, + }); + + const extra = createMockExtra({organization}); + const node = new TransactionNode(null, transactionValue, extra); + + // Mock the transaction event API + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/events/${project.slug}:${transactionValue.event_id}/`, + method: 'GET', + body: makeEventTransaction({ + eventID: transactionValue.event_id, + projectSlug: project.slug, + title: 'GET /api/users', + contexts: { + trace: { + op: 'http.server', + }, + }, + }), + }); + + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/events/`, + method: 'GET', + body: {data: []}, + }); + + render( + <TraceStateProvider initialPreferences={DEFAULT_TRACE_VIEW_PREFERENCES}> + <TransactionNodeDetails + node={node as any} + organization={organization} + onTabScrollToNode={jest.fn()} + onParentClick={jest.fn()} + manager={null} + replay={null} + traceId="test-trace-id" + tree={null as any} + /> + </TraceStateProvider> + ); + + expect(await screen.findByText('Transaction')).toBeInTheDocument(); + + expect(screen.getByText(/ID: test-transaction-id/)).toBeInTheDocument(); + + expect(screen.getAllByText('http.server').length).toBeGreaterThan(0); + + expect(screen.getAllByText(/GET \/api\/users/).length).toBeGreaterThan(0); + }); +}); diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/uptime/index.spec.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/uptime/index.spec.tsx new file mode 100644 index 00000000000000..3bb9821ffb2d13 --- /dev/null +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/uptime/index.spec.tsx @@ -0,0 +1,76 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; +import {ProjectFixture} from 'sentry-fixture/project'; + +import {act, render, screen} from 'sentry-test/reactTestingLibrary'; + +import ProjectsStore from 'sentry/stores/projectsStore'; +import type {TraceTreeNodeExtra} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; +import {UptimeCheckNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode'; +import {makeUptimeCheck} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeTestUtils'; +import {DEFAULT_TRACE_VIEW_PREFERENCES} from 'sentry/views/performance/newTraceDetails/traceState/tracePreferences'; +import {TraceStateProvider} from 'sentry/views/performance/newTraceDetails/traceState/traceStateProvider'; + +import {UptimeNodeDetails} from './index'; + +const createMockExtra = ( + overrides: Partial<TraceTreeNodeExtra> = {} +): TraceTreeNodeExtra => ({ + organization: OrganizationFixture(), + ...overrides, +}); + +describe('UptimeNodeDetails', () => { + beforeEach(() => { + MockApiClient.clearMockResponses(); + }); + + it('renders uptime check details with title and check ID', async () => { + const organization = OrganizationFixture(); + const project = ProjectFixture({id: '1', slug: 'project_slug'}); + + act(() => ProjectsStore.loadInitialData([project])); + + const uptimeCheckValue = makeUptimeCheck({ + event_id: 'test-uptime-check-id', + name: 'GET https://example.com', + description: 'Uptime check for example.com', + project_id: 1, + project_slug: 'project_slug', + }); + + const extra = createMockExtra({organization}); + const node = new UptimeCheckNode(null, uptimeCheckValue, extra); + + MockApiClient.addMockResponse({ + url: `/projects/${organization.slug}/${project.slug}/trace-items/${uptimeCheckValue.event_id}/`, + method: 'GET', + body: { + itemId: uptimeCheckValue.event_id, + timestamp: new Date().toISOString(), + attributes: [], + meta: {}, + }, + }); + + render( + <TraceStateProvider initialPreferences={DEFAULT_TRACE_VIEW_PREFERENCES}> + <UptimeNodeDetails + node={node as any} + organization={organization} + onTabScrollToNode={jest.fn()} + onParentClick={jest.fn()} + manager={null} + replay={null} + traceId="test-trace-id" + tree={null as any} + /> + </TraceStateProvider> + ); + + // Verify title is rendered + expect(await screen.findByText('Uptime Check Request')).toBeInTheDocument(); + + // Verify check ID subtitle is rendered + expect(screen.getByText(/Check ID: test-uptime-check-id/)).toBeInTheDocument(); + }); +}); diff --git a/static/app/views/performance/newTraceDetails/traceGuards.tsx b/static/app/views/performance/newTraceDetails/traceGuards.tsx index 7154903cf9728e..433f746ed934ca 100644 --- a/static/app/views/performance/newTraceDetails/traceGuards.tsx +++ b/static/app/views/performance/newTraceDetails/traceGuards.tsx @@ -1,6 +1,3 @@ -import type {Measurement} from 'sentry/types/event'; - -import type {TraceSplitResults} from './traceApi/types'; import type {TraceTree} from './traceModels/traceTree'; import type {BaseNode} from './traceModels/traceTreeNode/baseNode'; import type {CollapsedNode} from './traceModels/traceTreeNode/collapsedNode'; @@ -10,7 +7,6 @@ import type {ParentAutogroupNode} from './traceModels/traceTreeNode/parentAutogr import type {SiblingAutogroupNode} from './traceModels/traceTreeNode/siblingAutogroupNode'; import type {SpanNode} from './traceModels/traceTreeNode/spanNode'; import type {TransactionNode} from './traceModels/traceTreeNode/transactionNode'; -import type {UptimeCheckNode} from './traceModels/traceTreeNode/uptimeCheckNode'; export function isMissingInstrumentationNode( node: BaseNode @@ -30,22 +26,10 @@ export function isEAPSpan(value: TraceTree.NodeValue): value is TraceTree.EAPSpa return !!(value && 'is_transaction' in value); } -export function isEAPTransaction(value: TraceTree.NodeValue): value is TraceTree.EAPSpan { - return isEAPSpan(value) && value.is_transaction; -} - -export function isEAPTransactionNode(node: BaseNode): node is EapSpanNode { - return isEAPTransaction(node.value); -} - export function isEAPSpanNode(node: BaseNode): node is EapSpanNode { return isEAPSpan(node.value); } -export function isUptimeCheckNode(node: BaseNode): node is UptimeCheckNode { - return isUptimeCheck(node.value); -} - export function isTransactionNode(node: BaseNode): node is TransactionNode { return !!(node.value && 'transaction.op' in node.value); } @@ -88,121 +72,3 @@ export function isCollapsedNode(node: BaseNode): node is CollapsedNode { export function isTraceError(value: TraceTree.NodeValue): value is TraceTree.TraceError { return !!(value && 'level' in value && 'message' in value); } - -export function isTraceErrorNode(node: BaseNode): node is BaseNode<TraceTree.TraceError> { - return isTraceError(node.value); -} - -export function isTraceNode( - node: BaseNode -): node is BaseNode<TraceSplitResults<TraceTree.Transaction>> { - return !!(node.value && 'orphan_errors' in node.value && 'transactions' in node.value); -} - -export function isEAPTraceNode(node: BaseNode): node is BaseNode<TraceTree.EAPTrace> { - return !!node.value && Array.isArray(node.value) && !isTraceNode(node); -} - -export function shouldAddMissingInstrumentationSpan(sdk: string | undefined): boolean { - if (!sdk) { - return true; - } - if (sdk.length < 'sentry.javascript.'.length) { - return true; - } - - switch (sdk.toLowerCase()) { - case 'sentry.javascript.browser': - case 'sentry.javascript.react': - case 'sentry.javascript.gatsby': - case 'sentry.javascript.ember': - case 'sentry.javascript.vue': - case 'sentry.javascript.angular': - case 'sentry.javascript.angular-ivy': - case 'sentry.javascript.nextjs': - case 'sentry.javascript.nuxt': - case 'sentry.javascript.electron': - case 'sentry.javascript.remix': - case 'sentry.javascript.svelte': - case 'sentry.javascript.sveltekit': - case 'sentry.javascript.react-native': - case 'sentry.javascript.astro': - return false; - case undefined: - return true; - default: - return true; - } -} - -export function isJavascriptSDKEvent(value: TraceTree.NodeValue): boolean { - return ( - !!value && - 'sdk_name' in value && - /javascript|angular|astro|backbone|ember|gatsby|nextjs|react|remix|svelte|vue/.test( - value.sdk_name - ) - ); -} - -export function isBrowserRequestNode(node: BaseNode): boolean { - return ( - // Adjust for SDK changes in https://github.com/getsentry/sentry-javascript/pull/13527 - node.op === 'browser.request' || - (node.op === 'browser' && node.description === 'request') - ); -} - -export function isTraceOccurence( - issue: TraceTree.TraceIssue -): issue is TraceTree.TraceOccurrence { - return 'issue_id' in issue && issue.event_type !== 'error'; -} - -export function isEAPTraceOccurrence( - issue: TraceTree.TraceIssue -): issue is TraceTree.EAPOccurrence { - return ( - isTraceOccurence(issue) && 'event_type' in issue && issue.event_type === 'occurrence' - ); -} - -export function isEAPMeasurementValue( - value: number | Measurement | undefined -): value is number { - return value !== undefined && typeof value === 'number'; -} - -export function isEAPMeasurements( - value: Record<string, Measurement> | Record<string, number> | undefined -): value is Record<string, number> { - if (value === undefined) { - return false; - } - - return Object.values(value).every(isEAPMeasurementValue); -} - -export function isStandaloneSpanMeasurementNode(node: BaseNode): boolean { - if (node.value && 'op' in node.value && node.value.op) { - if ( - node.value.op.startsWith('ui.webvital.') || - node.value.op.startsWith('ui.interaction.') - ) { - return true; - } - } - - return false; -} - -export function isRootEvent(value: TraceTree.NodeValue): boolean { - // Root events has no parent_span_id - return !!value && 'parent_span_id' in value && value.parent_span_id === null; -} - -export function isTraceSplitResult( - result: TraceTree.Trace -): result is TraceSplitResults<TraceTree.Transaction> { - return 'transactions' in result && 'orphan_errors' in result; -} diff --git a/static/app/views/performance/newTraceDetails/traceHeader/index.tsx b/static/app/views/performance/newTraceDetails/traceHeader/index.tsx index 0104abe455efe7..ebc10acf889dc8 100644 --- a/static/app/views/performance/newTraceDetails/traceHeader/index.tsx +++ b/static/app/views/performance/newTraceDetails/traceHeader/index.tsx @@ -16,7 +16,6 @@ import {useModuleURLBuilder} from 'sentry/views/insights/common/utils/useModuleU import {useDomainViewFilters} from 'sentry/views/insights/pages/useFilters'; import type {TraceMetaQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceMeta'; import type {TraceRootEventQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceRootEvent'; -import {getRepresentativeTraceEvent} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; import Highlights from 'sentry/views/performance/newTraceDetails/traceHeader/highlights'; import {PlaceHolder} from 'sentry/views/performance/newTraceDetails/traceHeader/placeholder'; import Projects from 'sentry/views/performance/newTraceDetails/traceHeader/projects'; @@ -64,12 +63,12 @@ export function TraceMetaDataHeader(props: TraceMetadataHeaderProps) { return <PlaceHolder organization={props.organization} traceSlug={props.traceSlug} />; } - const rep = getRepresentativeTraceEvent(props.tree, props.logs); + const rep = props.tree.findRepresentativeTraceNode({logs: props.logs}); const project = projects.find(p => { const id = - rep.event && OurLogKnownFieldKey.PROJECT_ID in rep.event + rep?.event && OurLogKnownFieldKey.PROJECT_ID in rep.event ? rep.event[OurLogKnownFieldKey.PROJECT_ID] - : rep.event?.project_id; + : rep?.event?.projectId; return p.id === String(id); }); diff --git a/static/app/views/performance/newTraceDetails/traceHeader/meta.tsx b/static/app/views/performance/newTraceDetails/traceHeader/meta.tsx index 7d9e06c6244660..66173376064a8f 100644 --- a/static/app/views/performance/newTraceDetails/traceHeader/meta.tsx +++ b/static/app/views/performance/newTraceDetails/traceHeader/meta.tsx @@ -6,15 +6,14 @@ import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; import type {Organization} from 'sentry/types/organization'; import getDuration from 'sentry/utils/duration/getDuration'; -import type {OurLogsResponseItem} from 'sentry/views/explore/logs/types'; +import { + OurLogKnownFieldKey, + type OurLogsResponseItem, +} from 'sentry/views/explore/logs/types'; import type {TraceMetaQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceMeta'; -import type {RepresentativeTraceEvent} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; import {TraceDrawerComponents} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/styles'; -import { - isEAPError, - isTraceError, -} from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; +import type {BaseNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; import {useTraceQueryParams} from 'sentry/views/performance/newTraceDetails/useTraceQueryParams'; type MetaDataProps = { @@ -52,21 +51,23 @@ interface MetaProps { logs: OurLogsResponseItem[] | undefined; meta: TraceMetaQueryResults['data']; organization: Organization; - representativeEvent: RepresentativeTraceEvent; + representativeEvent: TraceTree.RepresentativeTraceEvent | null; tree: TraceTree; } -function getRootDuration(event: TraceTree.TraceEvent | null) { - if (!event || isEAPError(event) || isTraceError(event)) { +function getRootDuration(node: BaseNode | null) { + if (!node) { return '\u2014'; } - return getDuration( - ('timestamp' in event ? event.timestamp : event.end_timestamp) - - event.start_timestamp, - 2, - true - ); + const startTimestamp = node.startTimestamp; + const endTimestamp = node.endTimestamp; + + if (!startTimestamp || !endTimestamp) { + return '\u2014'; + } + + return getDuration(endTimestamp - startTimestamp, 2, true); } export function Meta(props: MetaProps) { @@ -88,6 +89,8 @@ export function Meta(props: MetaProps) { const hasSpans = spansCount > 0; const hasLogs = (props.logs?.length ?? 0) > 0; + const repEvent = props.representativeEvent?.event; + return ( <MetaWrapper> <MetaSection @@ -120,9 +123,13 @@ export function Meta(props: MetaProps) { <MetaSection headingText={t('Root Duration')} rightAlignBody - bodyText={getRootDuration( - props.representativeEvent.event as TraceTree.TraceEvent - )} + bodyText={ + repEvent + ? OurLogKnownFieldKey.PROJECT_ID in repEvent + ? '\u2014' // Logs don't have a duration + : getRootDuration(repEvent) + : '\u2014' + } /> ) : hasLogs ? ( <MetaSection diff --git a/static/app/views/performance/newTraceDetails/traceHeader/title.tsx b/static/app/views/performance/newTraceDetails/traceHeader/title.tsx index 90375f515401a4..3863482c004c15 100644 --- a/static/app/views/performance/newTraceDetails/traceHeader/title.tsx +++ b/static/app/views/performance/newTraceDetails/traceHeader/title.tsx @@ -11,28 +11,21 @@ import useOrganization from 'sentry/utils/useOrganization'; import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types'; import {Divider} from 'sentry/views/issueDetails/divider'; import type {TraceRootEventQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceRootEvent'; -import { - isTraceItemDetailsResponse, - type RepresentativeTraceEvent, -} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; +import {isTraceItemDetailsResponse} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; import {findSpanAttributeValue} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/utils'; -import { - isEAPError, - isTraceError, - isUptimeCheck, -} from 'sentry/views/performance/newTraceDetails/traceGuards'; +import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import {makeReplaysPathname} from 'sentry/views/replays/pathnames'; interface TitleProps { - representativeEvent: RepresentativeTraceEvent; + representativeEvent: TraceTree.RepresentativeTraceEvent | null; rootEventResults: TraceRootEventQueryResults; } -function getTitle(representativeEvent: RepresentativeTraceEvent): { - subtitle: string | undefined; +function getTitle(representativeEvent: TraceTree.RepresentativeTraceEvent | null): { title: string; + subtitle?: string; } | null { - const {event} = representativeEvent; + const event = representativeEvent?.event; if (!event) { return null; } @@ -45,36 +38,7 @@ function getTitle(representativeEvent: RepresentativeTraceEvent): { }; } - // Handle error events - if (isEAPError(event) || isTraceError(event)) { - const subtitle = isEAPError(event) ? event.description : event.title || event.message; - - return { - title: t('Trace'), - subtitle, - }; - } - - // Handle uptime check traces - if (isUptimeCheck(event)) { - return { - title: t('Uptime Monitor Check'), - subtitle: `${event.additional_attributes?.method} ${event.additional_attributes?.request_url}`, - }; - } - - if (!('transaction' in event)) { - return null; - } - - // Normalize operation field access across event types - const op = - 'transaction.op' in event ? event['transaction.op'] : 'op' in event ? event.op : ''; - - return { - title: op || t('Trace'), - subtitle: event.transaction, - }; + return event.traceHeaderTitle ?? null; } function ContextBadges({rootEventResults}: Pick<TitleProps, 'rootEventResults'>) { diff --git a/static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.spec.tsx b/static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.spec.tsx index 64f07b3260d54c..f621251a4fbde4 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.spec.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.spec.tsx @@ -3,8 +3,6 @@ import {OrganizationFixture} from 'sentry-fixture/organization'; import {EntryType} from 'sentry/types/event'; import { isSpanNode, - isTraceErrorNode, - isTraceNode, isTransactionNode, } from 'sentry/views/performance/newTraceDetails/traceGuards'; import {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; @@ -61,13 +59,6 @@ function mockSpansResponse( }); } -function hasErrors(n: BaseNode): boolean { - return ( - (isTraceErrorNode(n) || n.errors.size > 0 || n.occurrences.size > 0) && - !isTraceNode(n) - ); -} - describe('IssuesTraceTree', () => { it('collapsed nodes without errors', () => { const tree = IssuesTraceTree.FromTrace(traceWithErrorInMiddle, { @@ -76,7 +67,7 @@ describe('IssuesTraceTree', () => { organization, }); - const issues = tree.root.findAllChildren(n => hasErrors(n)); + const issues = tree.root.children[0]!.findAllChildren(n => n.hasIssues); expect(tree.build().collapseList(issues, 3, 0).serialize()).toMatchSnapshot(); }); @@ -87,11 +78,11 @@ describe('IssuesTraceTree', () => { organization, }); - const error = tree.root.findChild(n => hasErrors(n)); + const error = tree.root.children[0]!.findChild(n => n.hasIssues); let node = error; const nodes: Array<BaseNode<any>> = []; - while (node && !isTraceNode(node)) { + while (node) { nodes.push(node); node = node.parent; } @@ -107,7 +98,13 @@ describe('IssuesTraceTree', () => { organization, }); - const errors = tree.root.findAllChildren(n => hasErrors(n)).slice(0, 10); + const errors = tree.root + .findAllChildren( + n => + n.hasIssues && + !!(n.value && 'event_type' in n.value && n.value.event_type === 'error') + ) + .slice(0, 10); expect(tree.build().collapseList(errors, 3, 0).serialize()).toMatchSnapshot(); }); @@ -118,7 +115,7 @@ describe('IssuesTraceTree', () => { organization, }); - const issues = tree.root.findAllChildren(n => hasErrors(n)); + const issues = tree.root.children[0]!.findAllChildren(n => n.hasIssues); // Test with default value (3) const defaultCollapsed = tree.build().collapseList(issues, 3, 0).serialize(); @@ -139,7 +136,7 @@ describe('IssuesTraceTree', () => { organization, }); - const issues = tree.root.findAllChildren(n => hasErrors(n)); + const issues = tree.root.children[0]!.findAllChildren(n => n.hasIssues); // Test with default minShownNodes value const defaultMinShown = tree.build().collapseList(issues, 0, 3).serialize(); diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTree.measurements.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTree.measurements.tsx index 7782b59bfbb27d..da39930b452575 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTree.measurements.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTree.measurements.tsx @@ -1,10 +1,5 @@ import type {Measurement} from 'sentry/types/event'; import {MobileVital, WebVital} from 'sentry/utils/fields'; -import { - isEAPMeasurements, - isEAPMeasurementValue, - isStandaloneSpanMeasurementNode, -} from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {BaseNode} from './traceTreeNode/baseNode'; import type {TraceTree} from './traceTree'; @@ -91,7 +86,7 @@ export function collectTraceMeasurements( tree: TraceTree, node: BaseNode, start_timestamp: number, - measurements: Record<string, Measurement> | Record<string, number> | undefined, + measurements: Record<string, Measurement> | undefined, vitals: Map<BaseNode, TraceTree.CollectedVital[]>, vital_types: Set<'web' | 'mobile'> ): TraceTree.Indicator[] { @@ -101,12 +96,10 @@ export function collectTraceMeasurements( const indicators: TraceTree.Indicator[] = []; - for (const measurement of COLLECTABLE_MEASUREMENTS) { - const value = isEAPMeasurements(measurements) - ? measurements[`measurements.${measurement}`] - : measurements[measurement]; + for (const collectableMeasurement of COLLECTABLE_MEASUREMENTS) { + const measurement = measurements[collectableMeasurement]; - if (!value || (!isEAPMeasurementValue(value) && typeof value.value !== 'number')) { + if (!measurement || typeof measurement.value !== 'number' || !measurement.value) { continue; } @@ -114,19 +107,19 @@ export function collectTraceMeasurements( vitals.set(node, []); } - if (WEB_VITALS_LOOKUP.has(measurement)) { + if (WEB_VITALS_LOOKUP.has(collectableMeasurement)) { vital_types.add('web'); - } else if (MOBILE_VITALS_LOOKUP.has(measurement)) { + } else if (MOBILE_VITALS_LOOKUP.has(collectableMeasurement)) { vital_types.add('mobile'); } - const eapScoreRatioKey = `measurements.score.ratio.${measurement}`; - const legacyScoreKey = `score.${measurement}`; - const legacyScoreWeightKey = `score.weight.${measurement}`; - const score = isEAPMeasurements(measurements) - ? measurements[eapScoreRatioKey] === undefined + const eapScoreKey = `score.ratio.${collectableMeasurement}`; + const legacyScoreKey = `score.${collectableMeasurement}`; + const legacyScoreWeightKey = `score.weight.${collectableMeasurement}`; + const score = node.isEAPEvent + ? measurements[eapScoreKey]?.value === undefined ? undefined - : Math.round(measurements[eapScoreRatioKey] * 100) + : Math.round(measurements[eapScoreKey].value * 100) : measurements[legacyScoreKey]?.value !== undefined && measurements[legacyScoreWeightKey]?.value !== undefined ? Math.round( @@ -137,15 +130,15 @@ export function collectTraceMeasurements( : undefined; vitals.get(node)!.push({ - key: measurement, - measurement: isEAPMeasurementValue(value) ? {value} : value, + key: collectableMeasurement, + measurement, score, }); const hasSeenMeasurement = tree.indicators.some( - indicator => indicator.type === measurement + indicator => indicator.type === collectableMeasurement ); - if (!RENDERABLE_MEASUREMENTS[measurement] || hasSeenMeasurement) { + if (!RENDERABLE_MEASUREMENTS[collectableMeasurement] || hasSeenMeasurement) { continue; } @@ -153,27 +146,37 @@ export function collectTraceMeasurements( // We pass in 0 as the measurement value to prevent applying any unnecessary offset. const timestamp = traceMeasurementToTimestamp( start_timestamp, - isStandaloneSpanMeasurementNode(node) - ? 0 - : isEAPMeasurementValue(value) - ? value - : value.value, - isEAPMeasurementValue(value) ? 'millisecond' : (value.unit ?? 'millisecond') + isStandaloneSpanMeasurementNode(node) ? 0 : measurement.value, + measurement.unit ?? 'millisecond' ); indicators.push({ start: timestamp, duration: 0, - measurement: isEAPMeasurementValue(value) ? {value} : value, - poor: MEASUREMENT_THRESHOLDS[measurement] - ? (isEAPMeasurementValue(value) ? value : value.value) > - MEASUREMENT_THRESHOLDS[measurement] + measurement, + poor: MEASUREMENT_THRESHOLDS[collectableMeasurement] + ? measurement.value > MEASUREMENT_THRESHOLDS[collectableMeasurement] : false, - type: measurement, - label: (MEASUREMENT_ACRONYM_MAPPING[measurement] ?? measurement).toUpperCase(), + type: collectableMeasurement, + label: ( + MEASUREMENT_ACRONYM_MAPPING[collectableMeasurement] ?? collectableMeasurement + ).toUpperCase(), score, }); } return indicators; } + +function isStandaloneSpanMeasurementNode(node: BaseNode): boolean { + if (node.value && 'op' in node.value && node.value.op) { + if ( + node.value.op.startsWith('ui.webvital.') || + node.value.op.startsWith('ui.interaction.') + ) { + return true; + } + } + + return false; +} diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx index 2d2a77ba1befd0..982adb8e2335a7 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx @@ -9,12 +9,12 @@ import { isSiblingAutogroupedNode, isSpanNode, isTransactionNode, - isUptimeCheckNode, } from './../traceGuards'; import type {BaseNode} from './traceTreeNode/baseNode'; import type {EapSpanNode} from './traceTreeNode/eapSpanNode'; import type {ParentAutogroupNode} from './traceTreeNode/parentAutogroupNode'; import type {SiblingAutogroupNode} from './traceTreeNode/siblingAutogroupNode'; +import type {UptimeCheckNode} from './traceTreeNode/uptimeCheckNode'; import type {UptimeCheckTimingNode} from './traceTreeNode/uptimeCheckTimingNode'; import {TraceShape, TraceTree} from './traceTree'; import { @@ -81,16 +81,6 @@ const traceWithEventId = makeTrace({ ], }); -const traceWithVitals = makeTrace({ - transactions: [ - makeTransaction({ - start_timestamp: start, - timestamp: start + 2, - measurements: {ttfb: {value: 0, unit: 'millisecond'}}, - }), - ], -}); - const traceWithOrphanError = makeTrace({ transactions: [ makeTransaction({ @@ -406,9 +396,37 @@ describe('TraceTree', () => { describe('indicators', () => { it('measurements are converted to indicators', () => { - const tree = TraceTree.FromTrace(traceWithVitals, traceOptions); + const measurementValue = 1; + const tree = TraceTree.FromTrace( + makeTrace({ + transactions: [ + makeTransaction({ + start_timestamp: start, + timestamp: start + 2, + measurements: {ttfb: {value: measurementValue, unit: 'millisecond'}}, + }), + ], + }), + traceOptions + ); expect(tree.indicators).toHaveLength(1); - expect(tree.indicators[0]!.start).toBe(start * 1e3); + expect(tree.indicators[0]!.start).toBe(start * 1e3 + measurementValue); + }); + + it('zero measurements are not converted to indicators', () => { + const tree = TraceTree.FromTrace( + makeTrace({ + transactions: [ + makeTransaction({ + start_timestamp: start, + timestamp: start + 2, + measurements: {ttfb: {value: 0, unit: 'millisecond'}}, + }), + ], + }), + traceOptions + ); + expect(tree.indicators).toHaveLength(0); }); it('sorts indicators by start', () => { @@ -2052,6 +2070,14 @@ describe('TraceTree', () => { ); } + function isUptimeCheckNode(node: BaseNode): node is UptimeCheckNode { + return !!( + node.value && + 'event_type' in node.value && + node.value.event_type === 'uptime_check' + ); + } + it('automatically creates timing nodes when uptime check node is created', () => { const uptimeCheck = makeUptimeCheck({ additional_attributes: { diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx index 5e8c3bf77d53d4..07025e9ccf14d2 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx @@ -6,6 +6,8 @@ import type {Client} from 'sentry/api'; import type {RawSpanType} from 'sentry/components/events/interfaces/spans/types'; import type {Level, Measurement} from 'sentry/types/event'; import type {Organization} from 'sentry/types/organization'; +import type {OurLogsResponseItem} from 'sentry/views/explore/logs/types'; +import {TraceItemDataset} from 'sentry/views/explore/types'; import type { TraceError as TraceErrorType, TraceFullDetailed, @@ -14,18 +16,15 @@ import type { } from 'sentry/views/performance/newTraceDetails/traceApi/types'; import {getTraceQueryParams} from 'sentry/views/performance/newTraceDetails/traceApi/useTrace'; import type {TraceMetaQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceMeta'; +import {isTraceSplitResult} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; import { isEAPError, isEAPSpan, - isJavascriptSDKEvent, isMissingInstrumentationNode, isParentAutogroupedNode, - isRootEvent, isSiblingAutogroupedNode, isTraceError, - isTraceSplitResult, isUptimeCheck, - shouldAddMissingInstrumentationSpan, } from 'sentry/views/performance/newTraceDetails/traceGuards'; import { collectTraceMeasurements, @@ -218,11 +217,6 @@ export declare namespace TraceTree { type Trace = TraceSplitResults<Transaction> | EAPTrace; - // Represents events that we get from the trace endpoints and render an - // individual row for in the trace waterfall, on load. This excludes spans as - // they are rendered on-demand as the user zooms in. - type TraceEvent = Transaction | TraceError | EAPSpan | EAPError | UptimeCheck; - type TraceError = TraceErrorType; type TraceErrorIssue = TraceError | EAPError; @@ -231,6 +225,11 @@ export declare namespace TraceTree { type TraceIssue = TraceErrorIssue | TraceOccurrence; + type RepresentativeTraceEvent = { + dataset: TraceItemDataset | null; + event: BaseNode | OurLogsResponseItem | null; + }; + type Profile = {profile_id: string} | {profiler_id: string}; type Project = { slug: string; @@ -806,7 +805,7 @@ export class TraceTree extends TraceTreeEventDispatcher { while ( tail && tail.children.length === 1 && - (tail.children[0]!.canAutogroup || tail.children[0]!.canAutogroup) && + tail.children[0]!.canAutogroup && // skip `op: default` spans as `default` is added to op-less spans: tail.children[0]!.op !== 'default' && tail.children[0]!.op === head.op @@ -828,6 +827,18 @@ export class TraceTree extends TraceTreeEventDispatcher { continue; } + if (!node.parent) { + throw new Error('Parent node is missing, this should be unreachable code'); + } + + // Check for direct visible children first, this helps respect the expanded state of the node in concern. + const children = node.parent.children; + + const index = children.indexOf(node); + if (index === -1) { + throw new Error('Node is not a child of its parent'); + } + const autoGroupedNode = new ParentAutogroupNode( node.parent, { @@ -845,21 +856,6 @@ export class TraceTree extends TraceTreeEventDispatcher { ); autogroupCount++; - - if (!node.parent) { - throw new Error('Parent node is missing, this should be unreachable code'); - } - - // Check for direct visible children first, this helps respect the expanded state of the node in concern. - const children = - node.parent.directVisibleChildren.length > 0 - ? node.parent.directVisibleChildren - : node.parent.children; - - const index = children.indexOf(node); - if (index === -1) { - throw new Error('Node is not a child of its parent'); - } children[index] = autoGroupedNode; autoGroupedNode.head.parent = autoGroupedNode; @@ -1326,6 +1322,64 @@ export class TraceTree extends TraceTreeEventDispatcher { throw new Error('Not a valid trace'); } + findRepresentativeTraceNode({logs}: {logs: OurLogsResponseItem[] | undefined}): { + dataset: TraceItemDataset | null; + event: BaseNode | OurLogsResponseItem | null; + } | null { + const hasLogs = logs && logs.length > 0; + if (this.type === 'empty' && hasLogs) { + return { + event: logs[0]!, + dataset: TraceItemDataset.LOGS, + }; + } + + const traceNode = this.root.children[0]; + + if (this.type !== 'trace' || !traceNode) { + return null; + } + + let preferredRootEvent: BaseNode | null = null; + let firstRootEvent: BaseNode | null = null; + let candidateEvent: BaseNode | null = null; + let firstEvent: BaseNode | null = null; + + for (const node of traceNode.children) { + if (isRootEvent(node.value)) { + if (!firstRootEvent) { + firstRootEvent = node; + } + + if (hasPreferredOp(node)) { + preferredRootEvent = node; + break; + } + // Otherwise we keep looking for a root eap transaction. If we don't find one, we use other roots, like standalone spans. + continue; + } else if ( + // If we haven't found a root transaction, but we found a candidate transaction + // with an op that we care about, we can use it for the title. We keep looking for + // a root. + !candidateEvent && + hasPreferredOp(node) + ) { + candidateEvent = node; + continue; + } else if (!firstEvent) { + // If we haven't found a root or candidate transaction, we can use the first transaction + // in the trace for the title. + firstEvent = node; + } + } + + const event = preferredRootEvent ?? firstRootEvent ?? candidateEvent ?? firstEvent; + return { + event, + dataset: event?.traceItemDataset ?? null, + }; + } + fetchAdditionalTraces(options: { api: Client; filters: any; @@ -1486,3 +1540,62 @@ function traceQueueIterator( } } } + +const CANDIDATE_TRACE_TITLE_OPS = ['pageload', 'navigation', 'ui.load']; + +/** + * Prefer "special" root events over generic root events when generating a title + * for the waterfall view. Picking these improves contextual navigation for linked + * traces, resulting in more meaningful waterfall titles. + */ +function hasPreferredOp(node: BaseNode): boolean { + const op = node.op; + return !!op && CANDIDATE_TRACE_TITLE_OPS.includes(op); +} + +function shouldAddMissingInstrumentationSpan(sdk: string | undefined): boolean { + if (!sdk) { + return true; + } + if (sdk.length < 'sentry.javascript.'.length) { + return true; + } + + switch (sdk.toLowerCase()) { + case 'sentry.javascript.browser': + case 'sentry.javascript.react': + case 'sentry.javascript.gatsby': + case 'sentry.javascript.ember': + case 'sentry.javascript.vue': + case 'sentry.javascript.angular': + case 'sentry.javascript.angular-ivy': + case 'sentry.javascript.nextjs': + case 'sentry.javascript.nuxt': + case 'sentry.javascript.electron': + case 'sentry.javascript.remix': + case 'sentry.javascript.svelte': + case 'sentry.javascript.sveltekit': + case 'sentry.javascript.react-native': + case 'sentry.javascript.astro': + return false; + case undefined: + return true; + default: + return true; + } +} + +function isJavascriptSDKEvent(value: TraceTree.NodeValue): boolean { + return ( + !!value && + 'sdk_name' in value && + /javascript|angular|astro|backbone|ember|gatsby|nextjs|react|remix|svelte|vue/.test( + value.sdk_name + ) + ); +} + +function isRootEvent(value: TraceTree.NodeValue): boolean { + // Root events has no parent_span_id + return !!value && 'parent_span_id' in value && value.parent_span_id === null; +} diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx index 93cbe40e4b1b5d..ba5e3865c917ae 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode.tsx @@ -4,10 +4,11 @@ import type {Client} from 'sentry/api'; import {pickBarColor} from 'sentry/components/performance/waterfall/utils'; import type {Measurement} from 'sentry/types/event'; import type {Organization} from 'sentry/types/organization'; +import type {TraceItemDataset} from 'sentry/views/explore/types'; import type {TraceMetaQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceMeta'; import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails'; import { - isEAPTransactionNode, + isEAPSpanNode, isTransactionNode, } from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; @@ -123,6 +124,11 @@ export abstract class BaseNode<T extends TraceTree.NodeValue = TraceTree.NodeVal */ isEAPEvent = false; + /** + * The dataset used to fetch the details for the item. If not provided we fetch from nodestore. + */ + traceItemDataset: TraceItemDataset | null = null; + /** * The priority of the node in when we find multiple nodes matching the same search query. */ @@ -327,10 +333,25 @@ export abstract class BaseNode<T extends TraceTree.NodeValue = TraceTree.NodeVal : undefined; } - get measurements(): Record<string, number> | Record<string, Measurement> | undefined { - return this.value && 'measurements' in this.value - ? this.value.measurements - : undefined; + private _isValidMeasurements( + measurements: Record<string, any> + ): measurements is Record<string, Measurement> { + return Object.values(measurements).every( + m => m && 'value' in m && typeof m.value === 'number' + ); + } + + get measurements(): Record<string, Measurement> | undefined { + if ( + this.value && + 'measurements' in this.value && + this.value.measurements && + this._isValidMeasurements(this.value.measurements) + ) { + return this.value.measurements; + } + + return undefined; } get attributes(): Record<string, string | number | boolean> | undefined { @@ -364,12 +385,7 @@ export abstract class BaseNode<T extends TraceTree.NodeValue = TraceTree.NodeVal occurrence => occurrence.event_id === id ); - return ( - this.id === id || - this.transactionId === id || - hasMatchingErrors || - hasMatchingOccurrences - ); + return this.id === id || hasMatchingErrors || hasMatchingOccurrences; } invalidate() { @@ -471,7 +487,7 @@ export abstract class BaseNode<T extends TraceTree.NodeValue = TraceTree.NodeVal } findParentEapTransaction(): EapSpanNode | null { - return this.findParent<EapSpanNode>(p => isEAPTransactionNode(p)); + return this.findParent<EapSpanNode>(p => isEAPSpanNode(p) && p.value.is_transaction); } expand(expanding: boolean, tree: TraceTree): boolean { diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx index 4d8184cf254e1f..a7f060b5e23886 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/eapSpanNode.tsx @@ -2,14 +2,12 @@ import type {Theme} from '@emotion/react'; import {pickBarColor} from 'sentry/components/performance/waterfall/utils'; import {t} from 'sentry/locale'; +import type {Measurement} from 'sentry/types/event'; +import {TraceItemDataset} from 'sentry/views/explore/types'; +import {isBrowserRequestNode} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; import {EAPSpanNodeDetails} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/span'; import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails'; -import { - isBrowserRequestNode, - isEAPSpan, - isEAPSpanNode, - isEAPTransaction, -} from 'sentry/views/performance/newTraceDetails/traceGuards'; +import {isEAPSpanNode} from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import {TraceEAPSpanRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceEAPSpanRow'; import type {TraceRowProps} from 'sentry/views/performance/newTraceDetails/traceRow/traceRow'; @@ -35,13 +33,14 @@ export class EapSpanNode extends BaseNode<TraceTree.EAPSpan> { // For eap transactions, on load we only display the embedded transactions children. // Mimics the behavior of non-eap traces, enabling a less noisy/summarized view of the trace const parentNode = value.is_transaction - ? (parent?.findParent(p => isEAPTransaction(p.value)) ?? parent) + ? (parent?.findParent(p => isEAPSpanNode(p) && p.value.is_transaction) ?? parent) : parent; super(parentNode, value, extra); this.id = value.event_id; this.type = 'span'; + this.traceItemDataset = TraceItemDataset.SPANS; this.searchPriority = this.value.is_transaction ? 1 : 2; this.isEAPEvent = true; @@ -96,6 +95,22 @@ export class EapSpanNode extends BaseNode<TraceTree.EAPSpan> { return this.op + (this.description ? ' - ' + this.description : ''); } + get measurements(): Record<string, Measurement> | undefined { + if (!this.value.measurements) { + return undefined; + } + + const result: Record<string, Measurement> = {}; + for (const key in this.value.measurements) { + const value = this.value.measurements[key]; + if (typeof value === 'number') { + const normalizedKey = key.replace('measurements.', ''); + result[normalizedKey] = {value}; + } + } + return result; + } + get profileId(): string | undefined { const profileId = super.profileId; @@ -121,13 +136,11 @@ export class EapSpanNode extends BaseNode<TraceTree.EAPSpan> { } get transactionId(): string | undefined { - const transactionId = super.transactionId; - - if (transactionId) { - return transactionId; - } - - return this.findClosestParentTransaction()?.transactionId; + // If the node represents a transaction, we use the transaction_id attached to the node, + // otherwise we use the transaction_id of the closest parent transaction. + return this.value.is_transaction + ? this.value.transaction_id + : this.findClosestParentTransaction()?.transactionId; } get traceHeaderTitle(): { @@ -141,10 +154,12 @@ export class EapSpanNode extends BaseNode<TraceTree.EAPSpan> { } get directVisibleChildren(): Array<BaseNode<TraceTree.NodeValue>> { - if (isEAPTransaction(this.value) && !this.expanded) { + if (this.value.is_transaction && !this.expanded) { // For collapsed eap-transactions we still render the embedded eap-transactions as visible children. // Mimics the behavior of non-eap traces, enabling a less noisy/summarized view of the trace - return this.children.filter(child => isEAPTransaction(child.value)); + return this.children.filter( + child => isEAPSpanNode(child) && child.value.is_transaction + ); } return super.directVisibleChildren; @@ -181,14 +196,14 @@ export class EapSpanNode extends BaseNode<TraceTree.EAPSpan> { // the eap-spans (by their parent_span_id) that were previously hidden. Note that this only impacts the // direct eap-transaction children of the targetted eap-transaction node. if (this.value.is_transaction) { - const eapTransactions = this.children.filter(c => - isEAPTransaction(c.value) + const eapTransactions = this.children.filter( + c => isEAPSpanNode(c) && c.value.is_transaction ) as EapSpanNode[]; for (const txn of eapTransactions) { // Find the eap-span that is the parent of the transaction const newParent = this.findChild(n => { - if (isEAPSpan(n.value)) { + if (isEAPSpanNode(n)) { return n.value.event_id === txn.value.parent_span_id; } return false; @@ -300,6 +315,13 @@ export class EapSpanNode extends BaseNode<TraceTree.EAPSpan> { ); } + matchById(id: string): boolean { + const superMatch = super.matchById(id); + + // Match by transaction_id if the node represents a transaction, otherwise use the super match. + return superMatch || (this.value.is_transaction ? id === this.transactionId : false); + } + resolveValueFromSearchKey(key: string): any | null { // @TODO Abdullah Khan: Add EAPSpanNode support for exclusive_time if (['duration', 'span.duration', 'span.total_time'].includes(key)) { diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/traceNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/traceNode.tsx index c0e7baf0ffead8..3d485682fe3efc 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/traceNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/traceNode.tsx @@ -1,5 +1,5 @@ +import {isTraceSplitResult} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails'; -import {isTraceSplitResult} from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import {TraceRootRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceRootNode'; import type {TraceRowProps} from 'sentry/views/performance/newTraceDetails/traceRow/traceRow'; @@ -21,7 +21,7 @@ export class TraceNode extends BaseNode<TraceTree.Trace> { this.canShowDetails = false; this.id = 'root'; this.type = 'trace'; - + this.isEAPEvent = !isTraceSplitResult(this.value); this.parent?.children.push(this); } diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/transactionNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/transactionNode.tsx index 22a3043ef63e9d..325665c55671ba 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/transactionNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/transactionNode.tsx @@ -7,12 +7,10 @@ import {t} from 'sentry/locale'; import type {EventTransaction} from 'sentry/types/event'; import type {Organization} from 'sentry/types/organization'; import {getStylingSliceName} from 'sentry/views/explore/tables/tracesTable/utils'; +import {isBrowserRequestNode} from 'sentry/views/performance/newTraceDetails/traceApi/utils'; import {TransactionNodeDetails} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/transaction'; import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails'; -import { - isBrowserRequestNode, - isTransactionNode, -} from 'sentry/views/performance/newTraceDetails/traceGuards'; +import {isTransactionNode} from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import type {TraceRowProps} from 'sentry/views/performance/newTraceDetails/traceRow/traceRow'; import {TraceTransactionRow} from 'sentry/views/performance/newTraceDetails/traceRow/traceTransactionRow'; diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode.tsx index 9d6904640ac45b..fab32562b8d46f 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode/uptimeCheckNode.tsx @@ -3,6 +3,7 @@ import type {Theme} from '@emotion/react'; import {pickBarColor} from 'sentry/components/performance/waterfall/utils'; import {t} from 'sentry/locale'; import {uniqueId} from 'sentry/utils/guid'; +import {TraceItemDataset} from 'sentry/views/explore/types'; import {UptimeNodeDetails} from 'sentry/views/performance/newTraceDetails/traceDrawer/details/uptime'; import type {TraceTreeNodeDetailsProps} from 'sentry/views/performance/newTraceDetails/traceDrawer/tabs/traceTreeNodeDetails'; import type {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; @@ -28,6 +29,7 @@ export class UptimeCheckNode extends BaseNode<TraceTree.UptimeCheck> { this.isEAPEvent = true; this.id = this.value.event_id; this.type = 'uptime-check'; + this.traceItemDataset = TraceItemDataset.UPTIME_RESULTS; this.parent?.children.push(this); } diff --git a/static/app/views/performance/newTraceDetails/traceRenderers/virtualizedViewManager.tsx b/static/app/views/performance/newTraceDetails/traceRenderers/virtualizedViewManager.tsx index 17f81edd50a5d4..2d68d585ad2dcd 100644 --- a/static/app/views/performance/newTraceDetails/traceRenderers/virtualizedViewManager.tsx +++ b/static/app/views/performance/newTraceDetails/traceRenderers/virtualizedViewManager.tsx @@ -9,7 +9,6 @@ import { cancelAnimationTimeout, requestAnimationTimeout, } from 'sentry/utils/profiling/hooks/useVirtualizedTree/virtualizedTreeUtils'; -import {isEAPError} from 'sentry/views/performance/newTraceDetails/traceGuards'; import {TraceTree} from 'sentry/views/performance/newTraceDetails/traceModels/traceTree'; import type {BaseNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; import {TraceRowWidthMeasurer} from 'sentry/views/performance/newTraceDetails/traceRenderers/traceRowWidthMeasurer'; @@ -1715,7 +1714,7 @@ function getIconTimestamps( } for (const err of node.errors) { - const timestamp = isEAPError(err) ? err.start_timestamp : err.timestamp; + const timestamp = 'start_timestamp' in err ? err.start_timestamp : err.timestamp; if (typeof timestamp === 'number') { min_icon_timestamp = Math.min(min_icon_timestamp, timestamp * 1e3 - icon_width); max_icon_timestamp = Math.max(max_icon_timestamp, timestamp * 1e3 + icon_width); diff --git a/static/app/views/performance/newTraceDetails/traceRow/traceCollapsedRow.tsx b/static/app/views/performance/newTraceDetails/traceRow/traceCollapsedRow.tsx index cd2bbfbd4d59ed..d700d53ef7856e 100644 --- a/static/app/views/performance/newTraceDetails/traceRow/traceCollapsedRow.tsx +++ b/static/app/views/performance/newTraceDetails/traceRow/traceCollapsedRow.tsx @@ -3,7 +3,8 @@ import {useMemo} from 'react'; import {t} from 'sentry/locale'; import { isCollapsedNode, - isTraceErrorNode, + isEAPError, + isTraceError, } from 'sentry/views/performance/newTraceDetails/traceGuards'; import type {BaseNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; import type {CollapsedNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/collapsedNode'; @@ -25,7 +26,7 @@ export function TraceCollapsedRow(props: TraceRowProps<CollapsedNode>) { seen.add(c); - if (isTraceErrorNode(c)) { + if (isTraceError(c.value) || isEAPError(c.value)) { childStatistics.issues++; } else { childStatistics.events++; diff --git a/static/app/views/performance/newTraceDetails/traceRow/traceIcons.tsx b/static/app/views/performance/newTraceDetails/traceRow/traceIcons.tsx index a7b710898e0db8..16feb7d15372d5 100644 --- a/static/app/views/performance/newTraceDetails/traceRow/traceIcons.tsx +++ b/static/app/views/performance/newTraceDetails/traceRow/traceIcons.tsx @@ -1,7 +1,6 @@ import {Fragment, useMemo} from 'react'; import clamp from 'lodash/clamp'; -import {isEAPError} from 'sentry/views/performance/newTraceDetails/traceGuards'; import {TraceIcons} from 'sentry/views/performance/newTraceDetails/traceIcons'; import type {BaseNode} from 'sentry/views/performance/newTraceDetails/traceModels/traceTreeNode/baseNode'; import type {VirtualizedViewManager} from 'sentry/views/performance/newTraceDetails/traceRenderers/virtualizedViewManager'; @@ -24,7 +23,8 @@ export function TraceErrorIcons(props: ErrorIconsProps) { return ( <Fragment> {errors.map((error, i) => { - const timestamp = isEAPError(error) ? error.start_timestamp : error.timestamp; + const timestamp = + 'start_timestamp' in error ? error.start_timestamp : error.timestamp; // Clamp the error timestamp to the span's timestamp const left = props.manager.computeRelativeLeftPositionFromOrigin( clamp(