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

ref(processing/store): remove references to feedback attachments option #3617

Merged
merged 3 commits into from
May 21, 2024
Merged

ref(processing/store): remove references to feedback attachments option #3617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d0efcd2
ref(processing/store): remove references to feedback attachments option
aliu39 May 16, 2024
bcd66be
Rm parametrize
aliu39 May 20, 2024
409e5c8
Merge branch 'master' into aliu/rm-feedback-attachments-option
aliu39 May 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions relay-dynamic-config/src/global.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,19 +198,6 @@ pub struct Options {
)]
pub feedback_ingest_topic_rollout_rate: f32,

/// Flag for handling feedback and attachments in the same envelope. This is for the SDK team to send less requests
/// for the user feedback screenshots feature. Prior to this change, feedback sent w/attachments would be produced
/// to the attachments topic, rather than its own topic. The items are now split up accordingly.
///
/// This option is used as a temporary FF/kill-switch to toggle back to the old code path in relay's StoreService.
/// This is for testing convenience and will be removed after user feedback's GA release.
#[serde(
rename = "feedback.ingest-inline-attachments",
deserialize_with = "default_on_error",
skip_serializing_if = "is_default"
)]
pub feedback_ingest_same_envelope_attachments: bool,

/// Overall sampling of span extraction.
///
/// This number represents the fraction of transactions for which
Expand Down
15 changes: 3 additions & 12 deletions relay-server/src/services/store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,19 +190,10 @@ impl StoreService {
let retention = envelope.retention();
let event_id = envelope.event_id();

let feedback_ingest_same_envelope_attachments = self
.global_config
.current()
.options
.feedback_ingest_same_envelope_attachments;

let event_item = envelope.as_mut().take_item_by(|item| {
matches!(
(item.ty(), feedback_ingest_same_envelope_attachments),
(ItemType::Event, _)
| (ItemType::Transaction, _)
| (ItemType::Security, _)
| (ItemType::UserReportV2, false)
item.ty(),
ItemType::Event | ItemType::Transaction | ItemType::Security
)
});
let client = envelope.meta().client();
Expand Down Expand Up @@ -251,7 +242,7 @@ impl StoreService {
item,
)?;
}
ItemType::UserReportV2 if feedback_ingest_same_envelope_attachments => {
ItemType::UserReportV2 => {
let remote_addr = envelope.meta().client_addr().map(|addr| addr.to_string());
self.produce_user_report_v2(
event_id.ok_or(StoreError::NoEventId)?,
Expand Down
13 changes: 1 addition & 12 deletions tests/integration/test_feedback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,22 +88,17 @@ def assert_expected_feedback(parsed_feedback, sent_feedback):
}


@pytest.mark.parametrize("use_feedback_ingest_v2", (False, True))
@pytest.mark.parametrize("use_feedback_topic", (False, True))
def test_feedback_event_with_processing(
mini_sentry,
relay_with_processing,
events_consumer,
feedback_consumer,
use_feedback_topic,
use_feedback_ingest_v2,
):
mini_sentry.add_basic_project_config(
42, extra={"config": {"features": ["organizations:user-feedback-ingest"]}}
)
mini_sentry.set_global_config_option(
"feedback.ingest-inline-attachments", use_feedback_ingest_v2
)

if use_feedback_topic:
mini_sentry.set_global_config_option("feedback.ingest-topic.rollout-rate", 1.0)
Expand All @@ -129,19 +124,15 @@ def test_feedback_event_with_processing(
other_consumer.assert_empty()


@pytest.mark.parametrize("use_feedback_ingest_v2", (False, True))
@pytest.mark.parametrize("use_feedback_topic", (False, True))
def test_feedback_events_without_processing(
mini_sentry, relay_chain, use_feedback_topic, use_feedback_ingest_v2
mini_sentry, relay_chain, use_feedback_topic
):
project_id = 42
mini_sentry.add_basic_project_config(
project_id,
extra={"config": {"features": ["organizations:user-feedback-ingest"]}},
)
mini_sentry.set_global_config_option(
"feedback.ingest-inline-attachments", use_feedback_ingest_v2
)
mini_sentry.set_global_config_option(
"feedback.ingest-topic.rollout-rate", 1.0 if use_feedback_topic else 0.0
)
Expand Down Expand Up @@ -169,8 +160,6 @@ def test_feedback_with_attachment_in_same_envelope(
mini_sentry.add_basic_project_config(
42, extra={"config": {"features": ["organizations:user-feedback-ingest"]}}
)
# Test will only pass with this option set
mini_sentry.set_global_config_option("feedback.ingest-inline-attachments", True)

if use_feedback_topic:
mini_sentry.set_global_config_option("feedback.ingest-topic.rollout-rate", 1.0)
Expand Down
Loading