ref(profiling): forward profiles of non-sampled transactions (with no options filtering)

CHANGELOG.md

@@ -18,6 +18,7 @@
 - Record too long discard reason for session replays. ([#3950](https://github.com/getsentry/relay/pull/3950))
 - Add `EnvelopeStore` trait and implement `DiskUsage` for tracking disk usage. ([#3925](https://github.com/getsentry/relay/pull/3925))
 - Increase replay recording limit to two hours. ([#3961](https://github.com/getsentry/relay/pull/3961))
+- Forward profiles of non-sampled transactions (with no options filtering). ([#3963](https://github.com/getsentry/relay/pull/3963))
 - Make EnvelopeBuffer a Service. ([#3965](https://github.com/getsentry/relay/pull/3965))
 
 **Internal**:

relay-dynamic-config/src/feature.rs

@@ -61,13 +61,6 @@ pub enum Feature {
     /// Serialized as `projects:relay-otel-endpoint`.
     #[serde(rename = "projects:relay-otel-endpoint")]
     OtelEndpoint,
-    /// Enable processing and extracting data from profiles that would normally be dropped by dynamic sampling.
-    ///
-    /// This is required for [slowest function aggregation](https://github.com/getsentry/snuba/blob/b5311b404a6bd73a9e1997a46d38e7df88e5f391/snuba/snuba_migrations/functions/0001_functions.py#L209-L256). The profile payload will be dropped on the sentry side.
-    ///
-    /// Serialized as `projects:profiling-ingest-unsampled-profiles`.
-    #[serde(rename = "projects:profiling-ingest-unsampled-profiles")]
-    IngestUnsampledProfiles,
 
     /// Discard transactions in a spans-only world.
     ///
@@ -119,6 +112,10 @@ pub enum Feature {
     #[doc(hidden)]
     #[serde(rename = "projects:span-metrics-extraction-all-modules")]
     Deprecated6,
+    /// Deprecated, still forwarded for older downstream Relays.
+    #[doc(hidden)]
+    #[serde(rename = "projects:profiling-ingest-unsampled-profiles")]
+    Deprecated7,
     /// Forward compatibility.
     #[doc(hidden)]
     #[serde(other)]

relay-dynamic-config/src/global.rs

@@ -107,31 +107,13 @@ fn is_err_or_empty(filters_config: &ErrorBoundary<GenericFiltersConfig>) -> bool
 #[derive(Default, Clone, Debug, Serialize, Deserialize, PartialEq)]
 #[serde(default)]
 pub struct Options {
-    /// List of platform names for which we allow using unsampled profiles for the purpose
-    /// of improving profile (function) metrics
-    #[serde(
-        rename = "profiling.profile_metrics.unsampled_profiles.platforms",
-        deserialize_with = "default_on_error",
-        skip_serializing_if = "Vec::is_empty"
-    )]
-    pub profile_metrics_allowed_platforms: Vec<String>,
-
-    /// Sample rate for tuning the amount of unsampled profiles that we "let through"
-    #[serde(
-        rename = "profiling.profile_metrics.unsampled_profiles.sample_rate",
-        deserialize_with = "default_on_error",
-        skip_serializing_if = "is_default"
-    )]
-    pub profile_metrics_sample_rate: f32,
-
     /// Kill switch for shutting down unsampled_profile metrics
     #[serde(
         rename = "profiling.profile_metrics.unsampled_profiles.enabled",
         deserialize_with = "default_on_error",
         skip_serializing_if = "is_default"
     )]
     pub unsampled_profiles_enabled: bool,
-
     /// Kill switch for shutting down profile function metrics
     /// ingestion in the generic-metrics platform
     #[serde(
@@ -225,6 +207,24 @@ pub struct Options {
     )]
     pub http_span_allowed_hosts: Vec<Host>,
 
+    /// Deprecated, still forwarded for older downstream Relays.
+    #[doc(hidden)]
+    #[serde(
+        rename = "profiling.profile_metrics.unsampled_profiles.platforms",
+        deserialize_with = "default_on_error",
+        skip_serializing_if = "Vec::is_empty"
+    )]
+    pub deprecated1: Vec<String>,
+
+    /// Deprecated, still forwarded for older downstream Relays.
+    #[doc(hidden)]
+    #[serde(
+        rename = "profiling.profile_metrics.unsampled_profiles.sample_rate",
+        deserialize_with = "default_on_error",
+        skip_serializing_if = "is_default"
+    )]
+    pub deprecated2: f32,
+
     /// All other unknown options.
     #[serde(flatten)]
     other: HashMap<String, Value>,
@@ -443,9 +443,6 @@ mod tests {
         }
       }
     ]
-  },
-  "options": {
-    "profiling.profile_metrics.unsampled_profiles.enabled": true
   }
 }"#;
 
