Skip to content

fix(trace): Remove eventTransaction in showJSONLink check - #120477

Merged
mjq merged 1 commit into
masterfrom
mjq/trace-waterfall-no-txns-1
Jul 24, 2026
Merged

fix(trace): Remove eventTransaction in showJSONLink check#120477
mjq merged 1 commit into
masterfrom
mjq/trace-waterfall-no-txns-1

Conversation

@mjq

@mjq mjq commented Jul 23, 2026

Copy link
Copy Markdown
Member

I'm gradually removing all references to eventTransaction from the EAP span version of the trace waterfall and span drawer (see BROWSE-646).

This PR removes it from the showJSONLink gate on TraceDrawerComponents.NodeActions - it actually works fine downstream already without a transaction, just had to make the transactionId check only affect the branch that uses the old transaction JSON URL.

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 23, 2026
@mjq
mjq force-pushed the mjq/trace-waterfall-no-txns-1 branch from 2961cac to 4aba171 Compare July 23, 2026 17:28

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 4aba171. Configure here.

@mjq
mjq marked this pull request as ready for review July 23, 2026 17:40
@mjq
mjq requested a review from a team as a code owner July 23, 2026 17:40
/>
</Tooltip>
{props.showJSONLink && transactionId ? (
{props.showJSONLink && (canShowEAPSpanJSON || transactionId) ? (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on your description, will we have to eventually get rid of transactionId? If so, might be worth leaving a todo comment or smth similar

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transaction IDs actually keep existing, even though transaction events are going away 😓 We still group spans into segments, which approximate transactions, and the user-facing name for them is still (for the moment) transactions.

Either way, this particular use of transactionId will go away when the non-EAP path is removed, which is hopefully happening in the next week or two. It'll show up as dead code so I'm not too worried about the todo. Good suggestion though!

@mjq
mjq merged commit 3a2c016 into master Jul 24, 2026
74 of 75 checks passed
@mjq
mjq deleted the mjq/trace-waterfall-no-txns-1 branch July 24, 2026 13:22
Christinarlong pushed a commit that referenced this pull request Jul 24, 2026
I'm gradually removing all references to `eventTransaction` from the EAP
span version of the trace waterfall and span drawer (see BROWSE-646).

This PR removes it from the `showJSONLink` gate on
`TraceDrawerComponents.NodeActions` - it actually works fine downstream
already without a transaction, just had to make the `transactionId`
check only affect the branch that uses the old transaction JSON URL.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants