ref(server): Remove internal dynamic sampling metrics

Author: jan-auer

relay-server/src/actors/processor.rs

@@ -32,7 +32,7 @@ use relay_log::LogError;
 use relay_metrics::{Bucket, InsertMetrics, MergeBuckets, Metric};
 use relay_quotas::{DataCategory, ReasonCode};
 use relay_redis::RedisPool;
-use relay_sampling::{DynamicSamplingContext, RuleId};
+use relay_sampling::RuleId;
 use relay_statsd::metric;
 use relay_system::{Addr, FromMessage, NoResponse, Service};
 
@@ -45,7 +45,7 @@ use crate::envelope::{AttachmentType, ContentType, Envelope, Item, ItemType};
 use crate::metrics_extraction::sessions::{extract_session_metrics, SessionMetricsConfig};
 use crate::metrics_extraction::transactions::{extract_transaction_metrics, ExtractMetricsError};
 use crate::service::REGISTRY;
-use crate::statsd::{RelayCounters, RelayHistograms, RelayTimers};
+use crate::statsd::{RelayCounters, RelayTimers};
 use crate::utils::{
     self, ChunkedFormDataAggregator, EnvelopeContext, ErrorBoundary, FormDataIter, SamplingResult,
 };
@@ -358,109 +358,6 @@ fn outcome_from_profile_error(err: relay_profiling::ProfileError) -> Outcome {
     Outcome::Invalid(discard_reason)
 }
 
-fn track_sampling_metrics(
-    project_state: &ProjectState,
-    context: &DynamicSamplingContext,
-    event: &Event,
-) {
-    // We only collect this metric for the root transaction event, so ignore secondary projects.
-    if !project_state.is_matching_key(context.public_key) {
-        return;
-    }
-
-    let transaction_info = match event.transaction_info.value() {
-        Some(info) => info,
-        None => return,
-    };
-
-    let changes = match transaction_info.changes.value() {
-        Some(value) => value.as_slice(),
-        None => return,
-    };
-
-    let last_change = changes
-        .iter()
-        .rev()
-        // skip all broken change records
-        .filter_map(|a| a.value())
-        // skip records without a timestamp
-        .filter(|c| c.timestamp.value().is_some())
-        // take the last that did not occur when the event was sent
-        .find(|c| c.timestamp.value() != event.timestamp.value());
-
-    let source = event.get_transaction_source().as_str();
-    let platform = event.platform.as_str().unwrap_or("other");
-    let sdk_name = event.sdk_name();
-    let sdk_version = event.sdk_version();
-
-    metric!(
-        histogram(RelayHistograms::DynamicSamplingChanges) = changes.len() as u64,
-        source = source,
-        platform = platform,
-        sdk_name = sdk_name,
-        sdk_version = sdk_version,
-    );
-
-    if let Some(&total) = transaction_info.propagations.value() {
-        // If there was no change, there were no propagations that happened with a wrong name.
-        let change = last_change
-            .and_then(|c| c.propagations.value())
-            .map_or(0, |v| *v);
-
-        metric!(
-            histogram(RelayHistograms::DynamicSamplingPropagationCount) = change,
-            source = source,
-            platform = platform,
-            sdk_name = sdk_name,
-            sdk_version = sdk_version,
-        );
-
-        let percentage = match (change, total) {
-            (0, 0) => 0.0, // 0% indicates no premature changes.
-            _ => ((change as f64) / (total as f64)).min(1.0) * 100.0,
-        };
-
-        metric!(
-            histogram(RelayHistograms::DynamicSamplingPropagationPercentage) = percentage,
-            source = source,
-            platform = platform,
-            sdk_name = sdk_name,
-            sdk_version = sdk_version,
-        );
-    }
-
-    if let (Some(&start), Some(&change), Some(&end)) = (
-        event.start_timestamp.value(),
-        last_change
-            .and_then(|c| c.timestamp.value())
-            .or_else(|| event.start_timestamp.value()), // default to start if there was no change
-        event.timestamp.value(),
-    ) {
-        let delay_ms = (change - start).num_milliseconds();
-        if delay_ms >= 0 {
-            metric!(
-                histogram(RelayHistograms::DynamicSamplingChangeDuration) = delay_ms as u64,
-                source = source,
-                platform = platform,
-                sdk_name = sdk_name,
-                sdk_version = sdk_version,
-            );
-        }
-
-        let duration_ms = (end - start).num_milliseconds() as f64;
-        if delay_ms >= 0 && duration_ms >= 0.0 {
-            let percentage = ((delay_ms as f64) / duration_ms).min(1.0) * 100.0;
-            metric!(
-                histogram(RelayHistograms::DynamicSamplingChangePercentage) = percentage,
-                source = source,
-                platform = platform,
-                sdk_name = sdk_name,
-                sdk_version = sdk_version,
-            );
-        }
-    }
-}
-
 /// Response of the [`ProcessEnvelope`] message.
 #[cfg_attr(not(feature = "processing"), allow(dead_code))]
 pub struct ProcessEnvelopeResponse {
@@ -1859,11 +1756,8 @@ impl EnvelopeProcessorService {
 
             state.transaction_metrics_extracted = true;
             state.envelope_context.set_event_metrics_extracted();
-
-            if let Some(context) = state.envelope.sampling_context() {
-                track_sampling_metrics(&state.project_state, context, event);
-            }
         }
+
         Ok(())
     }
 

