Skip to content

Replays: Self-Serve Bulk Delete System#6

Open
zaibkhan wants to merge 32 commits into
replays-delete-vulnerablefrom
replays-delete-stable
Open

Replays: Self-Serve Bulk Delete System#6
zaibkhan wants to merge 32 commits into
replays-delete-vulnerablefrom
replays-delete-stable

Conversation

@zaibkhan

@zaibkhan zaibkhan commented Sep 4, 2025

Copy link
Copy Markdown

This PR introduces a self-serve bulk delete system for Replays, enabling large-scale data cleanup and management.

Key changes:

  • Adds endpoints and logic for bulk deleting Replay data
  • Implements safeguards for performance and safety
  • Introduces monitoring hooks for deletion progress
  • Improves error handling and rollback for failed deletions

Benchmark reference:
Replicated from ai-code-review-evaluation/sentry-greptile PR #5

armenzg and others added 30 commits June 20, 2025 12:49
…o 'low' (#93927)"

This reverts commit 8d04522.

Co-authored-by: roaga <47861399+roaga@users.noreply.github.com>
Missed in the initial commit, leading to some relevant logs being
unannotated.
We have had a few tasks get killed at 10% rollout.
Also add a test, so that this doesn't happen again
Fixes DE-129 and DE-156

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
These transitions should be matching
…` (#93946)

Use `project_id` on the replay record instead of the URL (where it does
not always exist).

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Also fixed `replay.view_html` -> `replay.view-html`

---------

Co-authored-by: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com>
…948)

gets `npx @typescript/native-preview` passing again
The conditions associated with a DCG can change over time, and it's good
if we can be completely confident that they're consistent within a given
task execution.
This is unused and most regex experiments have required broader changes
to ensure that regexes are evaluated in a specific order (ex:
traceparent). Removing this for now to simplify the code and very
slightly improve runtime performance.
From some testing (on feedback lists of all different lengths), this
prompt seems to work better. It doesn't write overly long sentences and
also does a better job at "summarizing" versus just mentioning a few
specific topics and leaving out others.
Just remove a couple custom Flex* classes in favor of the Flex primitive
This has been killed a few times.

Refs SENTRY-42M7
…n table (#93892)

<!-- Describe your PR here. -->

[ticket](https://linear.app/getsentry/issue/ID-156/grouping-info-remove-type-field-from-ui)
The Type field in the Grouping Info section of the issue details page
was redundant.
This removes the Type row from all variant types while keeping the
underlying data structure intact.

before
![Screenshot 2025-06-20 at 12 00
54 PM](https://github.com/user-attachments/assets/97ca72da-0a52-4446-9825-cd4fcb505adf)

after
![Screenshot 2025-06-20 at 11 59
29 AM](https://github.com/user-attachments/assets/a4284d2b-c9f5-442f-b010-7fe72a598e39)
### Changes
Related to this PR: getsentry/sentry#93810. This
is part 1 of the change, which is pulling out the new component and just
adding it to the repo. Also includes some simplification of the logic in
the base component.

Part 2 will be replacing tables in widgets.

### Before/After

There is no UI change as the table is not being used yet. There is a new
story page for the component.
…93943)

to prevent this issue from becoming too noisy, add a noise config
Unfortunately, 'event_data' went from being the variable for current
event context to being the complete parsed data from Redis, and we
continued logging it per group.
That's more data than we should be logging even arguably once, let alone
per group.
Co-authored-by: Abdullah Khan <abdullahkhan@PG9Y57YDXQ.local>
Adds some simple analytics to our endpoint so we can begin building a
dashboard in Amplitude.
Previously, explore supported multiple y axis per chart, so each
visualize supported multiple y axis. That functionality has since been
removed for simplicity so update the types here to match. Keep in mind
that saved queries still store them as an array so when
serializing/deserializing, we still need to treat it as an array.
We'll need the `useGetTraceItemAttributeKeys` hook in other places so
refactoring it so that it can exported.
mrduncan and others added 2 commits June 20, 2025 13:20
When the max segment ID is null the process fails. We should exit early
since if there aren't any segments to delete there's nothing to do.
@codoki-ai codoki-ai deleted a comment from codoki-pr-intelligence Bot Sep 5, 2025
@codoki-ai codoki-ai deleted a comment from codoki-pr-intelligence Bot Sep 5, 2025
@codoki-ai codoki-ai deleted a comment from codoki-pr-intelligence Bot Sep 5, 2025
@codoki-ai codoki-ai deleted a comment from codoki-pr-intelligence Bot Sep 5, 2025
@codoki-ai codoki-ai deleted a comment from codoki-pr-intelligence Bot Sep 5, 2025
@codoki-pr-intelligence

codoki-pr-intelligence Bot commented Sep 5, 2025

Copy link
Copy Markdown

Codoki PR Review

Summary: Refactor layout components, standardize templates
What’s good: Consistent migration to shared Flex component and removal of duplicate PR comment templates improves maintainability and consistency across integrations.
Review Status: ❌ Requires changes
Overall Priority: Critical
Review Update:
• Scope: Large PR detected (106 files)
• Coverage: Reviewed 30 highest-risk files across 3 batches
• Note: For full confidence, consider splitting this PR; current review covers top 30/106 files

This review covered the top 30 of 106 files (risk-ranked). For complete coverage and faster feedback, consider splitting into ~8 smaller PR(s).

Issues (Critical & High only)

Severity Issue Why it matters
Critical Correctness — Crash when query disabled due to undefined data …/list/useFeedbackSummary.tsx
When the query is disabled (feature flag not present), data is undefined and the code below accesses data.summary and data.numFeedbacksUsed, causing a runtime crash. Guard the disabled/undefined case here so the hook returns safe defaults.
High Correctness — Mismatched error_id to event data due to zip over dict va... …/endpoints/project_replay_summarize_breadcrumbs.py
Using zip(error_ids, events.values()) assumes nodestore.get_multi preserves insertion order matching error_ids, which is not guaranteed and can mispair IDs with the wrong event data. If one event is missing, zip truncates and further shifts mappings. This can yield incorrect error titles/messages being interleaved for a different error_id.

Showing top 2 issues. Critical: 1, High: 1. See inline suggestions for more.

Key Feedback (click to expand)
  • Needs improvement: None identified that would materially impact correctness, security, or performance based on the shown diffs.
  • Open questions: Are there downstream references to the removed ParameterizationRegexExperiment or the old PR single-issue template constant in other modules? If so, ensure they are updated to avoid import/runtime errors.

Confidence: 2/5 — Not ready to merge (1 critical · 1 high · status: Requires changes · scope: top 30/106 files reviewed)

React with 👍 or 👎 if you found this review useful.

ErrorEvent(
category="error",
id=event_id,
title=data.get("title", ""),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ High: Using zip(error_ids, events.values()) assumes nodestore.get_multi preserves insertion order matching error_ids, which is not guaranteed and can mispair IDs with the wrong event data. If one event is missing, zip truncates and further shifts mappings. This can yield incorrect error titles/messages being interleaved for a different error_id.

};
}

if (isError) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 Critical: When the query is disabled (feature flag not present), data is undefined and the code below accesses data.summary and data.numFeedbacksUsed, causing a runtime crash. Guard the disabled/undefined case here so the hook returns safe defaults.

Suggested change
if (isError) {
if (isError || !data) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.