Skip to content

Conversation

@aliu39
Copy link
Member

@aliu39 aliu39 commented Apr 15, 2025

Closes #89700

Doc describing the problem and changes: https://www.notion.so/sentry/UF-Ingest-Update-April-2025-1d78b10e4b5d80f08350fa1fbf204b24?pvs=4

Also adds userreport.py test coverage for the shim cases.


Testing Notes / Expected Behavior

  • Submit a feedback with the JS SDK's captureFeedback function. Be sure to include an associatedEventId corresponding to an error event in the same project.

  • The feedback should show up in Sentry /feedback/ page in a few seconds, at most 30s.

  • Open the feedback and find the "Linked Error" section. Click on it to visit the issue/event detail.

  • The feedback email, name, and message should be visible in the "User Feedback" section, and match what you see in /feedback/.

    • If it doesn't show up, there's a small chance it'll be linked by the async task, within 15min. However this should happen very rarely.
      SCR-20250417-nlvw
  • No duplicate feedback should be seen in /feedback/ (wait 15min to confirm this)

  • Widget feedbacks shouldn't break

@aliu39 aliu39 requested review from a team as code owners April 15, 2025 21:20
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 15, 2025
@codecov
Copy link

codecov bot commented Apr 15, 2025

Codecov Report

Attention: Patch coverage is 95.65217% with 5 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...rc/sentry/feedback/usecases/save_feedback_event.py 84.00% 4 Missing ⚠️
src/sentry/tasks/store.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master   #89685    +/-   ##
========================================
  Coverage   85.27%   85.27%            
========================================
  Files       10175    10186    +11     
  Lines      574316   575110   +794     
  Branches    22625    22625            
========================================
+ Hits       489739   490453   +714     
- Misses      84030    84110    +80     
  Partials      547      547            

Copy link
Member

@ryan953 ryan953 left a comment

Choose a reason for hiding this comment

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

seems reasonable to me. but i would have a hard time to know what, if anything, is missing

@aliu39 aliu39 changed the title fix(feedback): shim new feedbacks to postgres so they can be queried by group_id feat(feedback): shim feedback v2 to postgres so they can be queried by group_id Apr 18, 2025
@aliu39 aliu39 merged commit 9ccc5d2 into master Apr 18, 2025
71 checks passed
@aliu39 aliu39 deleted the aliu/feedback-reverse-shim branch April 18, 2025 18:12
@sentry
Copy link

sentry bot commented Apr 18, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TypeError: 'str' object cannot be interpreted as an integer sentry.tasks.store.save_event_feedback View Issue
  • ‼️ KeyError: 'name' sentry.tasks.store.save_event_feedback View Issue
  • ‼️ ValueError: invalid literal for int() with base 10: 'production' sentry.tasks.store.save_event_feedback View Issue
  • ‼️ DataError: StringDataRightTruncation('value too long for type character varying(75)\n') sentry.tasks.store.save_event_feedback View Issue
  • ‼️ Conflict: Feedback for this event cannot be modified. sentry.tasks.store.save_event_feedback View Issue

Did you find this useful? React with a 👍 or 👎

aliu39 added a commit that referenced this pull request Apr 18, 2025
Follow-up on #89685.

Fixes
[SENTRY-3SEX](https://sentry.sentry.io/issues/6551468609/events/9a88f59af64d4f86bddf42060cdff0d9/)
Fixes
[SENTRY-3SEZ](https://sentry.sentry.io/issues/6551532041/events/bd020dada1a74f1ba96faa07c33df30c/)
Fixes
[SENTRY-3SEY](https://sentry.sentry.io/issues/6551484362/events/42136852328a4acdb6f21922bd6425af/)

Only the message/comments field is required, so we need to handle
missing name or email. Also event timestamp is a ISO str.

Also adds better typing to `save_userreport` and cleans up the event
formatting code in `create_feedback` a bit. Functionality is the same,
except now we'll always include the `user.email` context/tag, defaulting
to empty str `""`
aliu39 added a commit that referenced this pull request Apr 21, 2025
Fixes SENTRY-3SFQ 
Followup to #89685
Improves the test coverage so we assert a UR is actually saved, instead
of asserting the arguments to `save_userreport`.
andrewshie-sentry pushed a commit that referenced this pull request Apr 22, 2025
Follow-up on #89685.

Fixes
[SENTRY-3SEX](https://sentry.sentry.io/issues/6551468609/events/9a88f59af64d4f86bddf42060cdff0d9/)
Fixes
[SENTRY-3SEZ](https://sentry.sentry.io/issues/6551532041/events/bd020dada1a74f1ba96faa07c33df30c/)
Fixes
[SENTRY-3SEY](https://sentry.sentry.io/issues/6551484362/events/42136852328a4acdb6f21922bd6425af/)

Only the message/comments field is required, so we need to handle
missing name or email. Also event timestamp is a ISO str.

Also adds better typing to `save_userreport` and cleans up the event
formatting code in `create_feedback` a bit. Functionality is the same,
except now we'll always include the `user.email` context/tag, defaulting
to empty str `""`
andrewshie-sentry pushed a commit that referenced this pull request Apr 22, 2025
Fixes SENTRY-3SFQ 
Followup to #89685
Improves the test coverage so we assert a UR is actually saved, instead
of asserting the arguments to `save_userreport`.
@github-actions github-actions bot locked and limited conversation to collaborators May 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[User Feedback] Feedbacks submitted through the new captureFeedback API do not show up in issue/event detail of associated error

3 participants