diff --git a/static/app/views/performance/newTraceDetails/issuesTraceWaterfall.tsx b/static/app/views/performance/newTraceDetails/issuesTraceWaterfall.tsx index 8defb620ec2cd5..2e6c17e43cbaa7 100644 --- a/static/app/views/performance/newTraceDetails/issuesTraceWaterfall.tsx +++ b/static/app/views/performance/newTraceDetails/issuesTraceWaterfall.tsx @@ -19,6 +19,9 @@ import useOrganization from 'sentry/utils/useOrganization'; import useProjects from 'sentry/utils/useProjects'; import {IssueTraceWaterfallOverlay} from 'sentry/views/performance/newTraceDetails/issuesTraceWaterfallOverlay'; import { + isEAPSpanNode, + isEAPTransactionNode, + isNonTransactionEAPSpanNode, isSpanNode, isTraceErrorNode, isTransactionNode, @@ -200,8 +203,9 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) { } } } - if (isSpanNode(n)) { - if (n.value.span_id === props.event.eventID) { + if (isSpanNode(n) || isEAPSpanNode(n)) { + const spanId = 'span_id' in n.value ? n.value.span_id : n.value.event_id; + if (spanId === props.event.eventID) { return true; } for (const e of n.errors) { @@ -223,8 +227,8 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) { // the error may have been attributed to, otherwise we look at the transaction. const node = nodes?.find(n => isTraceErrorNode(n)) || - nodes?.find(n => isSpanNode(n)) || - nodes?.find(n => isTransactionNode(n)); + nodes?.find(n => isSpanNode(n) || isNonTransactionEAPSpanNode(n)) || + nodes?.find(n => isTransactionNode(n) || isEAPTransactionNode(n)); const index = node ? props.tree.list.indexOf(node) : -1; 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 e66940b2ddded9..b5c058f50feff8 100644 --- a/static/app/views/performance/newTraceDetails/traceDrawer/details/issues/issues.tsx +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/issues/issues.tsx @@ -19,11 +19,7 @@ import {t, tct, tn} from 'sentry/locale'; import {space} from 'sentry/styles/space'; import type {Group} from 'sentry/types/group'; import type {Organization} from 'sentry/types/organization'; -import type { - TraceError, - TraceErrorOrIssue, - TracePerformanceIssue, -} from 'sentry/utils/performance/quickTrace/types'; +import type {TracePerformanceIssue} from 'sentry/utils/performance/quickTrace/types'; import {useApiQuery} from 'sentry/utils/queryClient'; import type RequestError from 'sentry/utils/requestError/requestError'; import useOrganization from 'sentry/utils/useOrganization'; @@ -40,7 +36,7 @@ import {TraceDrawerComponents} from '../styles'; import {IssueSummary} from './issueSummary'; type IssueProps = { - issue: TraceErrorOrIssue; + issue: TraceTree.TraceIssue; organization: Organization; }; @@ -63,7 +59,10 @@ const issueOrderPriority: Record = { unknown: 6, }; -function sortIssuesByLevel(a: TraceError, b: TraceError): number { +function sortIssuesByLevel( + a: TraceTree.TraceErrorIssue, + b: TraceTree.TraceErrorIssue +): number { // If the level is not defined in the priority map, default to unknown const aPriority = issueOrderPriority[a.level] ?? issueOrderPriority.unknown; const bPriority = issueOrderPriority[b.level] ?? issueOrderPriority.unknown; @@ -352,7 +351,7 @@ function LegacyIssue( ) : null; } type IssueListProps = { - issues: TraceErrorOrIssue[]; + issues: TraceTree.TraceIssue[]; node: TraceTreeNode; organization: Organization; }; @@ -360,7 +359,7 @@ type IssueListProps = { export function IssueList({issues, node, organization}: IssueListProps) { const hasTraceNewUi = useHasTraceNewUi(); const uniqueErrorIssues = useMemo(() => { - const unique: TraceError[] = []; + const unique: TraceTree.TraceErrorIssue[] = []; const seenIssues: Set = new Set(); @@ -451,7 +450,7 @@ function IssueListHeader({ errorIssues, performanceIssues, }: { - errorIssues: TraceError[]; + errorIssues: TraceTree.TraceErrorIssue[]; node: TraceTreeNode; performanceIssues: TracePerformanceIssue[]; }) { diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/details/span/index.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/index.tsx index ed14756744318a..bfa5102913d0b4 100644 --- a/static/app/views/performance/newTraceDetails/traceDrawer/details/span/index.tsx +++ b/static/app/views/performance/newTraceDetails/traceDrawer/details/span/index.tsx @@ -308,8 +308,10 @@ export function SpanNodeDetails({ analyticsPageSource={LogsAnalyticsPageSource.TRACE_DETAILS} > + {issues.length > 0 ? ( + + ) : null} -
TO DO: EAP Span Details
 diff --git a/static/app/views/performance/newTraceDetails/traceDrawer/tabs/trace/generalInfo.tsx b/static/app/views/performance/newTraceDetails/traceDrawer/tabs/trace/generalInfo.tsx index 3fa30747266b87..6417237f20aa80 100644 --- a/static/app/views/performance/newTraceDetails/traceDrawer/tabs/trace/generalInfo.tsx +++ b/static/app/views/performance/newTraceDetails/traceDrawer/tabs/trace/generalInfo.tsx @@ -7,7 +7,6 @@ import {t, tn} from 'sentry/locale'; import type {EventTransaction} from 'sentry/types/event'; import type {Organization} from 'sentry/types/organization'; import getDuration from 'sentry/utils/duration/getDuration'; -import type {TraceErrorOrIssue} from 'sentry/utils/performance/quickTrace/types'; import type {UseApiQueryResult} from 'sentry/utils/queryClient'; import type RequestError from 'sentry/utils/requestError/requestError'; import {useParams} from 'sentry/utils/useParams'; @@ -38,7 +37,7 @@ export function GeneralInfo(props: GeneralInfoProps) { return []; } - const unique: TraceErrorOrIssue[] = []; + const unique: TraceTree.TraceErrorIssue[] = []; const seenIssues: Set = new Set(); for (const issue of traceNode.errors) { @@ -57,7 +56,7 @@ export function GeneralInfo(props: GeneralInfoProps) { return []; } - const unique: TraceErrorOrIssue[] = []; + const unique: TraceTree.TracePerformanceIssue[] = []; const seenIssues: Set = new Set(); for (const issue of traceNode.performance_issues) { diff --git a/static/app/views/performance/newTraceDetails/traceGuards.tsx b/static/app/views/performance/newTraceDetails/traceGuards.tsx index 9c1a75587310ac..0110ab1e7571ea 100644 --- a/static/app/views/performance/newTraceDetails/traceGuards.tsx +++ b/static/app/views/performance/newTraceDetails/traceGuards.tsx @@ -39,6 +39,12 @@ export function isEAPSpanNode( return !!(node.value && 'is_transaction' in node.value); } +export function isNonTransactionEAPSpanNode( + node: TraceTreeNode +): node is TraceTreeNode { + return isEAPSpanNode(node) && !isEAPTransactionNode(node); +} + export function isTransactionNode( node: TraceTreeNode ): node is TraceTreeNode { @@ -49,6 +55,21 @@ export function isTransactionNode( ); } +export function isEAPError(value: TraceTree.NodeValue): value is TraceTree.EAPError { + return !!( + value && + 'event_type' in value && + value.event_type === 'error' && + !('message' in value) // a bit gross, but we won't need this soon as we remove the legacy error type + ); +} + +export function isEAPErrorNode( + node: TraceTreeNode +): node is TraceTreeNode { + return isEAPError(node.value); +} + export function isParentAutogroupedNode( node: TraceTreeNode ): node is ParentAutogroupNode { @@ -178,7 +199,7 @@ export function getPageloadTransactionChildCount( } export function isTracePerformanceIssue( - issue: TraceTree.TraceError | TraceTree.TracePerformanceIssue + issue: TraceTree.TraceIssue ): issue is TraceTree.TracePerformanceIssue { return 'suspect_spans' in issue; } diff --git a/static/app/views/performance/newTraceDetails/traceHeader/meta.tsx b/static/app/views/performance/newTraceDetails/traceHeader/meta.tsx index 5589ea44270d0c..617e2c6f7d1571 100644 --- a/static/app/views/performance/newTraceDetails/traceHeader/meta.tsx +++ b/static/app/views/performance/newTraceDetails/traceHeader/meta.tsx @@ -8,10 +8,7 @@ import {space} from 'sentry/styles/space'; import type {EventTransaction} from 'sentry/types/event'; import type {Organization} from 'sentry/types/organization'; import getDuration from 'sentry/utils/duration/getDuration'; -import type { - TraceErrorOrIssue, - TraceMeta, -} from 'sentry/utils/performance/quickTrace/types'; +import type {TraceMeta} from 'sentry/utils/performance/quickTrace/types'; import type {UseApiQueryResult} from 'sentry/utils/queryClient'; import type RequestError from 'sentry/utils/requestError/requestError'; @@ -65,7 +62,7 @@ export function Meta(props: MetaProps) { return []; } - const unique: TraceErrorOrIssue[] = []; + const unique: TraceTree.TraceErrorIssue[] = []; const seenIssues: Set = new Set(); for (const issue of traceNode.errors) { @@ -84,7 +81,7 @@ export function Meta(props: MetaProps) { return []; } - const unique: TraceErrorOrIssue[] = []; + const unique: TraceTree.TracePerformanceIssue[] = []; const seenIssues: Set = new Set(); for (const issue of traceNode.performance_issues) { diff --git a/static/app/views/performance/newTraceDetails/traceIcons.tsx b/static/app/views/performance/newTraceDetails/traceIcons.tsx index c045d47fe5b16a..fc25d9068adfe6 100644 --- a/static/app/views/performance/newTraceDetails/traceIcons.tsx +++ b/static/app/views/performance/newTraceDetails/traceIcons.tsx @@ -1,12 +1,7 @@ -import type { - TraceError, - TracePerformanceIssue, -} from 'sentry/utils/performance/quickTrace/types'; - import type {TraceTree} from './traceModels/traceTree'; interface EventTypeIconProps { - event: TracePerformanceIssue | TraceError | TraceTree.Profile; + event: TraceTree.TraceIssue | TraceTree.Profile; } function EventTypeIcon(props: EventTypeIconProps) { diff --git a/static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.tsx b/static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.tsx index d58e35bbd55d8d..13c0c9efc3be8a 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.tsx @@ -1,6 +1,7 @@ import type {Client} from 'sentry/api'; import type {Organization} from 'sentry/types/organization'; import { + isEAPSpanNode, isTraceErrorNode, isTransactionNode, } from 'sentry/views/performance/newTraceDetails/traceGuards'; @@ -65,7 +66,7 @@ export class IssuesTraceTree extends TraceTree { if (isTraceErrorNode(n)) { return n.value.event_id === eventId; } - if (isTransactionNode(n)) { + if (isTransactionNode(n) || isEAPSpanNode(n)) { if (n.value.event_id === eventId) { return true; } @@ -85,8 +86,14 @@ export class IssuesTraceTree extends TraceTree { return false; }); - if (node && isTransactionNode(node)) { - return tree.zoom(node, true, options).then(() => {}); + if (node) { + if (isTransactionNode(node)) { + return tree.zoom(node, true, options).then(() => {}); + } + + if (isEAPSpanNode(node)) { + tree.expand(node, true); + } } return Promise.resolve(); @@ -106,7 +113,9 @@ export class IssuesTraceTree extends TraceTree { const preserveNodes = new Set(preserveLeafNodes); for (const node of preserveLeafNodes) { - const parentTransaction = TraceTree.ParentTransaction(node); + const parentTransaction = isEAPSpanNode(node) + ? TraceTree.ParentEAPTransaction(node) + : TraceTree.ParentTransaction(node); if (parentTransaction) { preserveNodes.add(parentTransaction); } diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx index e289813678a62d..7d6628dbda9325 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTree.spec.tsx @@ -17,6 +17,7 @@ import type {ParentAutogroupNode} from './parentAutogroupNode'; import {TraceTree} from './traceTree'; import { assertTransactionNode, + makeEAPError, makeEAPSpan, makeEAPTrace, makeEventTransaction, @@ -170,6 +171,21 @@ const parentAutogroupSpansWithTailChildren = [ }), ]; +const eapTraceWithErrors = makeEAPTrace([ + makeEAPSpan({ + event_id: 'eap-span-1', + is_transaction: true, + errors: [], + children: [ + makeEAPSpan({ + event_id: 'eap-span-2', + is_transaction: false, + errors: [makeEAPError()], + }), + ], + }), +]); + function findTransactionByEventId(tree: TraceTree, eventId: string) { return TraceTree.Find( tree.root, @@ -536,6 +552,18 @@ describe('TraceTree', () => { expect(tree.build().serialize()).toMatchSnapshot(); }); + it('adds eap errors to tree nodes', () => { + const tree = TraceTree.FromTrace(eapTraceWithErrors, traceMetadata); + + expect(tree.root.children[0]!.errors.size).toBe(1); + + const eapTransaction = findEAPSpanByEventId(tree, 'eap-span-1'); + const eapSpan = findEAPSpanByEventId(tree, 'eap-span-2'); + + expect(eapTransaction?.errors.size).toBe(1); + expect(eapSpan?.errors.size).toBe(1); + }); + it('initializes expanded based on is_transaction property', () => { const tree = TraceTree.FromTrace( makeEAPTrace([ diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx index 7bcfbf48d28fdb..116931b75612a4 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx @@ -4,7 +4,7 @@ import * as qs from 'query-string'; import type {Client} from 'sentry/api'; import type {RawSpanType} from 'sentry/components/events/interfaces/spans/types'; -import type {Event, EventTransaction, Measurement} from 'sentry/types/event'; +import type {Event, EventTransaction, Level, Measurement} from 'sentry/types/event'; import type {Organization} from 'sentry/types/organization'; import type { TraceError as TraceErrorType, @@ -32,6 +32,7 @@ import { isEAPTransactionNode, isJavascriptSDKEvent, isMissingInstrumentationNode, + isNonTransactionEAPSpanNode, isPageloadTransactionNode, isParentAutogroupedNode, isRootNode, @@ -125,10 +126,23 @@ export declare namespace TraceTree { ['trace timeline change']: (view: [number, number]) => void; } + type EAPError = { + event_id: string; + event_type: string; + issue_id: number; + level: Level; + project_id: number; + project_slug: string; + start_timestamp: number; + transaction: string; + description?: string; + }; + type EAPSpan = { children: EAPSpan[]; duration: number; end_timestamp: number; + errors: EAPError[]; event_id: string; is_transaction: boolean; op: string; @@ -140,8 +154,6 @@ export declare namespace TraceTree { description?: string; }; - type EAPTrace = EAPSpan[]; - // Raw node values interface Span extends RawSpanType { measurements?: Record; @@ -153,19 +165,28 @@ export declare namespace TraceTree { } type Trace = TraceSplitResults | EAPTrace; + type TraceError = TraceErrorType; + type TraceErrorIssue = TraceError | EAPError; + type TracePerformanceIssue = TracePerformanceIssueType; + + type TraceIssue = TraceErrorIssue | TracePerformanceIssue; + type Profile = {profile_id: string} | {profiler_id: string}; type Project = { slug: string; }; type Root = null; + type EAPTrace = EAPSpan[]; + // All possible node value types type NodeValue = | Trace | Transaction | TraceError + | EAPError | Span | EAPSpan | MissingInstrumentationSpan @@ -382,11 +403,25 @@ export class TraceTree extends TraceTreeEventDispatcher { traceSpaceBounds[0] = Math.min(traceSpaceBounds[0]!, c.space[0]); traceSpaceBounds[1] = Math.max(traceSpaceBounds[1]!, c.space[0] + c.space[1]); - if (isTransactionNode(c)) { + if (isTransactionNode(c) || isEAPSpanNode(c)) { + let closestEAPTransaction: TraceTreeNode | null = null; + + if (isNonTransactionEAPSpanNode(c)) { + closestEAPTransaction = TraceTree.ParentEAPTransaction(c); + } + for (const error of c.value.errors) { traceNode.errors.add(error); + + // Propagate errors to the closest EAP transaction for visibility in the initially collapsed + // eap-transactions only view, on load + if (closestEAPTransaction) { + closestEAPTransaction.errors.add(error); + } } + } + if (isTransactionNode(c)) { for (const performanceIssue of c.value.performance_issues) { traceNode.performance_issues.add(performanceIssue); } @@ -782,7 +817,7 @@ export class TraceTree extends TraceTreeEventDispatcher { let tail = node; let groupMatchCount = 0; - let errors: TraceErrorType[] = []; + let errors: TraceTree.TraceErrorIssue[] = []; let performance_issues: TraceTree.TracePerformanceIssue[] = []; let start = head.space[0]; @@ -1300,8 +1335,9 @@ export class TraceTree extends TraceTreeEventDispatcher { } } } - if (isSpanNode(n)) { - if (n.value.span_id === eventId) { + if (isSpanNode(n) || isEAPSpanNode(n)) { + const spanId = 'span_id' in n.value ? n.value.span_id : n.value.event_id; + if (spanId === eventId) { return true; } @@ -1317,11 +1353,6 @@ export class TraceTree extends TraceTreeEventDispatcher { } } } - if (isEAPSpanNode(n)) { - if (n.value.event_id === eventId) { - return true; - } - } if (isTraceErrorNode(n)) { return n.value.event_id === eventId; } diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode.tsx index 26f31e562cc4fa..c04c8a499c224f 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeNode.tsx @@ -68,7 +68,7 @@ export class TraceTreeNode event: EventTransaction | null = null; // Events associated with the node, these are inferred from the node value. - errors = new Set(); + errors = new Set(); performance_issues = new Set(); profiles: TraceTree.Profile[] = []; diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTreeTestUtils.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTreeTestUtils.tsx index e9ba962e9f2f2e..71167be71e2b85 100644 --- a/static/app/views/performance/newTraceDetails/traceModels/traceTreeTestUtils.tsx +++ b/static/app/views/performance/newTraceDetails/traceModels/traceTreeTestUtils.tsx @@ -106,11 +106,29 @@ export function makeEAPSpan( transaction: 'span.transaction', parent_span_id: null, children: [], + errors: [], duration: 10, ...overrides, } as TraceTree.EAPSpan; } +export function makeEAPError( + overrides: Partial = {} +): TraceTree.EAPError { + return { + event_id: overrides.event_id ?? uuid4(), + description: 'Test Error', + start_timestamp: 0, + project_id: 1, + project_slug: 'project_slug', + level: 'error', + event_type: 'error', + issue_id: 1, + transaction: 'test error transaction', + ...overrides, + } as TraceTree.EAPError; +} + export function makeTraceError( overrides: Partial = {} ): TraceTree.TraceError { diff --git a/static/app/views/performance/newTraceDetails/traceRenderers/virtualizedViewManager.tsx b/static/app/views/performance/newTraceDetails/traceRenderers/virtualizedViewManager.tsx index 9db0db198ae1db..e6a8e599fdd711 100644 --- a/static/app/views/performance/newTraceDetails/traceRenderers/virtualizedViewManager.tsx +++ b/static/app/views/performance/newTraceDetails/traceRenderers/virtualizedViewManager.tsx @@ -10,7 +10,7 @@ import { requestAnimationTimeout, } from 'sentry/utils/profiling/hooks/useVirtualizedTree/virtualizedTreeUtils'; -import {isMissingInstrumentationNode} from '../traceGuards'; +import {isEAPError, isMissingInstrumentationNode} from '../traceGuards'; import {TraceTree} from '../traceModels/traceTree'; import type {TraceTreeNode} from '../traceModels/traceTreeNode'; import {TraceRowWidthMeasurer} from '../traceRenderers/traceRowWidthMeasurer'; @@ -1706,9 +1706,10 @@ function getIconTimestamps( } for (const err of node.errors) { - if (typeof err.timestamp === 'number') { - min_icon_timestamp = Math.min(min_icon_timestamp, err.timestamp * 1e3 - icon_width); - max_icon_timestamp = Math.max(max_icon_timestamp, err.timestamp * 1e3 + icon_width); + const timestamp = isEAPError(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/traceBackgroundPatterns.tsx b/static/app/views/performance/newTraceDetails/traceRow/traceBackgroundPatterns.tsx index 4f441bbe1b93c6..58895f4813baf2 100644 --- a/static/app/views/performance/newTraceDetails/traceRow/traceBackgroundPatterns.tsx +++ b/static/app/views/performance/newTraceDetails/traceRow/traceBackgroundPatterns.tsx @@ -5,7 +5,7 @@ import type {TraceTree} from '../traceModels/traceTree'; import type {TraceTreeNode} from '../traceModels/traceTreeNode'; import type {VirtualizedViewManager} from '../traceRenderers/virtualizedViewManager'; -function getMaxErrorSeverity(errors: TraceTree.TraceError[]) { +function getMaxErrorSeverity(errors: TraceTree.TraceErrorIssue[]) { return errors.reduce((acc, error) => { if (error.level === 'fatal') { return 'fatal'; diff --git a/static/app/views/performance/newTraceDetails/traceRow/traceIcons.tsx b/static/app/views/performance/newTraceDetails/traceRow/traceIcons.tsx index 82ee67716ce7f4..2655568c0af00f 100644 --- a/static/app/views/performance/newTraceDetails/traceRow/traceIcons.tsx +++ b/static/app/views/performance/newTraceDetails/traceRow/traceIcons.tsx @@ -2,6 +2,7 @@ import {Fragment, useMemo} from 'react'; import clamp from 'lodash/clamp'; import {PlatformIcon} from 'platformicons'; +import {isEAPError} from 'sentry/views/performance/newTraceDetails/traceGuards'; import {useHasTraceNewUi} from 'sentry/views/performance/newTraceDetails/useHasTraceNewUi'; import {TraceIcons} from '../traceIcons'; @@ -27,11 +28,11 @@ export function TraceErrorIcons(props: ErrorIconsProps) { return ( {errors.map((error, i) => { - const timestamp = error.timestamp ? error.timestamp * 1e3 : props.node_space![0]; + const timestamp = isEAPError(error) ? error.start_timestamp : error.timestamp; // Clamp the error timestamp to the span's timestamp const left = props.manager.computeRelativeLeftPositionFromOrigin( clamp( - timestamp, + timestamp ? timestamp * 1e3 : props.node_space![0], props.node_space![0], props.node_space![0] + props.node_space![1] ),