@@ -458,8 +455,7 @@ mod tests {
     fn test_global_config_invalid_value_is_default() {
         let options: Options = serde_json::from_str(
             r#"{
-                "relay.cardinality-limiter.mode": "passive",
-                "profiling.profile_metrics.unsampled_profiles.sample_rate": "foo"
+                "relay.cardinality-limiter.mode": "passive"
             }"#,
         )
         .unwrap();

relay-server/src/services/processor.rs

@@ -1784,14 +1784,13 @@ impl EnvelopeProcessorService {
         };
 
         if let Some(outcome) = sampling_result.into_dropped_outcome() {
-            let keep_profiles = dynamic_sampling::forward_unsampled_profiles(state, &global_config);
+            let keep_profiles = global_config.options.unsampled_profiles_enabled;
             // Process profiles before dropping the transaction, if necessary.
             // Before metric extraction to make sure the profile count is reflected correctly.
             let profile_id = match keep_profiles {
                 true => profile::process(state),
                 false => profile_id,
             };
-
             // Extract metrics here, we're about to drop the event/transaction.
             self.extract_transaction_metrics(state, SamplingDecision::Drop, profile_id)?;
 

relay-server/src/services/processor/dynamic_sampling.rs

@@ -4,7 +4,7 @@ use std::ops::ControlFlow;
 
 use chrono::Utc;
 use relay_config::Config;
-use relay_dynamic_config::{ErrorBoundary, Feature, GlobalConfig};
+use relay_dynamic_config::ErrorBoundary;
 use relay_event_schema::protocol::{Contexts, Event, TraceContext};
 use relay_protocol::{Annotated, Empty};
 use relay_sampling::config::RuleType;
@@ -16,7 +16,7 @@ use crate::services::outcome::Outcome;
 use crate::services::processor::{
     EventProcessing, ProcessEnvelopeState, Sampling, TransactionGroup,
 };
-use crate::utils::{self, sample, SamplingResult};
+use crate::utils::{self, SamplingResult};
 
 /// Ensures there is a valid dynamic sampling context and corresponding project state.
 ///
@@ -229,33 +229,6 @@ pub fn tag_error_with_sampling_decision<G: EventProcessing>(
     }
 }
 
-/// Determines whether profiles that would otherwise be dropped by dynamic sampling should be kept.
-pub fn forward_unsampled_profiles(
-    state: &ProcessEnvelopeState<TransactionGroup>,
-    global_config: &GlobalConfig,
-) -> bool {
-    let global_options = &global_config.options;
-
-    if !global_options.unsampled_profiles_enabled {
-        return false;
-    }
-
-    let event_platform = state
-        .event
-        .value()
-        .and_then(|e| e.platform.as_str())
-        .unwrap_or("");
-
-    state
-        .project_state
-        .has_feature(Feature::IngestUnsampledProfiles)
-        && global_options
-            .profile_metrics_allowed_platforms
-            .iter()
-            .any(|s| s == event_platform)
-        && sample(global_options.profile_metrics_sample_rate)
-}
-
 #[cfg(test)]
 mod tests {
     use std::collections::BTreeMap;
@@ -264,6 +237,7 @@ mod tests {
     use bytes::Bytes;
     use relay_base_schema::events::EventType;
     use relay_base_schema::project::{ProjectId, ProjectKey};
+    use relay_dynamic_config::GlobalConfig;
     use relay_dynamic_config::{MetricExtractionConfig, TransactionMetricsConfig};
     use relay_event_schema::protocol::{EventId, LenientString};
     use relay_protocol::RuleCondition;

relay-server/src/services/processor/profile.rs

@@ -19,7 +19,7 @@ use crate::utils::ItemAction;
 pub fn filter<G>(state: &mut ProcessEnvelopeState<G>) -> Option<ProfileId> {
     let profiling_disabled = state.should_filter(Feature::Profiling);
     let has_transaction = state.event_type() == Some(EventType::Transaction);
-    let keep_unsampled_profiles = !state.should_filter(Feature::IngestUnsampledProfiles);
+    let keep_unsampled_profiles = true;
 
     let mut profile_id = None;
     state.managed_envelope.retain_items(|item| match item.ty() {

tests/integration/test_dynamic_sampling.py

@@ -666,8 +666,6 @@ def test_relay_chain_keep_unsampled_profile(
     mini_sentry, relay, relay_with_processing, profiles_consumer, mode
 ):
     mini_sentry.global_config["options"] = {
-        "profiling.profile_metrics.unsampled_profiles.platforms": ["python"],
-        "profiling.profile_metrics.unsampled_profiles.sample_rate": 1.0,
         "profiling.profile_metrics.unsampled_profiles.enabled": True,
     }
 
@@ -706,7 +704,6 @@ def make_envelope(public_key):
     }
     config["config"]["features"] = [
         "organizations:profiling",
-        "projects:profiling-ingest-unsampled-profiles",
     ]
 
     public_key = config["publicKeys"][0]["publicKey"]