Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[User Feedback] - Issue Trace Timeline #68481

Closed
bruno-garcia opened this issue Apr 8, 2024 · 8 comments · Fixed by #76858
Closed

[User Feedback] - Issue Trace Timeline #68481

bruno-garcia opened this issue Apr 8, 2024 · 8 comments · Fixed by #76858
Assignees
@aliu39
Labels
Product Area: User Feedback

Comments

@bruno-garcia
Copy link
Member

bruno-garcia commented Apr 8, 2024 •
edited
Loading

tl;dr: Add the new trace timeline to User Feedback details

A new component was built for the Issue Details page called Issue Trace Timeline. Previously known as Trace Navigator

image

With the JS SDK v8, each new page load or navigation will create a new trace. That means that if a user submits a User Feedback through the widget, that feedback will be using the same trace-id as other relevant data during that last page load. Including client side errors, as well as calls to the backend that might have be the culprit of that error.

With the JS SDK v7 though, there is a global trace id that wasn't used during page load requests that resulted in HTTP calls to the backend. This means that User Feedbacks from the widget are not likely to be connected to any other telemetry.

Additionally, feedback received from the legacy API (that only takes name/email/comments) doesn't have a trace id. They are connected to an error though (through event_id) so we potentially leverage that trace-id. We can do so by coping the trace_id from the event linked in the shim code.

To verify the data, we can use ?dataset=issuePlatform on Discover. For example this query. Once v8 is installed in Sentry we'll be able to confirm what data would be visible in the component.

Also note the component might need to be adjusted to show User Feedback. It was designed for Issue Details so it shows other error events only (no spans).

Additional context

Spans on trace navigator

image

Trace-id behavior in JS SDK v7 vs v8

image image image
@getsantry
Copy link
Contributor

getsantry bot commented Apr 8, 2024

Routing to @getsentry/product-owners-user-feedback for triage ⏲️

@bruno-garcia
Copy link
Member Author

Depends on TwP working properly:

@aliu39
Copy link
Member

aliu39 commented Jul 23, 2024

@bruno-garcia

To verify the data, we can use ?dataset=issuePlatform on Discover. For example this query. Once v8 is installed in Sentry we'll be able to confirm what data would be visible in the component.

Can you clarify what is meant by "verify the data" here? Also, do you know when v8 will be installed?

@jas-kas
Copy link
Member

jas-kas commented Jul 23, 2024

@aliu39 the v8 SDK has been installed for Sentry :)

by verify the data, i think bruno means to click into the trace IDs associated with the feedback events and confirm if the same issues in those traces, show up in the issue trace timeline

Image

