From 4df7283f3419b72c647792bbea66a3cec26fbee7 Mon Sep 17 00:00:00 2001 From: David Herberth Date: Thu, 20 Mar 2025 10:27:40 +0100 Subject: [PATCH 1/3] fix(profiling): Classify profile chunks for rate limits and outcomes --- CHANGELOG.md | 4 + Cargo.lock | 1 + relay-profiling/Cargo.toml | 1 + relay-profiling/src/lib.rs | 99 +++++++++----- relay-profiling/src/sample/mod.rs | 2 +- relay-server/src/envelope.rs | 24 +++- relay-server/src/services/processor.rs | 22 +++- .../src/services/processor/profile_chunk.rs | 60 +++++---- relay-server/src/utils/managed_envelope.rs | 8 ++ relay-server/src/utils/rate_limits.rs | 98 ++++++++++++-- tests/integration/test_filters.py | 7 +- tests/integration/test_profile_chunks.py | 121 +++--------------- 12 files changed, 270 insertions(+), 177 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16c02e55855..8b8277f6233 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Add experimental playstation endpoint. ([#4555](https://github.com/getsentry/relay/pull/4555)) +**Bug Fixes**: + +- Separates profiles into backend and ui profiles. ([#4595](https://github.com/getsentry/relay/pull/4595)) + **Internal**: - Add ui chunk profiling data category. ([#4593](https://github.com/getsentry/relay/pull/4593)) diff --git a/Cargo.lock b/Cargo.lock index 818df1e11e7..ec235d1ebd4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3828,6 +3828,7 @@ name = "relay-profiling" version = "25.3.0" dependencies = [ "android_trace_log", + "bytes", "chrono", "data-encoding", "insta", diff --git a/relay-profiling/Cargo.toml b/relay-profiling/Cargo.toml index 7533709fef0..b7515c8b260 100644 --- a/relay-profiling/Cargo.toml +++ b/relay-profiling/Cargo.toml @@ -14,6 +14,7 @@ workspace = true [dependencies] android_trace_log = { workspace = true, features = ["serde"] } +bytes = { workspace = true } chrono = { workspace = true } data-encoding = { workspace = true } itertools = { workspace = true } diff --git a/relay-profiling/src/lib.rs b/relay-profiling/src/lib.rs index 398aafc1661..53854009a0f 100644 --- a/relay-profiling/src/lib.rs +++ b/relay-profiling/src/lib.rs @@ -42,6 +42,8 @@ use std::error::Error; use std::net::IpAddr; use std::time::Duration; + +use bytes::Bytes; use url::Url; use relay_base_schema::project::ProjectId; @@ -80,6 +82,12 @@ const MAX_PROFILE_CHUNK_DURATION: Duration = Duration::from_secs(66); /// Same format as event IDs. pub type ProfileId = EventId; +#[derive(Debug, Clone, Copy)] +pub enum ProfileType { + Backend, + Ui, +} + #[derive(Debug, Deserialize)] struct MinimalProfile { #[serde(alias = "profile_id", alias = "chunk_id")] @@ -275,41 +283,72 @@ pub fn expand_profile( } } -pub fn expand_profile_chunk( - payload: &[u8], - client_ip: Option, - filter_settings: &ProjectFiltersConfig, - global_config: &GlobalConfig, -) -> Result, ProfileError> { - let profile = match minimal_profile_from_json(payload) { - Ok(profile) => profile, - Err(err) => { - relay_log::warn!( - error = &err as &dyn Error, - from = "minimal", - "invalid profile chunk", - ); - return Err(ProfileError::InvalidJson(err)); +/// Intermediate type for all processing on a profile chunk. +pub struct ProfileChunk { + profile: MinimalProfile, + payload: Bytes, +} + +impl ProfileChunk { + /// Parses a new [`Self`] from raw bytes. + pub fn new(payload: Bytes) -> Result { + match minimal_profile_from_json(&payload) { + Ok(profile) => Ok(Self { profile, payload }), + Err(err) => { + relay_log::debug!( + error = &err as &dyn Error, + from = "minimal", + "invalid profile chunk", + ); + Err(ProfileError::InvalidJson(err)) + } } - }; + } - if let Err(filter_stat_key) = relay_filter::should_filter( - &profile, - client_ip, - filter_settings, - global_config.filters(), - ) { - return Err(ProfileError::Filtered(filter_stat_key)); + /// Returns the [`ProfileType`] this chunk belongs to. + /// + /// The profile type is currently determined based on the contained profile + /// platform. It determines the data category this profile chunk belongs to. + /// + /// This needs to be synchronized with the implementation in Sentry: + /// + pub fn profile_type(&self) -> ProfileType { + match self.profile.platform.as_str() { + "cocoa" | "android" | "javascript" => ProfileType::Ui, + _ => ProfileType::Backend, + } } - match (profile.platform.as_str(), profile.version) { - ("android", _) => android::chunk::parse(payload), - (_, sample::Version::V2) => { - let mut profile = sample::v2::parse(payload)?; - profile.normalize()?; - serde_json::to_vec(&profile).map_err(|_| ProfileError::CannotSerializePayload) + /// Applies inbound filters to the profile chunk. + /// + /// The profile needs to be filtered (rejected) when this returns an error. + pub fn filter( + &self, + client_ip: Option, + filter_settings: &ProjectFiltersConfig, + global_config: &GlobalConfig, + ) -> Result<(), ProfileError> { + relay_filter::should_filter( + &self.profile, + client_ip, + filter_settings, + global_config.filters(), + ) + .map_err(ProfileError::Filtered) + } + + /// Normalizes and 'expands' the profile chunk into its normalized form Sentry expects. + pub fn expand(&self) -> Result, ProfileError> { + match (self.profile.platform.as_str(), self.profile.version) { + ("android", _) => android::chunk::parse(&self.payload), + (_, sample::Version::V2) => { + let mut profile = sample::v2::parse(&self.payload)?; + profile.normalize()?; + Ok(serde_json::to_vec(&profile) + .map_err(|_| ProfileError::CannotSerializePayload)?) + } + (_, _) => Err(ProfileError::PlatformNotSupported), } - (_, _) => Err(ProfileError::PlatformNotSupported), } } diff --git a/relay-profiling/src/sample/mod.rs b/relay-profiling/src/sample/mod.rs index 6534824dc5e..d3d1b0ba335 100644 --- a/relay-profiling/src/sample/mod.rs +++ b/relay-profiling/src/sample/mod.rs @@ -7,7 +7,7 @@ pub mod v1; pub mod v2; /// Possible values for the version field of the Sample Format. -#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq)] +#[derive(Debug, Serialize, Deserialize, Copy, Clone, Default, PartialEq, Eq)] pub enum Version { #[default] Unknown, diff --git a/relay-server/src/envelope.rs b/relay-server/src/envelope.rs index ff02c9e0a9f..ae7d6f1f523 100644 --- a/relay-server/src/envelope.rs +++ b/relay-server/src/envelope.rs @@ -31,6 +31,7 @@ //! ``` use relay_base_schema::project::ProjectKey; +use relay_profiling::ProfileType; use std::borrow::Borrow; use std::collections::BTreeMap; use std::fmt; @@ -596,6 +597,12 @@ pub struct ItemHeaders { #[serde(default, skip)] ingest_span_in_eap: bool, + /// Tracks whether the item is a backend or ui profile chunk. + /// + /// NOTE: This is internal-only and not exposed into the Envelope. + #[serde(default, skip)] + profile_type: Option, + /// Other attributes for forward compatibility. #[serde(flatten)] other: BTreeMap, @@ -673,6 +680,7 @@ impl Item { sampled: true, fully_normalized: false, ingest_span_in_eap: false, + profile_type: None, }, payload: Bytes::new(), } @@ -725,7 +733,11 @@ impl Item { ItemType::Span | ItemType::OtelSpan => smallvec![(DataCategory::Span, 1)], // NOTE: semantically wrong, but too expensive to parse. ItemType::OtelTracesData => smallvec![(DataCategory::Span, 1)], - ItemType::ProfileChunk => smallvec![(DataCategory::ProfileChunk, 1)], // TODO: should be seconds? + ItemType::ProfileChunk => match self.headers.profile_type { + Some(ProfileType::Backend) => smallvec![(DataCategory::ProfileChunk, 1)], + Some(ProfileType::Ui) => smallvec![(DataCategory::ProfileChunkUi, 1)], + None => smallvec![], + }, ItemType::Unknown(_) => smallvec![], } } @@ -890,6 +902,16 @@ impl Item { self.headers.ingest_span_in_eap = ingest_span_in_eap; } + /// Returns the associated profile type of a profile chunk. + pub fn profile_type(&self) -> Option { + self.headers.profile_type + } + + /// Set the profile type of the profile chunk. + pub fn set_profile_type(&mut self, profile_type: ProfileType) { + self.headers.profile_type = Some(profile_type); + } + /// Gets the `sampled` flag. pub fn sampled(&self) -> bool { self.headers.sampled diff --git a/relay-server/src/services/processor.rs b/relay-server/src/services/processor.rs index 5815205d69b..d984783a5d7 100644 --- a/relay-server/src/services/processor.rs +++ b/relay-server/src/services/processor.rs @@ -1922,15 +1922,25 @@ impl EnvelopeProcessorService { &self, managed_envelope: &mut TypedEnvelope, project_info: Arc, + _rate_limits: Arc, ) -> Result, ProcessingError> { profile_chunk::filter(managed_envelope, project_info.clone()); if_processing!(self.inner.config, { profile_chunk::process( managed_envelope, - project_info, + project_info.clone(), &self.inner.global_config.current(), &self.inner.config, ); + + self.enforce_quotas( + managed_envelope, + Annotated::empty(), + &mut ProcessingExtractedMetrics::new(), + project_info, + _rate_limits, + ) + .await?; }); Ok(None) @@ -1943,7 +1953,7 @@ impl EnvelopeProcessorService { config: Arc, project_id: ProjectId, project_info: Arc, - #[allow(unused_variables)] rate_limits: Arc, + _rate_limits: Arc, ) -> Result, ProcessingError> { #[allow(unused_mut)] let mut extracted_metrics = ProcessingExtractedMetrics::new(); @@ -1964,7 +1974,7 @@ impl EnvelopeProcessorService { Annotated::empty(), &mut extracted_metrics, project_info.clone(), - rate_limits, + _rate_limits, ) .await?; }); @@ -2297,7 +2307,9 @@ impl EnvelopeProcessorService { rate_limits, reservoir_counters ), - ProcessingGroup::ProfileChunk => run!(process_profile_chunks, project_info), + ProcessingGroup::ProfileChunk => { + run!(process_profile_chunks, project_info, rate_limits) + } // Currently is not used. ProcessingGroup::Metrics => { // In proxy mode we simply forward the metrics. @@ -3650,7 +3662,7 @@ impl UpstreamRequest for SendMetricsRequest { /// Container for global and project level [`Quota`]. #[cfg(feature = "processing")] -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug)] struct CombinedQuotas<'a> { global_quotas: &'a [Quota], project_quotas: &'a [Quota], diff --git a/relay-server/src/services/processor/profile_chunk.rs b/relay-server/src/services/processor/profile_chunk.rs index 3304359b859..c03d7ef1347 100644 --- a/relay-server/src/services/processor/profile_chunk.rs +++ b/relay-server/src/services/processor/profile_chunk.rs @@ -14,6 +14,7 @@ use { crate::services::processor::ProfileChunkGroup, relay_config::Config, relay_dynamic_config::GlobalConfig, + relay_profiling::ProfileError, }; /// Removes profile chunks from the envelope if the feature is not enabled. @@ -40,44 +41,55 @@ pub fn process( ) { let client_ip = managed_envelope.envelope().meta().client_addr(); let filter_settings = &project_info.config.filter_settings; + let continuous_profiling_enabled = if project_info.has_feature(Feature::ContinuousProfilingBetaIngest) { project_info.has_feature(Feature::ContinuousProfilingBeta) } else { project_info.has_feature(Feature::ContinuousProfiling) }; + managed_envelope.retain_items(|item| match item.ty() { ItemType::ProfileChunk => { if !continuous_profiling_enabled { return ItemAction::DropSilently; } - match relay_profiling::expand_profile_chunk( - &item.payload(), - client_ip, - filter_settings, - global_config, - ) { - Ok(payload) => { - if payload.len() <= config.max_profile_size() { - item.set_payload(ContentType::Json, payload); - ItemAction::Keep - } else { - ItemAction::Drop(Outcome::Invalid(DiscardReason::Profiling( - relay_profiling::discard_reason( - relay_profiling::ProfileError::ExceedSizeLimit, - ), - ))) - } - } - Err(relay_profiling::ProfileError::Filtered(filter_stat_key)) => { - ItemAction::Drop(Outcome::Filtered(filter_stat_key)) - } - Err(err) => ItemAction::Drop(Outcome::Invalid(DiscardReason::Profiling( - relay_profiling::discard_reason(err), - ))), + let chunk = match relay_profiling::ProfileChunk::new(item.payload()) { + Ok(chunk) => chunk, + Err(err) => return error_to_action(err), + }; + // Important: set the profile type to get outcomes in the correct category. + item.set_profile_type(chunk.profile_type()); + + if let Err(err) = chunk.filter(client_ip, filter_settings, global_config) { + return error_to_action(err); + } + + let payload = match chunk.expand() { + Ok(expanded) => expanded, + Err(err) => return error_to_action(err), + }; + + if payload.len() > config.max_profile_size() { + return error_to_action(relay_profiling::ProfileError::ExceedSizeLimit); } + + item.set_payload(ContentType::Json, payload); + ItemAction::Keep } _ => ItemAction::Keep, }); } + +#[cfg(feature = "processing")] +fn error_to_action(err: ProfileError) -> ItemAction { + match err { + ProfileError::Filtered(filter_stat_key) => { + ItemAction::Drop(Outcome::Filtered(filter_stat_key)) + } + err => ItemAction::Drop(Outcome::Invalid(DiscardReason::Profiling( + relay_profiling::discard_reason(err), + ))), + } +} diff --git a/relay-server/src/utils/managed_envelope.rs b/relay-server/src/utils/managed_envelope.rs index 0ca927b3d87..2facce2e4de 100644 --- a/relay-server/src/utils/managed_envelope.rs +++ b/relay-server/src/utils/managed_envelope.rs @@ -470,6 +470,14 @@ impl ManagedEnvelope { ); } + if self.context.summary.profile_chunk_ui_quantity > 0 { + self.track_outcome( + outcome.clone(), + DataCategory::ProfileChunkUi, + self.context.summary.profile_chunk_ui_quantity, + ); + } + self.finish(RelayCounters::EnvelopeRejected, handling); } diff --git a/relay-server/src/utils/rate_limits.rs b/relay-server/src/utils/rate_limits.rs index 720ffc0e78b..423e1462f49 100644 --- a/relay-server/src/utils/rate_limits.rs +++ b/relay-server/src/utils/rate_limits.rs @@ -2,6 +2,7 @@ use std::fmt::{self, Write}; use std::future::Future; use std::marker::PhantomData; +use relay_profiling::ProfileType; use relay_quotas::{ DataCategories, DataCategory, ItemScoping, QuotaScope, RateLimit, RateLimitScope, RateLimits, ReasonCode, Scoping, @@ -198,6 +199,8 @@ pub struct EnvelopeSummary { /// The number of profile chunks in this envelope. pub profile_chunk_quantity: usize, + /// The number of profile chunks in this envelope. + pub profile_chunk_ui_quantity: usize, } impl EnvelopeSummary { @@ -253,6 +256,7 @@ impl EnvelopeSummary { DataCategory::LogItem => &mut self.log_item_quantity, DataCategory::LogByte => &mut self.log_byte_quantity, DataCategory::ProfileChunk => &mut self.profile_chunk_quantity, + DataCategory::ProfileChunkUi => &mut self.profile_chunk_ui_quantity, // TODO: This catch-all return looks dangerous _ => return, }; @@ -369,6 +373,8 @@ pub struct Enforcement { pub user_reports_v2: CategoryLimit, /// The combined profile chunk item rate limit. pub profile_chunks: CategoryLimit, + /// The combined profile chunk ui item rate limit. + pub profile_chunks_ui: CategoryLimit, } impl Enforcement { @@ -408,6 +414,7 @@ impl Enforcement { spans_indexed, user_reports_v2, profile_chunks, + profile_chunks_ui, } = self; let limits = [ @@ -425,6 +432,7 @@ impl Enforcement { spans_indexed, user_reports_v2, profile_chunks, + profile_chunks_ui, ]; limits @@ -515,7 +523,11 @@ impl Enforcement { ItemType::Span | ItemType::OtelSpan | ItemType::OtelTracesData => { !self.spans_indexed.is_active() } - ItemType::ProfileChunk => !self.profile_chunks.is_active(), + ItemType::ProfileChunk => match item.profile_type() { + Some(ProfileType::Backend) => !self.profile_chunks.is_active(), + Some(ProfileType::Ui) => !self.profile_chunks_ui.is_active(), + None => true, + }, ItemType::Event | ItemType::Transaction | ItemType::Security @@ -892,16 +904,30 @@ where // Handle profile chunks. if summary.profile_chunk_quantity > 0 { let item_scoping = scoping.item(DataCategory::ProfileChunk); - let profile_chunk_limits = self + let limits = self .check .apply(item_scoping, summary.profile_chunk_quantity) .await?; enforcement.profile_chunks = CategoryLimit::new( DataCategory::ProfileChunk, summary.profile_chunk_quantity, - profile_chunk_limits.longest(), + limits.longest(), ); - rate_limits.merge(profile_chunk_limits); + rate_limits.merge(limits); + } + + if summary.profile_chunk_ui_quantity > 0 { + let item_scoping = scoping.item(DataCategory::ProfileChunkUi); + let limits = self + .check + .apply(item_scoping, summary.profile_chunk_ui_quantity) + .await?; + enforcement.profile_chunks_ui = CategoryLimit::new( + DataCategory::ProfileChunkUi, + summary.profile_chunk_ui_quantity, + limits.longest(), + ); + rate_limits.merge(limits); } Ok((enforcement, rate_limits)) @@ -1354,19 +1380,75 @@ mod tests { /// Limit profile chunks. #[tokio::test] - async fn test_enforce_limit_profile_chunks() { + async fn test_enforce_limit_profile_chunks_no_profile_type() { + // In this test we have profile chunks which have not yet been classified, which means they + // should not be rate limited. let mut envelope = envelope![ProfileChunk, ProfileChunk]; let mock = mock_limiter(Some(DataCategory::ProfileChunk)); let (enforcement, limits) = enforce_and_apply(mock.clone(), &mut envelope, None).await; + assert!(!limits.is_limited()); + assert_eq!(get_outcomes(enforcement), vec![]); + + let mock = mock_limiter(Some(DataCategory::ProfileChunkUi)); + let (enforcement, limits) = enforce_and_apply(mock.clone(), &mut envelope, None).await; + assert!(!limits.is_limited()); + assert_eq!(get_outcomes(enforcement), vec![]); + + assert_eq!(envelope.envelope().len(), 2); + } + + #[tokio::test] + async fn test_enforce_limit_profile_chunks_ui() { + let mut envelope = envelope![]; + + let mut item = Item::new(ItemType::ProfileChunk); + item.set_profile_type(ProfileType::Backend); + envelope.envelope_mut().add_item(item); + let mut item = Item::new(ItemType::ProfileChunk); + item.set_profile_type(ProfileType::Ui); + envelope.envelope_mut().add_item(item); + + let mock = mock_limiter(Some(DataCategory::ProfileChunkUi)); + let (enforcement, limits) = enforce_and_apply(mock.clone(), &mut envelope, None).await; assert!(limits.is_limited()); - assert_eq!(envelope.envelope().len(), 0); - mock.lock().await.assert_call(DataCategory::ProfileChunk, 2); + assert_eq!(envelope.envelope().len(), 1); + mock.lock() + .await + .assert_call(DataCategory::ProfileChunkUi, 1); + mock.lock().await.assert_call(DataCategory::ProfileChunk, 1); + + assert_eq!( + get_outcomes(enforcement), + vec![(DataCategory::ProfileChunkUi, 1)] + ); + } + + #[tokio::test] + async fn test_enforce_limit_profile_chunks_backend() { + let mut envelope = envelope![]; + + let mut item = Item::new(ItemType::ProfileChunk); + item.set_profile_type(ProfileType::Backend); + envelope.envelope_mut().add_item(item); + let mut item = Item::new(ItemType::ProfileChunk); + item.set_profile_type(ProfileType::Ui); + envelope.envelope_mut().add_item(item); + + let mock = mock_limiter(Some(DataCategory::ProfileChunk)); + let (enforcement, limits) = enforce_and_apply(mock.clone(), &mut envelope, None).await; + + assert!(limits.is_limited()); + assert_eq!(envelope.envelope().len(), 1); + mock.lock() + .await + .assert_call(DataCategory::ProfileChunkUi, 1); + mock.lock().await.assert_call(DataCategory::ProfileChunk, 1); assert_eq!( get_outcomes(enforcement), - vec![(DataCategory::ProfileChunk, 2),] + vec![(DataCategory::ProfileChunk, 1)] ); } diff --git a/tests/integration/test_filters.py b/tests/integration/test_filters.py index 8ac97e4675c..fb9127a522b 100644 --- a/tests/integration/test_filters.py +++ b/tests/integration/test_filters.py @@ -555,14 +555,14 @@ def android_profile_chunk_envelope(release): [ pytest.param(sample_profile_v1_envelope, DataCategory.PROFILE, id="profile v1"), pytest.param( - sample_profile_v2_envelope, DataCategory.PROFILE_CHUNK, id="profile v2" + sample_profile_v2_envelope, DataCategory.PROFILE_CHUNK_UI, id="profile v2" ), pytest.param( android_profile_legacy_envelope, DataCategory.PROFILE, id="android legacy" ), pytest.param( android_profile_chunk_envelope, - DataCategory.PROFILE_CHUNK, + DataCategory.PROFILE_CHUNK_UI, id="android chunk", ), ], @@ -613,6 +613,9 @@ def test_filters_are_applied_to_profiles( outcome.pop("timestamp") outcomes.append(outcome) + print("=======") + print(outcomes) + assert outcomes == [ { "category": data_category, diff --git a/tests/integration/test_profile_chunks.py b/tests/integration/test_profile_chunks.py index 7be2a645e18..8102643d9c5 100644 --- a/tests/integration/test_profile_chunks.py +++ b/tests/integration/test_profile_chunks.py @@ -4,6 +4,8 @@ import pytest from sentry_sdk.envelope import Envelope, Item, PayloadRef +from sentry_relay.consts import DataCategory +from .asserts import time_within_delta RELAY_ROOT = Path(__file__).parent.parent.parent @@ -121,29 +123,31 @@ def test_profile_chunk_outcomes_invalid( upstream = relay_with_processing(config) envelope = Envelope() - envelope.add_item(Item(payload=PayloadRef(bytes=b""), type="profile_chunk")) + payload = { + "chunk_id": "11111111111111111111111111111111", + "platform": "thisisnotvalid", + } + envelope.add_item(Item(payload=PayloadRef(json=payload), type="profile_chunk")) upstream.send_envelope(project_id, envelope) outcomes = outcomes_consumer.get_outcomes() outcomes.sort(key=lambda o: sorted(o.items())) - expected_outcomes = [ + assert outcomes == [ { - "category": 18, # DataCategory::ProfileChunk + "category": DataCategory.PROFILE_CHUNK.value, + "timestamp": time_within_delta(), "key_id": 123, "org_id": 1, "outcome": 3, # Invalid "project_id": 42, "quantity": 1, - "reason": "profiling_invalid_json", + "reason": "profiling_platform_not_supported", "source": "pop-relay", }, ] - for outcome in outcomes: - outcome.pop("timestamp") - assert outcomes == expected_outcomes, outcomes profiles_consumer.assert_empty() @@ -176,7 +180,7 @@ def test_profile_chunk_outcomes_rate_limited( project_config["quotas"] = [ { "id": f"test_rate_limiting_{uuid.uuid4().hex}", - "categories": ["profile_chunk"], # Target profile chunks specifically + "categories": ["profile_chunk_ui"], # Target profile chunks specifically "limit": 0, # Block all profile chunks "reasonCode": "profile_chunks_exceeded", } @@ -216,9 +220,10 @@ def test_profile_chunk_outcomes_rate_limited( outcomes = outcomes_consumer.get_outcomes() outcomes.sort(key=lambda o: sorted(o.items())) - expected_outcomes = [ + assert outcomes == [ { - "category": 18, # DataCategory::ProfileChunk + "category": DataCategory.PROFILE_CHUNK_UI.value, + "timestamp": time_within_delta(), "key_id": 123, "org_id": 1, "outcome": 2, # RateLimited @@ -227,102 +232,6 @@ def test_profile_chunk_outcomes_rate_limited( "reason": "profile_chunks_exceeded", }, ] - for outcome in outcomes: - outcome.pop("timestamp") - outcome.pop("event_id", None) - - assert outcomes == expected_outcomes, outcomes - - # Verify no profiles were forwarded to the consumer - profiles_consumer.assert_empty() - - -def test_profile_chunk_outcomes_rate_limited_via_profile_duration_rate_limit( - mini_sentry, - relay_with_processing, - outcomes_consumer, - profiles_consumer, -): - """ - Tests that Relay reports correct outcomes when profile chunks are rate limited via profile duration quotas. - - This test verifies that when a profile chunk hits a profile duration rate limit: - 1. The profile chunk is dropped and not forwarded to the profiles consumer - 2. A rate limited outcome is emitted with the correct category and reason code - 3. The rate limit is enforced at the organization level - 4. Both profile_chunk and profile_duration categories are affected - """ - outcomes_consumer = outcomes_consumer(timeout=2) - profiles_consumer = profiles_consumer() - - project_id = 42 - project_config = mini_sentry.add_full_project_config(project_id)["config"] - - # Enable profiling feature flag - project_config.setdefault("features", []).append( - "organizations:continuous-profiling" - ) - - # Configure rate limiting quota that blocks all profile durations and profile chunks at org level - project_config["quotas"] = [ - { - "categories": ["profile_duration", "profile_chunk"], - "limit": 0, # Block all profile durations and profile chunks - "reasonCode": "profile_duration_usage_exceeded", - "scope": "organization", # Apply at org level - }, - ] - - config = { - "outcomes": { - "emit_outcomes": True, - "batch_size": 1, - "batch_interval": 1, - "aggregator": { - "bucket_interval": 1, - "flush_interval": 0, - }, - }, - "aggregator": { - "bucket_interval": 1, - "initial_delay": 0, - }, - } - - upstream = relay_with_processing(config) - - # Load a valid profile chunk from test fixtures - with open( - RELAY_ROOT / "relay-profiling/tests/fixtures/sample/v2/valid.json", - "rb", - ) as f: - profile = f.read() - - # Create and send envelope containing the profile chunk - envelope = Envelope() - envelope.add_item(Item(payload=PayloadRef(bytes=profile), type="profile_chunk")) - upstream.send_envelope(project_id, envelope) - - # Verify the rate limited outcome was emitted with correct properties - outcomes = outcomes_consumer.get_outcomes() - outcomes.sort(key=lambda o: sorted(o.items())) - - expected_outcomes = [ - { - "category": 18, # DataCategory::ProfileChunk - "key_id": 123, - "org_id": 1, - "outcome": 2, # RateLimited - "project_id": 42, - "quantity": 1, - "reason": "profile_duration_usage_exceeded", - }, - ] - for outcome in outcomes: - outcome.pop("timestamp") - outcome.pop("event_id", None) - - assert outcomes == expected_outcomes, outcomes # Verify no profiles were forwarded to the consumer profiles_consumer.assert_empty() From db8378333966dacf4d29c5cc95298faa7714215a Mon Sep 17 00:00:00 2001 From: David Herberth Date: Thu, 20 Mar 2025 19:35:47 +0100 Subject: [PATCH 2/3] Update relay-server/src/utils/rate_limits.rs Co-authored-by: Sebastian Zivota --- 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 423e1462f49..b7270c14f2d 100644 --- a/relay-server/src/utils/rate_limits.rs +++ b/relay-server/src/utils/rate_limits.rs @@ -199,7 +199,7 @@ pub struct EnvelopeSummary { /// The number of profile chunks in this envelope. pub profile_chunk_quantity: usize, - /// The number of profile chunks in this envelope. + /// The number of UI profile chunks in this envelope. pub profile_chunk_ui_quantity: usize, } From 20c270cf28adb2c41bf54c2e55404da5f4546c91 Mon Sep 17 00:00:00 2001 From: David Herberth Date: Thu, 20 Mar 2025 21:46:29 +0100 Subject: [PATCH 3/3] cleanup --- tests/integration/test_filters.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/integration/test_filters.py b/tests/integration/test_filters.py index fb9127a522b..b36431c00d0 100644 --- a/tests/integration/test_filters.py +++ b/tests/integration/test_filters.py @@ -613,9 +613,6 @@ def test_filters_are_applied_to_profiles( outcome.pop("timestamp") outcomes.append(outcome) - print("=======") - print(outcomes) - assert outcomes == [ { "category": data_category,