From c7d474f587918d007e79198303601143481c7fd6 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 14 Nov 2024 09:33:45 -0600 Subject: [PATCH 1/9] Base64 encode recording payload on error and log to GCP --- Cargo.lock | 1 + Cargo.toml | 1 + relay-server/Cargo.toml | 1 + relay-server/src/services/processor/replay.rs | 30 +++++++++++++++++++ 4 files changed, 33 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 64ac5c6d7f..d5c68da3bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3761,6 +3761,7 @@ dependencies = [ "axum-extra", "axum-server", "backoff", + "base64 0.22.1", "brotli", "bytes", "bzip2", diff --git a/Cargo.toml b/Cargo.toml index 0626ba44f6..60c89bd8c0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -68,6 +68,7 @@ axum-extra = "0.9.3" axum-server = "0.7.1" arc-swap = "1.7.1" backoff = "0.4.0" +base64 = "0.22.1" bindgen = "0.70.1" brotli = "6.0.0" bytecount = "0.6.0" diff --git a/relay-server/Cargo.toml b/relay-server/Cargo.toml index 0e24b7a312..56c5276da0 100644 --- a/relay-server/Cargo.toml +++ b/relay-server/Cargo.toml @@ -37,6 +37,7 @@ axum-extra = { workspace = true } axum-server = { workspace = true } arc-swap = { workspace = true } backoff = { workspace = true } +base64 = { workspace = true } brotli = { workspace = true } bytes = { workspace = true, features = ["serde"] } bzip2 = { workspace = true } diff --git a/relay-server/src/services/processor/replay.rs b/relay-server/src/services/processor/replay.rs index 9f757814dc..d61a7cc3ad 100644 --- a/relay-server/src/services/processor/replay.rs +++ b/relay-server/src/services/processor/replay.rs @@ -1,8 +1,10 @@ //! Replay related processor code. +use base64::prelude::*; use std::error::Error; use std::net::IpAddr; use bytes::Bytes; +use rand::Rng; use relay_base_schema::organization::OrganizationId; use relay_base_schema::project::ProjectId; use relay_dynamic_config::{Feature, GlobalConfig, ProjectConfig}; @@ -262,6 +264,33 @@ fn handle_replay_recording_item( }) .map(Into::into) .map_err(|error| { + match &error { + relay_replays::recording::ParseRecordingError::Compression(e) => { + // 20k errors per day at 0.1% sample rate == 20 logs per day + if rand::thread_rng().gen_range(1..=1000) == 1 { + relay_log::warn!( + error = e as &dyn Error, + event_id = ?config.event_id, + project_id = config.project_id.map(|v| v.value()), + organization_id = config.organization_id.map(|o| o.value()), + encoded_payload = BASE64_STANDARD.encode(payload), + "invalid replay recording ParseRecordingError::Compression" + ); + } + } + relay_replays::recording::ParseRecordingError::Message(_) => { + // Only 118 errors in the past 30 days. We log everything. + relay_log::warn!( + event_id = ?config.event_id, + project_id = config.project_id.map(|v| v.value()), + organization_id = config.organization_id.map(|o| o.value()), + encoded_payload = BASE64_STANDARD.encode(payload), + "invalid replay recording ParseRecordingError::Message" + ); + } + _ => (), + }; + relay_log::error!( error = &error as &dyn Error, event_id = ?config.event_id, @@ -269,6 +298,7 @@ fn handle_replay_recording_item( organization_id = config.organization_id.map(|o| o.value()), "invalid replay recording" ); + ProcessingError::InvalidReplay(DiscardReason::InvalidReplayRecordingEvent) }) } From 18a396ac24976448f523fe0fa4129748d129da62 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 14 Nov 2024 09:36:07 -0600 Subject: [PATCH 2/9] Fix import order --- relay-server/src/services/processor/replay.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/services/processor/replay.rs b/relay-server/src/services/processor/replay.rs index d61a7cc3ad..1415fde734 100644 --- a/relay-server/src/services/processor/replay.rs +++ b/relay-server/src/services/processor/replay.rs @@ -1,8 +1,8 @@ //! Replay related processor code. -use base64::prelude::*; use std::error::Error; use std::net::IpAddr; +use base64::prelude::*; use bytes::Bytes; use rand::Rng; use relay_base_schema::organization::OrganizationId; From b86b33fc5b61c69a8c8a7e461f020a87f37ce516 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 14 Nov 2024 09:58:53 -0600 Subject: [PATCH 3/9] Include error message --- relay-server/src/services/processor/replay.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/relay-server/src/services/processor/replay.rs b/relay-server/src/services/processor/replay.rs index 1415fde734..a986103f93 100644 --- a/relay-server/src/services/processor/replay.rs +++ b/relay-server/src/services/processor/replay.rs @@ -278,9 +278,10 @@ fn handle_replay_recording_item( ); } } - relay_replays::recording::ParseRecordingError::Message(_) => { + relay_replays::recording::ParseRecordingError::Message(e) => { // Only 118 errors in the past 30 days. We log everything. relay_log::warn!( + error = e, event_id = ?config.event_id, project_id = config.project_id.map(|v| v.value()), organization_id = config.organization_id.map(|o| o.value()), From 9df4c9a003f69377bbcf02ccb5b6ade652d54e01 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 14 Nov 2024 13:36:49 -0600 Subject: [PATCH 4/9] Use attachment --- Cargo.lock | 1 - Cargo.toml | 1 - relay-server/Cargo.toml | 1 - relay-server/src/services/processor/replay.rs | 51 +++++++++++++------ 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d5c68da3bf..64ac5c6d7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3761,7 +3761,6 @@ dependencies = [ "axum-extra", "axum-server", "backoff", - "base64 0.22.1", "brotli", "bytes", "bzip2", diff --git a/Cargo.toml b/Cargo.toml index 60c89bd8c0..0626ba44f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -68,7 +68,6 @@ axum-extra = "0.9.3" axum-server = "0.7.1" arc-swap = "1.7.1" backoff = "0.4.0" -base64 = "0.22.1" bindgen = "0.70.1" brotli = "6.0.0" bytecount = "0.6.0" diff --git a/relay-server/Cargo.toml b/relay-server/Cargo.toml index 56c5276da0..0e24b7a312 100644 --- a/relay-server/Cargo.toml +++ b/relay-server/Cargo.toml @@ -37,7 +37,6 @@ axum-extra = { workspace = true } axum-server = { workspace = true } arc-swap = { workspace = true } backoff = { workspace = true } -base64 = { workspace = true } brotli = { workspace = true } bytes = { workspace = true, features = ["serde"] } bzip2 = { workspace = true } diff --git a/relay-server/src/services/processor/replay.rs b/relay-server/src/services/processor/replay.rs index a986103f93..ec02a6699d 100644 --- a/relay-server/src/services/processor/replay.rs +++ b/relay-server/src/services/processor/replay.rs @@ -2,7 +2,6 @@ use std::error::Error; use std::net::IpAddr; -use base64::prelude::*; use bytes::Bytes; use rand::Rng; use relay_base_schema::organization::OrganizationId; @@ -268,25 +267,47 @@ fn handle_replay_recording_item( relay_replays::recording::ParseRecordingError::Compression(e) => { // 20k errors per day at 0.1% sample rate == 20 logs per day if rand::thread_rng().gen_range(1..=1000) == 1 { - relay_log::warn!( - error = e as &dyn Error, - event_id = ?config.event_id, - project_id = config.project_id.map(|v| v.value()), - organization_id = config.organization_id.map(|o| o.value()), - encoded_payload = BASE64_STANDARD.encode(payload), - "invalid replay recording ParseRecordingError::Compression" + relay_log::with_scope( + move |scope| { + scope.add_attachment(relay_log::protocol::Attachment { + buffer: payload.into(), + filename: "payload.json".to_owned(), + content_type: Some("application/octet-stream".to_owned()), + ty: None, + }); + }, + || { + relay_log::error!( + error = e as &dyn Error, + event_id = ?config.event_id, + project_id = config.project_id.map(|v| v.value()), + organization_id = config.organization_id.map(|o| o.value()), + "ParseRecordingError::Compression" + ) + }, ); } } relay_replays::recording::ParseRecordingError::Message(e) => { // Only 118 errors in the past 30 days. We log everything. - relay_log::warn!( - error = e, - event_id = ?config.event_id, - project_id = config.project_id.map(|v| v.value()), - organization_id = config.organization_id.map(|o| o.value()), - encoded_payload = BASE64_STANDARD.encode(payload), - "invalid replay recording ParseRecordingError::Message" + relay_log::with_scope( + move |scope| { + scope.add_attachment(relay_log::protocol::Attachment { + buffer: payload.into(), + filename: "payload.json".to_owned(), + content_type: Some("application/octet-stream".to_owned()), + ty: None, + }); + }, + || { + relay_log::error!( + error = e, + event_id = ?config.event_id, + project_id = config.project_id.map(|v| v.value()), + organization_id = config.organization_id.map(|o| o.value()), + "ParseRecordingError::Compression" + ) + }, ); } _ => (), From e269bcd1f54f24cab8fd03c788d283c4a504c848 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 14 Nov 2024 13:50:44 -0600 Subject: [PATCH 5/9] Use sample function --- relay-server/src/services/processor/replay.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/relay-server/src/services/processor/replay.rs b/relay-server/src/services/processor/replay.rs index ec02a6699d..0513555a45 100644 --- a/relay-server/src/services/processor/replay.rs +++ b/relay-server/src/services/processor/replay.rs @@ -3,7 +3,6 @@ use std::error::Error; use std::net::IpAddr; use bytes::Bytes; -use rand::Rng; use relay_base_schema::organization::OrganizationId; use relay_base_schema::project::ProjectId; use relay_dynamic_config::{Feature, GlobalConfig, ProjectConfig}; @@ -21,6 +20,7 @@ use crate::envelope::{ContentType, ItemType}; use crate::services::outcome::DiscardReason; use crate::services::processor::{ProcessEnvelopeState, ProcessingError, ReplayGroup}; use crate::statsd::{RelayCounters, RelayTimers}; +use crate::utils::sample; /// Removes replays if the feature flag is not enabled. pub fn process( @@ -266,7 +266,7 @@ fn handle_replay_recording_item( match &error { relay_replays::recording::ParseRecordingError::Compression(e) => { // 20k errors per day at 0.1% sample rate == 20 logs per day - if rand::thread_rng().gen_range(1..=1000) == 1 { + if sample(0.001) { relay_log::with_scope( move |scope| { scope.add_attachment(relay_log::protocol::Attachment { From c7fc896fa47ebf0744125a12ab52b059ce1cdd3e Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 14 Nov 2024 13:51:37 -0600 Subject: [PATCH 6/9] Remove redundant logging --- relay-server/src/services/processor/replay.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/relay-server/src/services/processor/replay.rs b/relay-server/src/services/processor/replay.rs index 0513555a45..1239c376e6 100644 --- a/relay-server/src/services/processor/replay.rs +++ b/relay-server/src/services/processor/replay.rs @@ -312,15 +312,6 @@ fn handle_replay_recording_item( } _ => (), }; - - relay_log::error!( - error = &error as &dyn Error, - event_id = ?config.event_id, - project_id = config.project_id.map(|v| v.value()), - organization_id = config.organization_id.map(|o| o.value()), - "invalid replay recording" - ); - ProcessingError::InvalidReplay(DiscardReason::InvalidReplayRecordingEvent) }) } From dcfc16339839dafb743f827dac3c22362679227a Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 15 Nov 2024 07:36:55 -0600 Subject: [PATCH 7/9] Naming Co-authored-by: David Herberth --- relay-server/src/services/processor/replay.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/services/processor/replay.rs b/relay-server/src/services/processor/replay.rs index 1239c376e6..9c63e3a8e8 100644 --- a/relay-server/src/services/processor/replay.rs +++ b/relay-server/src/services/processor/replay.rs @@ -271,7 +271,7 @@ fn handle_replay_recording_item( move |scope| { scope.add_attachment(relay_log::protocol::Attachment { buffer: payload.into(), - filename: "payload.json".to_owned(), + filename: "payload".to_owned(), content_type: Some("application/octet-stream".to_owned()), ty: None, }); From a602712bc40a2393b5bcb19a34e336c7739ba5fe Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 15 Nov 2024 07:37:04 -0600 Subject: [PATCH 8/9] Naming Co-authored-by: David Herberth --- relay-server/src/services/processor/replay.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/services/processor/replay.rs b/relay-server/src/services/processor/replay.rs index 9c63e3a8e8..f4154e6b67 100644 --- a/relay-server/src/services/processor/replay.rs +++ b/relay-server/src/services/processor/replay.rs @@ -294,7 +294,7 @@ fn handle_replay_recording_item( move |scope| { scope.add_attachment(relay_log::protocol::Attachment { buffer: payload.into(), - filename: "payload.json".to_owned(), + filename: "payload".to_owned(), content_type: Some("application/octet-stream".to_owned()), ty: None, }); From 184c456e03e5d7e041d60f35ced601f9d8fe3fb7 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 15 Nov 2024 07:37:34 -0600 Subject: [PATCH 9/9] Use correct enumeration value Co-authored-by: David Herberth --- relay-server/src/services/processor/replay.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay-server/src/services/processor/replay.rs b/relay-server/src/services/processor/replay.rs index f4154e6b67..97e53d0da6 100644 --- a/relay-server/src/services/processor/replay.rs +++ b/relay-server/src/services/processor/replay.rs @@ -305,7 +305,7 @@ fn handle_replay_recording_item( event_id = ?config.event_id, project_id = config.project_id.map(|v| v.value()), organization_id = config.organization_id.map(|o| o.value()), - "ParseRecordingError::Compression" + "ParseRecordingError::Message" ) }, );