From 1d02602453bdc77cdd80e521a0f70d8458366f25 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Thu, 6 Jun 2024 09:09:39 +0200 Subject: [PATCH] instr(server): Remove cogs related statsd metrics (#3689) These were introduced for an ad-hoc COGS estimate. We're attempting to reduce the amount of metrics (see https://github.com/getsentry/team-ingest/issues/353). --- relay-server/src/services/processor.rs | 60 ++++++++----------- .../services/processor/dynamic_sampling.rs | 6 -- relay-server/src/statsd.rs | 13 ---- 3 files changed, 26 insertions(+), 53 deletions(-) diff --git a/relay-server/src/services/processor.rs b/relay-server/src/services/processor.rs index 52aed1a5a1..2b147e6b6a 100644 --- a/relay-server/src/services/processor.rs +++ b/relay-server/src/services/processor.rs @@ -1525,12 +1525,10 @@ impl EnvelopeProcessorService { dynamic_sampling::ensure_dsc(state); let filter_run = event::filter(state, &self.inner.global_config.current())?; - let mut sampling_should_drop = false; - if self.inner.config.processing_enabled() || matches!(filter_run, FiltersStatus::Ok) { let sampling_result = dynamic_sampling::run(state, &self.inner.config); // Remember sampling decision, before it is reset in `dynamic_sampling::sample_envelope_items`. - sampling_should_drop = sampling_result.should_drop(); + let sampling_should_drop = sampling_result.should_drop(); // We avoid extracting metrics if we are not sampling the event while in non-processing // relays, in order to synchronize rate limits on indexed and processed transactions. @@ -1546,41 +1544,35 @@ impl EnvelopeProcessorService { ); } - metric!( - timer(RelayTimers::TransactionProcessingAfterDynamicSampling), - sampling_decision = if sampling_should_drop { "drop" } else { "keep" }, - { - if_processing!(self.inner.config, { - profile::process(state, &self.inner.config); - }); + if_processing!(self.inner.config, { + profile::process(state, &self.inner.config); + }); - if state.has_event() { - event::scrub(state)?; + if state.has_event() { + event::scrub(state)?; - if_processing!(self.inner.config, { - if state - .project_state - .has_feature(Feature::ExtractSpansFromEvent) - { - span::extract_from_event( - state, - &self.inner.config, - &self.inner.global_config.current(), - ); - } - }); + if_processing!(self.inner.config, { + if state + .project_state + .has_feature(Feature::ExtractSpansFromEvent) + { + span::extract_from_event( + state, + &self.inner.config, + &self.inner.global_config.current(), + ); } + }); + } - if_processing!(self.inner.config, { - self.enforce_quotas(state)?; - span::maybe_discard_transaction(state); - }); - if state.has_event() { - event::serialize(state)?; - } - attachment::scrub(state); - } - ); + if_processing!(self.inner.config, { + self.enforce_quotas(state)?; + span::maybe_discard_transaction(state); + }); + if state.has_event() { + event::serialize(state)?; + } + attachment::scrub(state); Ok(()) } diff --git a/relay-server/src/services/processor/dynamic_sampling.rs b/relay-server/src/services/processor/dynamic_sampling.rs index 63156bb4ea..161b0b159a 100644 --- a/relay-server/src/services/processor/dynamic_sampling.rs +++ b/relay-server/src/services/processor/dynamic_sampling.rs @@ -11,14 +11,12 @@ use relay_protocol::{Annotated, Empty}; use relay_sampling::config::RuleType; use relay_sampling::evaluation::{ReservoirEvaluator, SamplingEvaluator}; use relay_sampling::{DynamicSamplingContext, SamplingConfig}; -use relay_statsd::metric; use crate::envelope::ItemType; use crate::services::outcome::Outcome; use crate::services::processor::{ profile, EventProcessing, ProcessEnvelopeState, Sampling, TransactionGroup, }; -use crate::statsd::RelayCounters; use crate::utils::{self, sample, ItemAction, SamplingResult}; /// Ensures there is a valid dynamic sampling context and corresponding project state. @@ -107,10 +105,6 @@ pub fn sample_envelope_items( }; let should_drop = event.ty.value() == Some(&EventType::Transaction) && sampling_match.should_drop(); - metric!( - counter(RelayCounters::DynamicSamplingDecision) += 1, - decision = if should_drop { "drop" } else { "keep" } - ); if should_drop { let unsampled_profiles_enabled = forward_unsampled_profiles(state, global_config); diff --git a/relay-server/src/statsd.rs b/relay-server/src/statsd.rs index 5f9e6f58a4..ba3908e0a4 100644 --- a/relay-server/src/statsd.rs +++ b/relay-server/src/statsd.rs @@ -252,10 +252,6 @@ pub enum RelayTimers { EventProcessingSerialization, /// Time used to extract span metrics from an event. EventProcessingSpanMetricsExtraction, - /// Time spent on transaction processing after dynamic sampling. - /// - /// This includes PII scrubbing and for processing relays also consistent rate limiting. - TransactionProcessingAfterDynamicSampling, /// Time spent between the start of request handling and processing of the envelope. /// /// This includes streaming the request body, scheduling overheads, project config fetching, @@ -419,9 +415,6 @@ impl TimerMetric for RelayTimers { "event_processing.span_metrics_extraction" } RelayTimers::EventProcessingSerialization => "event_processing.serialization", - RelayTimers::TransactionProcessingAfterDynamicSampling => { - "transaction.processing.post_ds" - } RelayTimers::EnvelopeWaitTime => "event.wait_time", RelayTimers::EnvelopeProcessingTime => "event.processing_time", RelayTimers::EnvelopeTotalTime => "event.total_time", @@ -667,11 +660,6 @@ pub enum RelayCounters { /// This metric is tagged with: /// - `success`: whether deserializing the global config succeeded. GlobalConfigFetched, - /// Counter for dynamic sampling decision. - /// - /// This metric is tagged with: - /// - `decision`: "drop" if dynamic sampling drops the envelope, else "keep". - DynamicSamplingDecision, /// Counts how many transactions were created from segment spans. #[cfg(feature = "processing")] TransactionsFromSpans, @@ -716,7 +704,6 @@ impl CounterMetric for RelayCounters { RelayCounters::MetricsTransactionNameExtracted => "metrics.transaction_name", RelayCounters::OpenTelemetryEvent => "event.opentelemetry", RelayCounters::GlobalConfigFetched => "global_config.fetch", - RelayCounters::DynamicSamplingDecision => "dynamic_sampling_decision", #[cfg(feature = "processing")] RelayCounters::TransactionsFromSpans => "transactions_from_spans", RelayCounters::MissingDynamicSamplingContext => "missing_dynamic_sampling_context",