Skip to content

Commit

Permalink
instr(server): Remove cogs related statsd metrics (#3689)
Browse files Browse the repository at this point in the history
These were introduced for an ad-hoc COGS estimate. We're attempting to
reduce the amount of metrics (see
getsentry/team-ingest#353).
  • Loading branch information
jjbayer committed Jun 6, 2024
1 parent 1e100ad commit 1d02602
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 53 deletions.
60 changes: 26 additions & 34 deletions relay-server/src/services/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(())
}
Expand Down
6 changes: 0 additions & 6 deletions relay-server/src/services/processor/dynamic_sampling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);

Expand Down
13 changes: 0 additions & 13 deletions relay-server/src/statsd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 1d02602

Please sign in to comment.