Skip to content

chore(feedback): remove user-feedback-ingest rollout flag#78097

Merged
aliu39 merged 6 commits intomasterfrom
aliu/rm-feedback-rollout-flag
Oct 3, 2024
Merged

chore(feedback): remove user-feedback-ingest rollout flag#78097
aliu39 merged 6 commits intomasterfrom
aliu/rm-feedback-rollout-flag

Conversation

@aliu39
Copy link
Member

@aliu39 aliu39 commented Sep 25, 2024

Removes this rollout flag since user feedback is GA'd. Applies the options denylist directly (it was previously checked in the flag handler).

Checks the denylist in the shim function. I think this is a better place to check it - previously we were missing a spot in post-process

PR making this change safe for internal and external (self hosted) relays: getsentry/relay#4076

@aliu39 aliu39 requested review from a team as code owners September 25, 2024 00:16
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 25, 2024
@codecov
Copy link

codecov bot commented Sep 25, 2024

Codecov Report

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

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/feedback/usecases/create_feedback.py 60.00% 1 Missing and 1 partial ⚠️
src/sentry/ingest/consumer/processors.py 33.33% 1 Missing and 1 partial ⚠️
src/sentry/tasks/update_user_reports.py 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #78097      +/-   ##
==========================================
+ Coverage   78.12%   78.18%   +0.06%     
==========================================
  Files        7089     7039      -50     
  Lines      312504   311906     -598     
  Branches    51052    51012      -40     
==========================================
- Hits       244132   243854     -278     
+ Misses      62016    56294    -5722     
- Partials     6356    11758    +5402     

"organizations:standalone-span-ingestion",
"organizations:transaction-name-mark-scrubbed-as-sanitized",
"organizations:transaction-name-normalize",
"organizations:user-feedback-ingest",
Copy link
Member

Choose a reason for hiding this comment

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

Is the Relay PR for this opened yet? We need to make sure that we don't accidentally drop user feedback in external Relays (i.e. internal Relays will have to hard-code this flag when propagating project config downstream).

Copy link
Member Author

Choose a reason for hiding this comment

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

Just opened it!getsentry/relay#4076

Copy link
Member Author

@aliu39 aliu39 Sep 25, 2024

Choose a reason for hiding this comment

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

We need to make sure that we don't accidentally drop user feedback in external Relays (i.e. internal Relays will have to hard-code this flag when propagating project config downstream).

Sorry I don't think I understand - what conditions would this happen under?

Copy link
Member

Choose a reason for hiding this comment

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

@aliu39 customers that run their own Relay instances do not have the updated Relay code yet, so they will still check the feature flag and drop data if it's missing.

I pushed a commit to your Relay PR to prevent that, hope that clarifies what I mean!

jjbayer added a commit to getsentry/relay that referenced this pull request Sep 27, 2024
Ref getsentry/sentry#78097

---------

Co-authored-by: Joris Bayer <joris.bayer@sentry.io>
Co-authored-by: David Herberth <david.herberth@sentry.io>
@aliu39 aliu39 enabled auto-merge (squash) October 3, 2024 19:09
@aliu39 aliu39 merged commit 14e2ac4 into master Oct 3, 2024
@aliu39 aliu39 deleted the aliu/rm-feedback-rollout-flag branch October 3, 2024 19:44
@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2024
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.

3 participants