relay-server/src/statsd.rs

@@ -134,77 +134,6 @@ pub enum RelayHistograms {
     /// Size of queries (projectconfig queries, i.e. the request payload, not the response) sent by
     /// Relay over HTTP in bytes.
     UpstreamEnvelopeBodySize,
-
-    /// Counts how often a transaction name was changed before submitting the final transaction.
-    ///
-    /// A value of `0` indicates that the transaction was created with the final transaction name.
-    /// In this case, the DSC will have the correct transaction name guaranteed. However, to
-    /// determine how many traces had wrong transaction names propagated, check the
-    /// `dynamic_sampling.propagations`.
-    ///
-    /// This metric is tagged with:
-    ///
-    ///  - `source`: The transaction source value in the final event payload.
-    ///  - `platform`: The SDK platform value of the event payload.
-    ///  - `sdk_name`: The name of the client SDK as reported in the event payload.
-    ///  - `sdk_version`: The version of the client SDK as reported in the event payload.
-    DynamicSamplingChanges,
-
-    /// Counts the number of propagations before the final transaction name has been determined.
-    ///
-    /// A value of `0` indicates that the entire trace had identical transaction names in the
-    /// Dynamic Sampling Context (DSC). This means that the entire trace is sampled consistently.
-    ///
-    /// Note that this differs from `dynamic_sampling.changes`, which indicates changes even in the
-    /// absence of propagations.
-    ///
-    /// This metric is tagged with:
-    ///
-    ///  - `source`: The transaction source value in the final event payload.
-    ///  - `platform`: The SDK platform value of the event payload.
-    ///  - `sdk_name`: The name of the client SDK as reported in the event payload.
-    ///  - `sdk_version`: The version of the client SDK as reported in the event payload.
-    DynamicSamplingPropagationCount,
-
-    /// The number of propagations before the final transaction name change relative to the total
-    /// number of propagations.
-    ///
-    /// Tracks the same as `dynamic_sampling.propagations`, except that the value of this metric is
-    /// a percentage between `0.0` and `100.0`. A value of `0` means that no propagations occurred,
-    /// and `100` means that all propagations occurred with the wrong transaction name.
-    ///
-    /// This metric is tagged with:
-    ///
-    ///  - `source`: The transaction source value in the final event payload.
-    ///  - `platform`: The SDK platform value of the event payload.
-    ///  - `sdk_name`: The name of the client SDK as reported in the event payload.
-    ///  - `sdk_version`: The version of the client SDK as reported in the event payload.
-    DynamicSamplingPropagationPercentage,
-
-    /// Time in milliseconds from the start of transaction until the final name is determined.
-    ///
-    /// If the transaction name changes multiple times, this records only the last instance. This
-    /// metric is not logged if there were no changes to the transaction name.
-    ///
-    /// This metric is tagged with:
-    ///
-    ///  - `source`: The transaction source value in the final event payload.
-    ///  - `platform`: The SDK platform value of the event payload.
-    ///  - `sdk_name`: The name of the client SDK as reported in the event payload.
-    ///  - `sdk_version`: The version of the client SDK as reported in the event payload.
-    DynamicSamplingChangeDuration,
-
-    /// Timing relative to the transaction duration until the final name is determined.
-    ///
-    /// This is a percentage between `0.0` and `100.0`.
-    ///
-    /// This metric is tagged with:
-    ///
-    ///  - `source`: The transaction source value in the final event payload.
-    ///  - `platform`: The SDK platform value of the event payload.
-    ///  - `sdk_name`: The name of the client SDK as reported in the event payload.
-    ///  - `sdk_version`: The version of the client SDK as reported in the event payload.
-    DynamicSamplingChangePercentage,
 }
 
 impl HistogramMetric for RelayHistograms {
@@ -231,13 +160,6 @@ impl HistogramMetric for RelayHistograms {
             RelayHistograms::UpstreamRetries => "upstream.retries",
             RelayHistograms::UpstreamQueryBodySize => "upstream.query.body_size",
             RelayHistograms::UpstreamEnvelopeBodySize => "upstream.envelope.body_size",
-            RelayHistograms::DynamicSamplingChanges => "dynamic_sampling.changes",
-            RelayHistograms::DynamicSamplingPropagationCount => "dynamic_sampling.propagations",
-            RelayHistograms::DynamicSamplingPropagationPercentage => {
-                "dynamic_sampling.propagation_pct"
-            }
-            RelayHistograms::DynamicSamplingChangeDuration => "dynamic_sampling.change_duration",
-            RelayHistograms::DynamicSamplingChangePercentage => "dynamic_sampling.change_pct",
         }
     }
 }