Skip to content

feat(trace-eap-waterfall): Adding support for occurrences #89221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ function NewTraceDetailsSpanDetail(props: SpanDetailProps) {

const profileId = props.event.contexts.profile?.profile_id || '';
const issues = useMemo(() => {
return [...props.node.errors, ...props.node.occurences];
}, [props.node.errors, props.node.occurences]);
return [...props.node.errors, ...props.node.occurrences];
}, [props.node.errors, props.node.occurrences]);

const {projects} = useProjects();
const project = projects.find(p => p.id === props.event.projectID);
Expand Down Expand Up @@ -299,7 +299,7 @@ function NewTraceDetailsSpanDetail(props: SpanDetailProps) {
}

function renderSpanErrorMessage() {
const hasErrors = props.node.errors.size > 0 || props.node.occurences.size > 0;
const hasErrors = props.node.errors.size > 0 || props.node.occurrences.size > 0;

if (!hasErrors || isGapSpan(props.node.value)) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,24 +189,39 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) {
}
}

for (const p of n.occurences) {
if (p.event_id === props.event.eventID) {
for (const o of n.occurrences) {
if (o.event_id === props.event.eventID) {
return true;
}
}
}
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) {
if (isSpanNode(n)) {
if (n.value.span_id === props.event.eventID) {
return true;
}
for (const e of n.errors) {
if (e.event_id === props.event.eventID) {
return true;
}
}
for (const p of n.occurences) {
if (p.event_id === props.event.eventID) {
for (const o of n.occurrences) {
if (o.event_id === props.event.eventID) {
return true;
}
}
}

if (isEAPSpanNode(n)) {
if (n.value.event_id === props.event.eventID) {
return true;
}
for (const e of n.errors) {
if (e.event_id === props.event.eventID) {
return true;
}
}
for (const o of n.occurrences) {
if (o.event_id === props.event.occurrence?.id) {
return true;
}
}
Expand All @@ -232,7 +247,7 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) {
if (
isTraceErrorNode(props.tree.list[start]!) ||
node.errors.size > 0 ||
node.occurences.size > 0
node.occurrences.size > 0
) {
preserveNodes.push(props.tree.list[start]!);
break;
Expand All @@ -244,7 +259,7 @@ export function IssuesTraceWaterfall(props: IssuesTraceWaterfallProps) {
if (
isTraceErrorNode(props.tree.list[start]!) ||
node.errors.size > 0 ||
node.occurences.size > 0
node.occurrences.size > 0
) {
preserveNodes.push(props.tree.list[start]!);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ export function ParentAutogroupNodeDetails({
}: TraceTreeNodeDetailsProps<ParentAutogroupNode>) {
const theme = useTheme();
const issues = useMemo(() => {
return [...node.errors, ...node.occurences];
}, [node.errors, node.occurences]);
return [...node.errors, ...node.occurrences];
}, [node.errors, node.occurrences]);

const parentTransaction = TraceTree.ParentTransaction(node);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ export function SiblingAutogroupNodeDetails({
}: TraceTreeNodeDetailsProps<SiblingAutogroupNode>) {
const theme = useTheme();
const issues = useMemo(() => {
return [...node.errors, ...node.occurences];
}, [node.errors, node.occurences]);
return [...node.errors, ...node.occurrences];
}, [node.errors, node.occurrences]);

const parentTransaction = TraceTree.ParentTransaction(node);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,10 @@ export function IssueList({issues, node, organization}: IssueListProps) {
}, [node, node.errors.size]);

const uniqueOccurences = useMemo(() => {
const unique: TraceTree.TraceOccurence[] = [];
const unique: TraceTree.TraceOccurrence[] = [];
const seenIssues: Set<number> = new Set();

for (const issue of node.occurences) {
for (const issue of node.occurrences) {
if (seenIssues.has(issue.issue_id)) {
continue;
}
Expand All @@ -323,7 +323,7 @@ export function IssueList({issues, node, organization}: IssueListProps) {
return unique;

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [node, node.occurences.size]);
}, [node, node.occurrences.size]);

const uniqueIssues = useMemo(() => {
return [...uniqueOccurences, ...uniqueErrorIssues.sort(sortIssuesByLevel)];
Expand Down Expand Up @@ -384,7 +384,7 @@ function IssueListHeader({
}: {
errorIssues: TraceTree.TraceErrorIssue[];
node: TraceTreeNode<TraceTree.NodeValue>;
occurences: TraceTree.TraceOccurence[];
occurences: TraceTree.TraceOccurrence[];
}) {
const [singular, plural] = useMemo((): [string, string] => {
const label = [t('Issue'), t('Issues')] as [string, string];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,8 @@ export function SpanNodeDetails({
const hasNewTraceUi = useHasTraceNewUi();
const {projects} = useProjects();
const issues = useMemo(() => {
return [...node.errors, ...node.occurences];
}, [node.errors, node.occurences]);
return [...node.errors, ...node.occurrences];
}, [node.errors, node.occurrences]);

const project = projects.find(proj => proj.slug === node.event?.projectSlug);
const profileMeta = getProfileMeta(node.event) || '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ export function TransactionNodeDetails({
}: TraceTreeNodeDetailsProps<TraceTreeNode<TraceTree.Transaction>>) {
const {projects} = useProjects();
const issues = useMemo(() => {
return [...node.errors, ...node.occurences];
}, [node.errors, node.occurences]);
return [...node.errors, ...node.occurrences];
}, [node.errors, node.occurrences]);
const {
data: event,
isError,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ export function GeneralInfo(props: GeneralInfoProps) {
return [];
}

const unique: TraceTree.TraceOccurence[] = [];
const unique: TraceTree.TraceOccurrence[] = [];
const seenIssues: Set<number> = new Set();

for (const issue of traceNode.occurences) {
for (const issue of traceNode.occurrences) {
if (seenIssues.has(issue.issue_id)) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ export function TraceDetails(props: TraceDetailsProps) {
return [];
}

return [...props.node.errors, ...props.node.occurences];
return [...props.node.errors, ...props.node.occurrences];

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [props.node, props.node?.errors.size, props.node?.occurences.size]);
}, [props.node, props.node?.errors.size, props.node?.occurrences.size]);

if (!props.node) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,6 @@ export function getPageloadTransactionChildCount(

export function isTraceOccurence(
issue: TraceTree.TraceIssue
): issue is TraceTree.TraceOccurence {
): issue is TraceTree.TraceOccurrence {
return 'issue_id' in issue && issue.event_type !== 'error';
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,10 @@ export function Meta(props: MetaProps) {
return [];
}

const unique: TraceTree.TraceOccurence[] = [];
const unique: TraceTree.TraceOccurrence[] = [];
const seenIssues: Set<number> = new Set();

for (const issue of traceNode.occurences) {
for (const issue of traceNode.occurrences) {
if (seenIssues.has(issue.issue_id)) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ function mockSpansResponse(

function hasErrors(n: TraceTreeNode<any>): boolean {
return (
(isTraceErrorNode(n) || n.errors.size > 0 || n.occurences.size > 0) && !isTraceNode(n)
(isTraceErrorNode(n) || n.errors.size > 0 || n.occurrences.size > 0) &&
!isTraceNode(n)
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type {Client} from 'sentry/api';
import type {Event} from 'sentry/types/event';
import type {Organization} from 'sentry/types/organization';
import type {TraceMetaQueryResults} from 'sentry/views/performance/newTraceDetails/traceApi/useTraceMeta';
import {
Expand Down Expand Up @@ -55,7 +56,7 @@ export class IssuesTraceTree extends TraceTree {

static ExpandToEvent(
tree: IssuesTraceTree,
eventId: string,
event: Event,
options: {
api: Client;
organization: Organization;
Expand All @@ -64,21 +65,25 @@ export class IssuesTraceTree extends TraceTree {
): Promise<void> {
const node = TraceTree.Find(tree.root, n => {
if (isTraceErrorNode(n) || isEAPErrorNode(n)) {
return n.value.event_id === eventId;
return n.value.event_id === event.eventID;
}
if (isTransactionNode(n) || isEAPSpanNode(n)) {
if (n.value.event_id === eventId) {
if (n.value.event_id === event.eventID) {
return true;
}

for (const e of n.errors) {
if (e.event_id === eventId) {
if (e.event_id === event.eventID) {
return true;
}
}

for (const p of n.occurences) {
if (p.event_id === eventId) {
for (const o of n.occurrences) {
if (isTransactionNode(n)) {
if (o.event_id === event.eventID) {
return true;
}
} else if (o.event_id === event.occurrence?.id) {
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
assertEAPSpanNode,
assertTransactionNode,
makeEAPError,
makeEAPOccurrence,
makeEAPSpan,
makeEAPTrace,
makeEventTransaction,
Expand Down Expand Up @@ -188,6 +189,21 @@ const eapTraceWithErrors = makeEAPTrace([
}),
]);

const eapTraceWithOccurences = makeEAPTrace([
makeEAPSpan({
event_id: 'eap-span-1',
is_transaction: true,
occurrences: [],
children: [
makeEAPSpan({
event_id: 'eap-span-2',
is_transaction: false,
occurrences: [makeEAPOccurrence({event_id: 'eap-occurence-1'})],
}),
],
}),
]);

const eapTraceWithOrphanErrors = makeEAPTrace([
makeEAPError({
event_id: 'eap-error-1',
Expand Down Expand Up @@ -252,7 +268,7 @@ describe('TraceTree', () => {
}),
traceMetadata
);
expect(tree.root.children[0]!.children[0]!.occurences.size).toBe(1);
expect(tree.root.children[0]!.children[0]!.occurrences.size).toBe(1);
});

it('adds transaction profile to node', () => {
Expand Down Expand Up @@ -584,6 +600,18 @@ describe('TraceTree', () => {
expect(eapSpan?.errors.size).toBe(1);
});

it('adds eap occurences to tree nodes', () => {
const tree = TraceTree.FromTrace(eapTraceWithOccurences, traceMetadata);

expect(tree.root.children[0]!.occurrences.size).toBe(1);

const eapTransaction = findEAPSpanByEventId(tree, 'eap-span-1');
const eapSpan = findEAPSpanByEventId(tree, 'eap-span-2');

expect(eapTransaction?.occurrences.size).toBe(1);
expect(eapSpan?.occurrences.size).toBe(1);
});

it('initializes expanded based on is_transaction property', () => {
const tree = TraceTree.FromTrace(
makeEAPTrace([
Expand Down
Loading
Loading