@aliu39 aliu39 mentioned this issue Aug 12, 2024
Merged
aliu39 added a commit that referenced this issue Aug 12, 2024
@aliu39
feat(feedback): add trace context to shimmed feedback events (#75981)
6282ea2 
Ref #68481
@aliu39 aliu39 mentioned this issue Aug 16, 2024
Closed
@aliu39
Copy link
Member

aliu39 commented Aug 17, 2024

Also note the component might need to be adjusted to show User Feedback. It was designed for Issue Details so it shows other error events only (no spans).

I need to find an example, but looking over the code, I believe the component already shows user feedback. UF is an Issue Platform event and this query for timeline events doesn't filter by category. So as long as a UF has a trace id, it'll show up

@aliu39 aliu39 mentioned this issue Sep 3, 2024
Merged
@aliu39
Copy link
Member

aliu39 commented Sep 3, 2024

Closed by #76858

@aliu39 aliu39 closed this as completed Sep 3, 2024
@aliu39
Copy link
Member

aliu39 commented Sep 17, 2024

Still need to GA this

@aliu39 aliu39 reopened this Sep 17, 2024
@aliu39 aliu39 mentioned this issue Sep 17, 2024
Merged
aliu39 added a commit that referenced this issue Sep 17, 2024
@aliu39
ref(feedback): move trace section to below replay and add section tit…
3db9788 
…le/icon (#77668)

Ref #68481. Also refactors
TraceDataSection to its own file.

After:
![Screenshot 2024-09-17 at 3 58
31 PM](https://github.com/user-attachments/assets/1c0cbb21-8f35-46c2-82d6-ed3c485997ae)

Before:
![Screenshot 2024-09-17 at 3 59
24 PM](https://github.com/user-attachments/assets/669e8ffd-3cce-4fb7-8f2e-2482c2e93565)
harshithadurai pushed a commit that referenced this issue Sep 19, 2024
@aliu39
ref(feedback): move trace section to below replay and add section tit…
057bf8f 
…le/icon (#77668)

Ref #68481. Also refactors
TraceDataSection to its own file.

After:
![Screenshot 2024-09-17 at 3 58
31 PM](https://github.com/user-attachments/assets/1c0cbb21-8f35-46c2-82d6-ed3c485997ae)

Before:
![Screenshot 2024-09-17 at 3 59
24 PM](https://github.com/user-attachments/assets/669e8ffd-3cce-4fb7-8f2e-2482c2e93565)
harshithadurai pushed a commit that referenced this issue Sep 19, 2024
@aliu39
ref(feedback): move trace section to below replay and add section tit…
a535bed 
…le/icon (#77668)

Ref #68481. Also refactors
TraceDataSection to its own file.

After:
![Screenshot 2024-09-17 at 3 58
31 PM](https://github.com/user-attachments/assets/1c0cbb21-8f35-46c2-82d6-ed3c485997ae)

Before:
![Screenshot 2024-09-17 at 3 59
24 PM](https://github.com/user-attachments/assets/669e8ffd-3cce-4fb7-8f2e-2482c2e93565)
@aliu39
Copy link
Member

aliu39 commented Oct 1, 2024

Amplitude dashboard: https://app.amplitude.com/analytics/sentry/dashboard/igfiqgc0?source=copy+url

@aliu39 aliu39 mentioned this issue Oct 17, 2024
Merged
aliu39 added a commit that referenced this issue Oct 17, 2024
@aliu39
ref(feedback): fix show/hide logic for trace section, use placeholder…
17dc703 
… while loading (#79284)

Ref #68481

The show/hide logic had a bug where we were showing only if the
same-trace issue is a duplicate of a crash report linked error. Which is
the opposite of what we want. Example:
[JAVASCRIPT-2VE1](https://sentry.sentry.io/feedback/?feedbackSlug=javascript%3A5735318217&project=11276).
_Analytics are still correct, but atm no timelines (2+ events) are
shown_.

Also: 
- use placeholder when the data is fetching.
- use error boundary.
- remove the `hasProject` prop. It makes the logic hard to read, and was
only used for the crash-report-dup condition. Comparing the issue ids is
sufficient for this.
@aliu39 aliu39 closed this as completed Oct 17, 2024
harshithadurai pushed a commit that referenced this issue Oct 19, 2024
@aliu39
ref(feedback): fix show/hide logic for trace section, use placeholder…
328711d 
… while loading (#79284)

Ref #68481

The show/hide logic had a bug where we were showing only if the
same-trace issue is a duplicate of a crash report linked error. Which is
the opposite of what we want. Example:
[JAVASCRIPT-2VE1](https://sentry.sentry.io/feedback/?feedbackSlug=javascript%3A5735318217&project=11276).
_Analytics are still correct, but atm no timelines (2+ events) are
shown_.

Also: 
- use placeholder when the data is fetching.
- use error boundary.
- remove the `hasProject` prop. It makes the logic hard to read, and was
only used for the crash-report-dup condition. Comparing the issue ids is
sufficient for this.
cmanallen pushed a commit that referenced this issue Oct 23, 2024
@aliu39 @cmanallen
ref(feedback): fix show/hide logic for trace section, use placeholder…
f8d287e 
… while loading (#79284)

Ref #68481

The show/hide logic had a bug where we were showing only if the
same-trace issue is a duplicate of a crash report linked error. Which is
the opposite of what we want. Example:
[JAVASCRIPT-2VE1](https://sentry.sentry.io/feedback/?feedbackSlug=javascript%3A5735318217&project=11276).
_Analytics are still correct, but atm no timelines (2+ events) are
shown_.

Also: 
- use placeholder when the data is fetching.
- use error boundary.
- remove the `hasProject` prop. It makes the logic hard to read, and was
only used for the crash-report-dup condition. Comparing the issue ids is
sufficient for this.
@github-actions github-actions bot locked and limited conversation to collaborators Nov 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@aliu39 aliu39

Labels
Product Area: User Feedback
Projects
GitHub Issues with 👀 2
Archived in project
Milestone
No milestone
3 participants
@bruno-garcia @jas-kas @aliu39