Skip to content

feat(trace-eap-waterfall): Fixing link to profile from trace drawer #96992

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 5 commits into from
Aug 5, 2025

Conversation

Abdkhan14
Copy link
Contributor

@Abdkhan14 Abdkhan14 commented Aug 1, 2025

This PR resolves two issues:

  1. This ISSUE, where the replay shows users clicking on profile links in the drawer:
  • This is from passing '' profile ids that passed the defined guard that only asserts undefined and null
  • Removed all redundant usage of defined
  1. There is a difference between the profiles that we link to from the old txn based trace view and the new spans only trace view:
  • This is because, we were looking for thread.id in eap spans. This attr is not passed with the eap span response type from the new /trace/ endpoint. We want to restrict the size of the response for performance, we don't want to add the threadId to every row in the response atm.
  • The solution for now is to just fetch and use the corresponding nodestore transaction data, to be consistent. We are already doing this for the context section in span details (i.e. everything that eap doesn't support yet like attachments), it's not an anti-pattern atm.

@Abdkhan14 Abdkhan14 requested a review from a team as a code owner August 1, 2025 19:07
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Aug 1, 2025
project_slug: previous.value.project_slug,
});

const event = eventTransaction ?? null;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you should be able to do data: eventTransaction = null so you dont need to define another variable here

cursor[bot]

This comment was marked as outdated.

@Abdkhan14 Abdkhan14 enabled auto-merge (squash) August 5, 2025 17:21
@Abdkhan14 Abdkhan14 merged commit f400459 into master Aug 5, 2025
46 checks passed
@Abdkhan14 Abdkhan14 deleted the abdk/trace-profile-preview branch August 5, 2025 17:32
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