Skip to content

ref(server): Remove internal dynamic sampling metrics #1671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 5, 2022
Merged

ref(server): Remove internal dynamic sampling metrics #1671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15d2f01
ref(server): Remove internal dynamic sampling metrics
jan-auer Dec 5, 2022
4fa434b
Merge branch 'master' into ref/remove-internal-ds-metrics
jan-auer Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 3 additions & 109 deletions relay-server/src/actors/processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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,
};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(())
}

Expand Down
78 changes: 0 additions & 78 deletions relay-server/src/statsd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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",
}
}
}
Expand Down