From c0b47cc16c173bc7479ecea4bec73994e0a9a892 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Tue, 2 Mar 2021 06:11:25 -0800 Subject: [PATCH 01/16] feat(stats): Emit second outcome for attachments when handling request --- relay-server/src/actors/events.rs | 1 + relay-server/src/actors/outcome.rs | 6 ++++++ relay-server/src/endpoints/common.rs | 31 +++++++++++++++++++++------- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/relay-server/src/actors/events.rs b/relay-server/src/actors/events.rs index 681d0908283..9fbe66ed13f 100644 --- a/relay-server/src/actors/events.rs +++ b/relay-server/src/actors/events.rs @@ -1700,6 +1700,7 @@ impl Handler for EventManager { event_id, remote_addr, category, + quantity: 1, }) } }) diff --git a/relay-server/src/actors/outcome.rs b/relay-server/src/actors/outcome.rs index 284a553f4cc..4aa15648442 100644 --- a/relay-server/src/actors/outcome.rs +++ b/relay-server/src/actors/outcome.rs @@ -83,6 +83,8 @@ pub struct TrackOutcome { pub remote_addr: Option, /// The event's data category. pub category: DataCategory, + /// The number of events or total attachment size in bytes. + pub quantity: usize, } impl Message for TrackOutcome { @@ -313,6 +315,9 @@ pub struct TrackRawOutcome { /// The event's data category. #[serde(default, skip_serializing_if = "Option::is_none")] pub category: Option, + /// The number of events or total attachment size in bytes. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub quantity: Option, } impl TrackRawOutcome { @@ -348,6 +353,7 @@ impl TrackRawOutcome { remote_addr: msg.remote_addr.map(|addr| addr.to_string()), source, category: msg.category.value(), + quantity: Some(msg.quantity), } } } diff --git a/relay-server/src/endpoints/common.rs b/relay-server/src/endpoints/common.rs index 77343ab9fd6..3de5bcfdcec 100644 --- a/relay-server/src/endpoints/common.rs +++ b/relay-server/src/endpoints/common.rs @@ -11,7 +11,7 @@ use failure::Fail; use futures::prelude::*; use serde::Deserialize; -use relay_common::{clone, metric, tryf}; +use relay_common::{clone, metric, tryf, DataCategory}; use relay_config::Config; use relay_general::protocol::{EventId, EventType}; use relay_log::LogError; @@ -26,7 +26,7 @@ use crate::envelope::{AttachmentType, Envelope, EnvelopeError, ItemType, Items}; use crate::extractors::RequestMeta; use crate::metrics::RelayCounters; use crate::service::{ServiceApp, ServiceState}; -use crate::utils::{self, ApiErrorResponse, FormDataIter, MultipartError}; +use crate::utils::{self, ApiErrorResponse, EnvelopeSummary, FormDataIter, MultipartError}; #[derive(Fail, Debug)] pub enum BadStoreRequest { @@ -410,19 +410,19 @@ where let scoping = Rc::new(RefCell::new(meta.get_partial_scoping())); let event_id = Rc::new(RefCell::new(None)); - let event_category = Rc::new(RefCell::new(None)); + let envelope_summary = Rc::new(RefCell::new(EnvelopeSummary::empty())); let config = request.state().config(); let processing_enabled = config.processing_enabled(); let future = project_manager .send(GetProject { public_key }) .map_err(BadStoreRequest::ScheduleFailed) - .and_then(clone!(event_id, event_category, scoping, |project| { + .and_then(clone!(event_id, envelope_summary, scoping, |project| { extract_envelope(&request, meta) .into_future() - .and_then(clone!(event_id, event_category, |envelope| { + .and_then(clone!(event_id, envelope_summary, |envelope| { event_id.replace(envelope.event_id()); - event_category.replace(envelope.get_event_category()); + envelope_summary.replace(EnvelopeSummary::compute(&envelope)); if envelope.is_empty() { Err(BadStoreRequest::EmptyEnvelope) @@ -446,6 +446,7 @@ where Some(envelope) => envelope, None => return Err(BadStoreRequest::RateLimited(checked.rate_limits)), }; + envelope_summary.replace(EnvelopeSummary::compute(&envelope)); if check_envelope_size_limits(&config, &envelope) { Ok((envelope, checked.rate_limits)) @@ -530,16 +531,30 @@ where .or_else(move |error: BadStoreRequest| { metric!(counter(RelayCounters::EnvelopeRejected) += 1); - if let Some(category) = *event_category.borrow() { + let envelope_summary = *envelope_summary.borrow(); + if let Some(category) = envelope_summary.event_category { if let Some(outcome) = error.to_outcome() { outcome_producer.do_send(TrackOutcome { timestamp: start_time, scoping: *scoping.borrow(), - outcome, + outcome: outcome.clone(), event_id: *event_id.borrow(), remote_addr, category, + quantity: 1, }); + + if envelope_summary.attachment_quantity > 0 { + outcome_producer.do_send(TrackOutcome { + timestamp: start_time, + scoping: *scoping.borrow(), + outcome, + event_id: *event_id.borrow(), + remote_addr, + category: DataCategory::Attachment, + quantity: envelope_summary.attachment_quantity, + }); + } } } From f10dececdc77376c0a50dfccb709b2fb7cf93925 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Tue, 2 Mar 2021 12:56:05 -0800 Subject: [PATCH 02/16] Small corrections --- relay-server/src/endpoints/common.rs | 31 ++++++++++++++-------------- tests/integration/test_outcome.py | 3 +++ 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/relay-server/src/endpoints/common.rs b/relay-server/src/endpoints/common.rs index 3de5bcfdcec..428bf62c2bb 100644 --- a/relay-server/src/endpoints/common.rs +++ b/relay-server/src/endpoints/common.rs @@ -446,7 +446,8 @@ where Some(envelope) => envelope, None => return Err(BadStoreRequest::RateLimited(checked.rate_limits)), }; - envelope_summary.replace(EnvelopeSummary::compute(&envelope)); + // TODO: Ensure that outcomes are emitted correctly for rate-limited attachments + // so that it is safe to update envelope_summary here. if check_envelope_size_limits(&config, &envelope) { Ok((envelope, checked.rate_limits)) @@ -531,9 +532,9 @@ where .or_else(move |error: BadStoreRequest| { metric!(counter(RelayCounters::EnvelopeRejected) += 1); - let envelope_summary = *envelope_summary.borrow(); - if let Some(category) = envelope_summary.event_category { - if let Some(outcome) = error.to_outcome() { + let envelope_summary = envelope_summary.borrow(); + if let Some(outcome) = error.to_outcome() { + if let Some(category) = envelope_summary.event_category { outcome_producer.do_send(TrackOutcome { timestamp: start_time, scoping: *scoping.borrow(), @@ -543,18 +544,18 @@ where category, quantity: 1, }); + } - if envelope_summary.attachment_quantity > 0 { - outcome_producer.do_send(TrackOutcome { - timestamp: start_time, - scoping: *scoping.borrow(), - outcome, - event_id: *event_id.borrow(), - remote_addr, - category: DataCategory::Attachment, - quantity: envelope_summary.attachment_quantity, - }); - } + if envelope_summary.attachment_quantity > 0 { + outcome_producer.do_send(TrackOutcome { + timestamp: start_time, + scoping: *scoping.borrow(), + outcome, + event_id: *event_id.borrow(), + remote_addr, + category: DataCategory::Attachment, + quantity: envelope_summary.attachment_quantity, + }); } } diff --git a/tests/integration/test_outcome.py b/tests/integration/test_outcome.py index 0efe6cb8524..6f6040fdbdc 100644 --- a/tests/integration/test_outcome.py +++ b/tests/integration/test_outcome.py @@ -104,6 +104,7 @@ def test_outcomes_non_processing(relay, mini_sentry, event_type): "event_id": event_id, "remote_addr": "127.0.0.1", "category": 2 if event_type == "transaction" else 1, + "quantity": 1, } assert outcome == expected_outcome @@ -303,6 +304,7 @@ def test_outcome_forwarding( "event_id": event_id, "remote_addr": "127.0.0.1", "category": 2 if event_type == "transaction" else 1, + "quantity": 1, } outcome.pop("timestamp") @@ -377,6 +379,7 @@ def test_outcomes_forwarding_rate_limited( "event_id": event_id, "source": "processing-layer", "category": 1, + "quantity": 1, } assert outcome == expected_outcome From a19b0999a0fd9060c4f447ac3bea2aa4cd4de268 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Tue, 2 Mar 2021 14:30:20 -0800 Subject: [PATCH 03/16] Emit attachments outcome from events.rs --- relay-server/src/actors/events.rs | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/relay-server/src/actors/events.rs b/relay-server/src/actors/events.rs index 9fbe66ed13f..8b0a9b65271 100644 --- a/relay-server/src/actors/events.rs +++ b/relay-server/src/actors/events.rs @@ -35,7 +35,7 @@ use crate::envelope::{self, AttachmentType, ContentType, Envelope, Item, ItemTyp use crate::http::{HttpError, RequestBuilder}; use crate::metrics::{RelayCounters, RelayHistograms, RelaySets, RelayTimers}; use crate::service::ServerError; -use crate::utils::{self, ChunkedFormDataAggregator, FormDataIter, FutureExt}; +use crate::utils::{self, ChunkedFormDataAggregator, EnvelopeSummary, FormDataIter, FutureExt}; #[cfg(feature = "processing")] use { @@ -1483,12 +1483,13 @@ impl Handler for EventManager { let scoping = Rc::new(RefCell::new(envelope.meta().get_partial_scoping())); let is_received = Rc::new(AtomicBool::from(false)); + let envelope_summary = Rc::new(RefCell::new(EnvelopeSummary::empty())); let future = project .send(CheckEnvelope::fetched(envelope)) .map_err(ProcessingError::ScheduleFailed) .and_then(|result| result.map_err(ProcessingError::ProjectFailed)) - .and_then(clone!(scoping, |response| { + .and_then(clone!(scoping, envelope_summary, |response| { // Use the project id from the loaded project state to account for redirects. let project_id = response.scoping.project_id.value(); metric!(set(RelaySets::UniqueProjects) = project_id as i64); @@ -1497,7 +1498,10 @@ impl Handler for EventManager { let checked = response.result.map_err(ProcessingError::EventRejected)?; match checked.envelope { - Some(envelope) => Ok(envelope), + Some(envelope) => { + envelope_summary.replace(EnvelopeSummary::compute(&envelope)); + Ok(envelope) + } None => Err(ProcessingError::RateLimited(checked.rate_limits)), } })) @@ -1696,12 +1700,25 @@ impl Handler for EventManager { outcome_producer.do_send(TrackOutcome { timestamp: Instant::now(), scoping: *scoping.borrow(), - outcome, + outcome: outcome.clone(), event_id, remote_addr, category, quantity: 1, - }) + }); + + let envelope_summary = envelope_summary.borrow(); + if envelope_summary.attachment_quantity > 0 { + outcome_producer.do_send(TrackOutcome { + timestamp: start_time, + scoping: *scoping.borrow(), + outcome, + event_id: event_id, + remote_addr, + category: DataCategory::Attachment, + quantity: envelope_summary.attachment_quantity, + }); + } } }) .then(move |x, slf, _| { From 292a0f09c3f9dea1040efabdef2e1bd2b75fca89 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Tue, 2 Mar 2021 14:49:04 -0800 Subject: [PATCH 04/16] lint fix --- relay-server/src/actors/events.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/actors/events.rs b/relay-server/src/actors/events.rs index 8b0a9b65271..a79433ba84f 100644 --- a/relay-server/src/actors/events.rs +++ b/relay-server/src/actors/events.rs @@ -1713,7 +1713,7 @@ impl Handler for EventManager { timestamp: start_time, scoping: *scoping.borrow(), outcome, - event_id: event_id, + event_id, remote_addr, category: DataCategory::Attachment, quantity: envelope_summary.attachment_quantity, From 3a1e03c8d545ee9e5f80e3902773694ff1a3d8b5 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Wed, 3 Mar 2021 16:43:07 -0800 Subject: [PATCH 05/16] Better comments on EnvelopeSummary updates --- relay-server/src/actors/events.rs | 1 + relay-server/src/endpoints/common.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/relay-server/src/actors/events.rs b/relay-server/src/actors/events.rs index a79433ba84f..dcb2ec8f1f0 100644 --- a/relay-server/src/actors/events.rs +++ b/relay-server/src/actors/events.rs @@ -1535,6 +1535,7 @@ impl Handler for EventManager { }) .map_err(ProcessingError::ScheduleFailed) .flatten() + // TODO: Update envelope_summary once the rate-limiting code emits its own outcomes. }) .and_then(clone!(project, |processed| { let rate_limits = processed.rate_limits; diff --git a/relay-server/src/endpoints/common.rs b/relay-server/src/endpoints/common.rs index 428bf62c2bb..0d21796100d 100644 --- a/relay-server/src/endpoints/common.rs +++ b/relay-server/src/endpoints/common.rs @@ -446,8 +446,8 @@ where Some(envelope) => envelope, None => return Err(BadStoreRequest::RateLimited(checked.rate_limits)), }; - // TODO: Ensure that outcomes are emitted correctly for rate-limited attachments - // so that it is safe to update envelope_summary here. + // TODO: Update envelope_summary from checked.envelope, once the rate-limiting + // code in CheckEnvelope emits its own outcomes. if check_envelope_size_limits(&config, &envelope) { Ok((envelope, checked.rate_limits)) From 948fbb231567d7bfac1fecc9bbdd1d79de7f25a2 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Wed, 3 Mar 2021 16:32:22 -0800 Subject: [PATCH 06/16] Preserve attachment outcome if envelope does not have an event category --- relay-server/src/actors/events.rs | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/relay-server/src/actors/events.rs b/relay-server/src/actors/events.rs index dcb2ec8f1f0..ba0e13307cc 100644 --- a/relay-server/src/actors/events.rs +++ b/relay-server/src/actors/events.rs @@ -1673,13 +1673,6 @@ impl Handler for EventManager { } } - // Envelopes not containing events (such as standalone attachment uploads or user - // reports) should never create outcomes. - let category = match event_category { - Some(event_category) => event_category, - None => return, - }; - let outcome = error.to_outcome(); if let Some(Outcome::Invalid(DiscardReason::Internal)) = outcome { // Errors are only logged for what we consider an internal discard reason. These @@ -1698,15 +1691,17 @@ impl Handler for EventManager { } if let Some(outcome) = outcome { - outcome_producer.do_send(TrackOutcome { - timestamp: Instant::now(), - scoping: *scoping.borrow(), - outcome: outcome.clone(), - event_id, - remote_addr, - category, - quantity: 1, - }); + if let Some(category) = event_category { + outcome_producer.do_send(TrackOutcome { + timestamp: Instant::now(), + scoping: *scoping.borrow(), + outcome: outcome.clone(), + event_id, + remote_addr, + category, + quantity: 1, + }); + } let envelope_summary = envelope_summary.borrow(); if envelope_summary.attachment_quantity > 0 { From d257433b714b24ddd8eb801e21dc5574f82d5c33 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Wed, 3 Mar 2021 15:50:04 -0800 Subject: [PATCH 07/16] Use envelope summary, replacing Envelope::get_data_category --- relay-server/src/actors/events.rs | 5 ++--- relay-server/src/envelope.rs | 10 +--------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/relay-server/src/actors/events.rs b/relay-server/src/actors/events.rs index ba0e13307cc..a8851b9c354 100644 --- a/relay-server/src/actors/events.rs +++ b/relay-server/src/actors/events.rs @@ -1476,7 +1476,6 @@ impl Handler for EventManager { start_time, sampling_project, } = message; - let event_category = envelope.get_event_category(); let event_id = envelope.event_id(); let remote_addr = envelope.meta().client_addr(); @@ -1690,8 +1689,9 @@ impl Handler for EventManager { return; } + let envelope_summary = envelope_summary.borrow(); if let Some(outcome) = outcome { - if let Some(category) = event_category { + if let Some(category) = envelope_summary.event_category { outcome_producer.do_send(TrackOutcome { timestamp: Instant::now(), scoping: *scoping.borrow(), @@ -1703,7 +1703,6 @@ impl Handler for EventManager { }); } - let envelope_summary = envelope_summary.borrow(); if envelope_summary.attachment_quantity > 0 { outcome_producer.do_send(TrackOutcome { timestamp: start_time, diff --git a/relay-server/src/envelope.rs b/relay-server/src/envelope.rs index ca23316e9fd..b2c49f7e02c 100644 --- a/relay-server/src/envelope.rs +++ b/relay-server/src/envelope.rs @@ -41,14 +41,13 @@ use failure::Fail; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use smallvec::SmallVec; -use relay_common::DataCategory; use relay_general::protocol::{EventId, EventType}; use relay_general::types::Value; use relay_sampling::TraceContext; use crate::constants::DEFAULT_EVENT_RETENTION; use crate::extractors::{PartialMeta, RequestMeta}; -use crate::utils::{infer_event_category, ErrorBoundary}; +use crate::utils::ErrorBoundary; pub const CONTENT_TYPE: &str = "application/x-sentry-envelope"; @@ -956,13 +955,6 @@ impl Envelope { .write_all(buf) .map_err(EnvelopeError::PayloadIoFailed) } - - /// Return the data category type of the event item, if any, in this envelope. - pub fn get_event_category(&self) -> Option { - self.items().find_map(infer_event_category) - // There are some cases where multiple items may have different categories, but returning - // the first is good enough for now. - } } #[cfg(test)] From f58dbf0725ce4b45a50d8dae977ef6dd420d34fc Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Wed, 3 Mar 2021 18:06:32 -0800 Subject: [PATCH 08/16] fix: initialize envelope_summary at start --- relay-server/src/actors/events.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/actors/events.rs b/relay-server/src/actors/events.rs index a8851b9c354..de6bd985467 100644 --- a/relay-server/src/actors/events.rs +++ b/relay-server/src/actors/events.rs @@ -1482,7 +1482,7 @@ impl Handler for EventManager { let scoping = Rc::new(RefCell::new(envelope.meta().get_partial_scoping())); let is_received = Rc::new(AtomicBool::from(false)); - let envelope_summary = Rc::new(RefCell::new(EnvelopeSummary::empty())); + let envelope_summary = Rc::new(RefCell::new(EnvelopeSummary::compute(&envelope))); let future = project .send(CheckEnvelope::fetched(envelope)) From 30bc59a46ab1fd70ca70feedc160cc9e31910be7 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Thu, 4 Mar 2021 14:11:23 -0800 Subject: [PATCH 09/16] Allow for multiple outcomes in OutcomesConsumer --- tests/integration/fixtures/processing.py | 40 ++++++++++++++++++------ 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/tests/integration/fixtures/processing.py b/tests/integration/fixtures/processing.py index ee7bbe17cda..1710ba57c65 100644 --- a/tests/integration/fixtures/processing.py +++ b/tests/integration/fixtures/processing.py @@ -206,23 +206,43 @@ def category_value(category): class OutcomesConsumer(ConsumerBase): + def _poll_all(self): + while True: + outcome = self.poll() + if outcome is None: + return + else: + yield outcome + + def get_outcomes(self): + outcomes = list(self._poll_all()) + for outcome in outcomes: + assert outcome.error() is None + return [json.loads(outcome.value()) for outcome in outcomes] + def get_outcome(self): - outcome = self.poll() - assert outcome is not None - assert outcome.error() is None - return json.loads(outcome.value()) + outcomes = self.get_outcomes() + assert len(outcomes) > 0, "No outcomes were consumed" + assert len(outcomes) == 1, "More than one outcome was consumed" + return outcomes[0] def assert_rate_limited(self, reason, key_id=None, category=None): - outcome = self.get_outcome() + if category is None: + outcome = self.get_outcome() + assert isinstance(outcome["category"], int) + else: + value = category_value(category) + outcomes = self.get_outcomes() + outcomes = [outcome for outcome in outcomes if outcome["category"] == value] + assert len(outcomes) == 1, "Categories are {!r} (expected {!r})".format( + [outcome["category"] for outcome in outcomes], value + ) + outcome = outcomes[0] + assert outcome["outcome"] == 2, outcome assert outcome["reason"] == reason if key_id is not None: assert outcome["key_id"] == key_id - if category is not None: - value = category_value(category) - assert outcome["category"] == value, outcome["category"] - else: - assert isinstance(outcome["category"], int) def assert_dropped_internal(self): outcome = self.get_outcome() From b14efc07f4ac4ddf6c2a5015c9792a34a8862bc9 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Thu, 4 Mar 2021 15:20:56 -0800 Subject: [PATCH 10/16] Check multiple categories in assert_rate_limited --- tests/integration/fixtures/processing.py | 23 +++++++-------- tests/integration/test_minidump.py | 8 +++-- tests/integration/test_store.py | 37 +++++++++++++++++++----- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/tests/integration/fixtures/processing.py b/tests/integration/fixtures/processing.py index 1710ba57c65..b0f48f5697c 100644 --- a/tests/integration/fixtures/processing.py +++ b/tests/integration/fixtures/processing.py @@ -226,23 +226,22 @@ def get_outcome(self): assert len(outcomes) == 1, "More than one outcome was consumed" return outcomes[0] - def assert_rate_limited(self, reason, key_id=None, category=None): - if category is None: + def assert_rate_limited(self, reason, key_id=None, categories=None): + if categories is None: outcome = self.get_outcome() assert isinstance(outcome["category"], int) + outcomes = [outcome] else: - value = category_value(category) outcomes = self.get_outcomes() - outcomes = [outcome for outcome in outcomes if outcome["category"] == value] - assert len(outcomes) == 1, "Categories are {!r} (expected {!r})".format( - [outcome["category"] for outcome in outcomes], value - ) - outcome = outcomes[0] + expected = set(category_value(category) for category in categories) + actual = set(outcome["category"] for outcome in outcomes) + assert actual == expected, (actual, expected) - assert outcome["outcome"] == 2, outcome - assert outcome["reason"] == reason - if key_id is not None: - assert outcome["key_id"] == key_id + for outcome in outcomes: + assert outcome["outcome"] == 2, outcome + assert outcome["reason"] == reason + if key_id is not None: + assert outcome["key_id"] == key_id def assert_dropped_internal(self): outcome = self.get_outcome() diff --git a/tests/integration/test_minidump.py b/tests/integration/test_minidump.py index cc77f407d5b..3622faa6009 100644 --- a/tests/integration/test_minidump.py +++ b/tests/integration/test_minidump.py @@ -464,11 +464,15 @@ def test_minidump_ratelimit( # First minidump returns 200 but is rate limited in processing relay.send_minidump(project_id=project_id, files=attachments) - outcomes_consumer.assert_rate_limited("static_disabled_quota", category="error") + outcomes_consumer.assert_rate_limited( + "static_disabled_quota", categories=["error", "attachment"] + ) # Minidumps never return rate limits relay.send_minidump(project_id=project_id, files=attachments) - outcomes_consumer.assert_rate_limited("static_disabled_quota", category="error") + outcomes_consumer.assert_rate_limited( + "static_disabled_quota", categories=["error", "attachment"] + ) def test_crashpad_annotations(mini_sentry, relay_with_processing, attachments_consumer): diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index 302561f5519..85f2163e250 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -65,10 +65,17 @@ def test_legacy_store(mini_sentry, relay_chain): ({"errorMessages": {"patterns": ["Panic: originalCreateNotification"]}}, True), ({"errorMessages": {"patterns": ["Warning"]}}, False), ], - ids=["error messages filtered", "error messages not filtered",], + ids=[ + "error messages filtered", + "error messages not filtered", + ], ) def test_filters_are_applied( - mini_sentry, relay_with_processing, events_consumer, filter_config, should_filter, + mini_sentry, + relay_with_processing, + events_consumer, + filter_config, + should_filter, ): """ Test that relay normalizes messages when processing is enabled and sends them via Kafka queues @@ -103,11 +110,21 @@ def test_filters_are_applied( @pytest.mark.parametrize( "is_enabled, should_filter", - [(True, True), (False, False),], - ids=["web crawlers filtered", "web crawlers not filtered",], + [ + (True, True), + (False, False), + ], + ids=[ + "web crawlers filtered", + "web crawlers not filtered", + ], ) def test_web_crawlers_filter_are_applied( - mini_sentry, relay_with_processing, events_consumer, is_enabled, should_filter, + mini_sentry, + relay_with_processing, + events_consumer, + is_enabled, + should_filter, ): """ Test that relay normalizes messages when processing is enabled and sends them via Kafka queues @@ -127,7 +144,11 @@ def test_web_crawlers_filter_are_applied( event = { "message": message_text, - "request": {"headers": {"User-Agent": "BingBot",}}, + "request": { + "headers": { + "User-Agent": "BingBot", + } + }, } relay.send_event(project_id, event) @@ -538,7 +559,7 @@ def test_processing_quotas( if outcomes_consumer is not None: outcomes_consumer.assert_rate_limited( - "get_lost", key_id=key_id, category=category + "get_lost", key_id=key_id, categories=[category] ) else: # since we don't wait for the outcome, wait a little for the event to go through @@ -558,7 +579,7 @@ def test_processing_quotas( assert rest == "%s:key:get_lost" % category if outcomes_consumer is not None: outcomes_consumer.assert_rate_limited( - "get_lost", key_id=key_id, category=category + "get_lost", key_id=key_id, categories=[category] ) for i in range(10): From 17449e151efc501a0bd298748de4b23c8cb90f00 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Thu, 4 Mar 2021 15:28:19 -0800 Subject: [PATCH 11/16] Lint from correct Black version --- tests/integration/test_store.py | 33 ++++++--------------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index 85f2163e250..89aa101c80b 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -65,17 +65,10 @@ def test_legacy_store(mini_sentry, relay_chain): ({"errorMessages": {"patterns": ["Panic: originalCreateNotification"]}}, True), ({"errorMessages": {"patterns": ["Warning"]}}, False), ], - ids=[ - "error messages filtered", - "error messages not filtered", - ], + ids=["error messages filtered", "error messages not filtered",], ) def test_filters_are_applied( - mini_sentry, - relay_with_processing, - events_consumer, - filter_config, - should_filter, + mini_sentry, relay_with_processing, events_consumer, filter_config, should_filter, ): """ Test that relay normalizes messages when processing is enabled and sends them via Kafka queues @@ -110,21 +103,11 @@ def test_filters_are_applied( @pytest.mark.parametrize( "is_enabled, should_filter", - [ - (True, True), - (False, False), - ], - ids=[ - "web crawlers filtered", - "web crawlers not filtered", - ], + [(True, True), (False, False),], + ids=["web crawlers filtered", "web crawlers not filtered",], ) def test_web_crawlers_filter_are_applied( - mini_sentry, - relay_with_processing, - events_consumer, - is_enabled, - should_filter, + mini_sentry, relay_with_processing, events_consumer, is_enabled, should_filter, ): """ Test that relay normalizes messages when processing is enabled and sends them via Kafka queues @@ -144,11 +127,7 @@ def test_web_crawlers_filter_are_applied( event = { "message": message_text, - "request": { - "headers": { - "User-Agent": "BingBot", - } - }, + "request": {"headers": {"User-Agent": "BingBot",}}, } relay.send_event(project_id, event) From ea1df32edf3286bccfbbafc12f20d52a412446ac Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Thu, 4 Mar 2021 16:14:21 -0800 Subject: [PATCH 12/16] Add changelog entry for #942 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e0878e7a3b..ee5bbe153a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ - Add inbound filters functionality to dynamic sampling rules. ([#920](https://github.com/getsentry/relay/pull/920)) - The undocumented `http._client` option has been removed. ([#938](https://github.com/getsentry/relay/pull/938)) - Log old events and sessions in the `requests.timestamp_delay` metric. ([#933](https://github.com/getsentry/relay/pull/933)) +- Emit the `quantity` field for outcomes of events. This field describes the total size in bytes of attachments and the event count for all other categories. Emit a separate outcome for attachments in a rejected envelope, in any, in addition to the event outcome. ([#942](https://github.com/getsentry/relay/pull/942)) ## 21.2.0 From 7f6e7e002da5bac2b9589850a7fc929ef359a770 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Thu, 4 Mar 2021 16:39:26 -0800 Subject: [PATCH 13/16] Fix typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee5bbe153a2..61b7e1d0596 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ - Add inbound filters functionality to dynamic sampling rules. ([#920](https://github.com/getsentry/relay/pull/920)) - The undocumented `http._client` option has been removed. ([#938](https://github.com/getsentry/relay/pull/938)) - Log old events and sessions in the `requests.timestamp_delay` metric. ([#933](https://github.com/getsentry/relay/pull/933)) -- Emit the `quantity` field for outcomes of events. This field describes the total size in bytes of attachments and the event count for all other categories. Emit a separate outcome for attachments in a rejected envelope, in any, in addition to the event outcome. ([#942](https://github.com/getsentry/relay/pull/942)) +- Emit the `quantity` field for outcomes of events. This field describes the total size in bytes of attachments and the event count for all other categories. Emit a separate outcome for attachments in a rejected envelope, if any, in addition to the event outcome. ([#942](https://github.com/getsentry/relay/pull/942)) ## 21.2.0 From efb78251b471da01aa2b4b0caf0764d204355353 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Fri, 5 Mar 2021 10:40:35 -0800 Subject: [PATCH 14/16] Revert infer_event_category to private --- relay-server/src/utils/rate_limits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/utils/rate_limits.rs b/relay-server/src/utils/rate_limits.rs index d212cdd877b..c488e603c13 100644 --- a/relay-server/src/utils/rate_limits.rs +++ b/relay-server/src/utils/rate_limits.rs @@ -85,7 +85,7 @@ pub fn parse_rate_limits(scoping: &Scoping, string: &str) -> RateLimits { /// to be set on the event item. /// - `Attachment`: If the attachment creates an event (e.g. for minidumps), the category is assumed /// to be `Error`. -pub fn infer_event_category(item: &Item) -> Option { +fn infer_event_category(item: &Item) -> Option { match item.ty() { ItemType::Event => Some(DataCategory::Error), ItemType::Transaction => Some(DataCategory::Transaction), From 1a9691df25a81f25df5859990d9ea8eba77fd7ad Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Tue, 9 Mar 2021 16:06:24 -0800 Subject: [PATCH 15/16] Combine changelog entries to category and quantity fields --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b39417a740..efb7fa5114f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,12 +15,11 @@ **Internal**: -- Emit the `category` field for outcomes of events. This field disambiguates error events, security events and transactions. As a side-effect, Relay no longer emits outcomes for broken JSON payloads or network errors. ([#931](https://github.com/getsentry/relay/pull/931)) +- Emit the `category` and `quantity` fields for outcomes of events. The `category` field disambiguates error events, security events and transactions. As a side-effect, Relay no longer emits outcomes for broken JSON payloads or network errors. The `quantity` field describes the total size in bytes for attachments or the event count for all other categories. A separate outcome is emitted for attachments in a rejected envelope, if any, in addition to the event outcome. ([#931](https://github.com/getsentry/relay/pull/931), [#942](https://github.com/getsentry/relay/pull/942)) - Add inbound filters functionality to dynamic sampling rules. ([#920](https://github.com/getsentry/relay/pull/920)) - The undocumented `http._client` option has been removed. ([#938](https://github.com/getsentry/relay/pull/938)) - Log old events and sessions in the `requests.timestamp_delay` metric. ([#933](https://github.com/getsentry/relay/pull/933)) - Add rule id to outcomes coming from event sampling. ([#943](https://github.com/getsentry/relay/pull/943)) -- Emit the `quantity` field for outcomes of events. This field describes the total size in bytes of attachments and the event count for all other categories. Emit a separate outcome for attachments in a rejected envelope, if any, in addition to the event outcome. ([#942](https://github.com/getsentry/relay/pull/942)) ## 21.2.0 From f121228534722ccc6c7b8592e04857de3f3f51b9 Mon Sep 17 00:00:00 2001 From: Ryan Skonnord Date: Tue, 16 Mar 2021 07:37:09 -0700 Subject: [PATCH 16/16] Move new changelog entry to unreleased --- CHANGELOG.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4673642b83e..0d6b7058db8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +**Internal**: + +- Emit the `quantity` field for outcomes of events. This field describes the total size in bytes for attachments or the event count for all other categories. A separate outcome is emitted for attachments in a rejected envelope, if any, in addition to the event outcome. ([#942](https://github.com/getsentry/relay/pull/942)) + ## 21.3.0 **Features**: @@ -16,7 +22,7 @@ **Internal**: -- Emit the `category` and `quantity` fields for outcomes of events. The `category` field disambiguates error events, security events and transactions. As a side-effect, Relay no longer emits outcomes for broken JSON payloads or network errors. The `quantity` field describes the total size in bytes for attachments or the event count for all other categories. A separate outcome is emitted for attachments in a rejected envelope, if any, in addition to the event outcome. ([#931](https://github.com/getsentry/relay/pull/931), [#942](https://github.com/getsentry/relay/pull/942)) +- Emit the `category` field for outcomes of events. This field disambiguates error events, security events and transactions. As a side-effect, Relay no longer emits outcomes for broken JSON payloads or network errors. ([#931](https://github.com/getsentry/relay/pull/931)) - Add inbound filters functionality to dynamic sampling rules. ([#920](https://github.com/getsentry/relay/pull/920)) - The undocumented `http._client` option has been removed. ([#938](https://github.com/getsentry/relay/pull/938)) - Log old events and sessions in the `requests.timestamp_delay` metric. ([#933](https://github.com/getsentry/relay/pull/933))