From 75fc8de81aed91837a5c7449d79e033182b32e01 Mon Sep 17 00:00:00 2001 From: Joris Bayer Date: Tue, 21 May 2024 12:32:03 +0200 Subject: [PATCH] fix(metrics): Apply global tags to transaction metrics (#3615) Prerequisite to https://github.com/getsentry/sentry/pull/71004: Global tag mappings were not yet being applied to transaction metrics. Beside the bug fix, this PR also bumps the metrics extraction version(s), and attempts to simplify the extraction function for transactions. --- CHANGELOG.md | 20 +- relay-dynamic-config/src/metrics.rs | 5 +- .../metrics_extraction/transactions/mod.rs | 46 +- relay-server/src/services/processor.rs | 152 ++-- .../fixtures/histogram-outliers.yml | 783 ++++++++++++++++++ tests/integration/test_metrics.py | 71 +- tests/integration/test_spans.py | 4 + 7 files changed, 977 insertions(+), 104 deletions(-) create mode 100644 tests/integration/fixtures/histogram-outliers.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index d6ff75b3da..a67bcab3b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,19 @@ ## Unreleased +**Bug fixes**: + +- Apply globally defined metric tags to legacy transaction metrics. ([#3615](https://github.com/getsentry/relay/pull/3615)) + +**Features**: + +- Apply legacy inbound filters to standalone spans. ([#3552](https://github.com/getsentry/relay/pull/3552)) + **Internal**: +- Send microsecond precision timestamps. ([#3613](https://github.com/getsentry/relay/pull/3613)) - Map outcome reasons for dynamic sampling to reduced set of values. ([#3623](https://github.com/getsentry/relay/pull/3623)) +- Extract status for spans. ([#3606](https://github.com/getsentry/relay/pull/3606)) ## 24.5.0 @@ -12,7 +22,7 @@ - Remove the AWS lambda extension. ([#3568](https://github.com/getsentry/relay/pull/3568)) -**Bug fixes:** +**Bug fixes**: - Properly handle AI metrics from the Python SDK's `@ai_track` decorator. ([#3539](https://github.com/getsentry/relay/pull/3539)) - Mitigate occasional slowness and timeouts of the healthcheck endpoint. The endpoint will now respond promptly an unhealthy state. ([#3567](https://github.com/getsentry/relay/pull/3567)) @@ -22,10 +32,6 @@ - Apple trace-based sampling rules to standalone spans. ([#3476](https://github.com/getsentry/relay/pull/3476)) - Localhost inbound filter filters sudomains of localhost. ([#3608](https://github.com/getsentry/relay/pull/3608)) -**Features**: - -- Apply legacy inbound filters to standalone spans. ([#3552](https://github.com/getsentry/relay/pull/3552)) - **Internal**: - Add metrics extraction config to global config. ([#3490](https://github.com/getsentry/relay/pull/3490), [#3504](https://github.com/getsentry/relay/pull/3504)) @@ -43,10 +49,8 @@ - Add a calculated measurement based on the AI model and the tokens used. ([#3554](https://github.com/getsentry/relay/pull/3554)) - Restrict usage of OTel endpoint. ([#3597](github.com/getsentry/relay/pull/3597)) - Support new cache span ops in metrics and tag extraction. ([#3598](https://github.com/getsentry/relay/pull/3598)) -- Send microsecond precision timestamps. ([#3613](https://github.com/getsentry/relay/pull/3613)) - Extract additional user fields for spans. ([#3599](https://github.com/getsentry/relay/pull/3599)) - Disable `db.redis` span metrics extraction. ([#3600](https://github.com/getsentry/relay/pull/3600)) -- Extract status for spans. ([#3606](https://github.com/getsentry/relay/pull/3606)) ## 24.4.2 @@ -54,7 +58,7 @@ - Stop supporting dynamic sampling mode `"total"`, which adjusted for the client sample rate. ([#3474](https://github.com/getsentry/relay/pull/3474)) -**Bug fixes:** +**Bug fixes**: - Respect country code TLDs when scrubbing span tags. ([#3458](https://github.com/getsentry/relay/pull/3458)) - Extract HTTP status code from span data when sent as integers. ([#3491](https://github.com/getsentry/relay/pull/3491)) diff --git a/relay-dynamic-config/src/metrics.rs b/relay-dynamic-config/src/metrics.rs index a9518c4821..f504bef380 100644 --- a/relay-dynamic-config/src/metrics.rs +++ b/relay-dynamic-config/src/metrics.rs @@ -115,7 +115,8 @@ pub struct CustomMeasurementConfig { /// - Delay metrics extraction for indexed transactions. /// - 4: Adds support for `RuleConfigs` with string comparisons. /// - 5: No change, bumped together with [`MetricExtractionConfig::MAX_SUPPORTED_VERSION`]. -const TRANSACTION_EXTRACT_MAX_SUPPORTED_VERSION: u16 = 5; +/// - 6: Bugfix to make transaction metrics extraction apply globally defined tag mappings. +const TRANSACTION_EXTRACT_MAX_SUPPORTED_VERSION: u16 = 6; /// Deprecated. Defines whether URL transactions should be considered low cardinality. #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq)] @@ -324,7 +325,7 @@ impl MetricExtractionConfig { /// The latest version for this config struct. /// /// This is the maximum version supported by this Relay instance. - pub const MAX_SUPPORTED_VERSION: u16 = 3; + pub const MAX_SUPPORTED_VERSION: u16 = 4; /// Returns an empty `MetricExtractionConfig` with the latest version. /// diff --git a/relay-server/src/metrics_extraction/transactions/mod.rs b/relay-server/src/metrics_extraction/transactions/mod.rs index 1c05896dd1..a05ccf9cc7 100644 --- a/relay-server/src/metrics_extraction/transactions/mod.rs +++ b/relay-server/src/metrics_extraction/transactions/mod.rs @@ -2,7 +2,7 @@ use std::collections::{BTreeMap, BTreeSet}; use relay_base_schema::events::EventType; use relay_common::time::UnixTimestamp; -use relay_dynamic_config::{TagMapping, TransactionMetricsConfig}; +use relay_dynamic_config::{CombinedMetricExtractionConfig, TransactionMetricsConfig}; use relay_event_normalization::utils as normalize_utils; use relay_event_schema::protocol::{ AsPair, BrowserContext, Event, OsContext, PerformanceScoreContext, TraceContext, @@ -247,7 +247,7 @@ impl ExtractedMetrics { /// A utility that extracts metrics from transactions. pub struct TransactionExtractor<'a> { pub config: &'a TransactionMetricsConfig, - pub generic_tags: &'a [TagMapping], + pub generic_config: Option<&'a CombinedMetricExtractionConfig<'a>>, pub transaction_from_dsc: Option<&'a str>, pub sampling_result: &'a SamplingResult, pub has_profile: bool, @@ -469,8 +469,10 @@ impl TransactionExtractor<'_> { // Apply shared tags from generic metric extraction. Transaction metrics will adopt generic // metric extraction, after which this is done automatically. - generic::tmp_apply_tags(&mut metrics.project_metrics, event, self.generic_tags); - generic::tmp_apply_tags(&mut metrics.sampling_metrics, event, self.generic_tags); + if let Some(generic_config) = self.generic_config { + generic::tmp_apply_tags(&mut metrics.project_metrics, event, generic_config.tags()); + generic::tmp_apply_tags(&mut metrics.sampling_metrics, event, generic_config.tags()); + } Ok(metrics) } @@ -500,7 +502,9 @@ fn get_measurement_rating(name: &str, value: f64) -> Option { #[cfg(test)] mod tests { - use relay_dynamic_config::AcceptTransactionNames; + use relay_dynamic_config::{ + AcceptTransactionNames, CombinedMetricExtractionConfig, MetricExtractionConfig, TagMapping, + }; use relay_event_normalization::{ normalize_event, set_default_transaction_source, validate_event_timestamps, validate_transaction, BreakdownsConfig, CombinedMeasurementsConfig, EventValidationConfig, @@ -620,7 +624,7 @@ mod tests { let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -981,7 +985,7 @@ mod tests { let config = TransactionMetricsConfig::default(); let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -1150,7 +1154,7 @@ mod tests { let config: TransactionMetricsConfig = TransactionMetricsConfig::default(); let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -1296,7 +1300,7 @@ mod tests { let config = TransactionMetricsConfig::default(); let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -1371,7 +1375,7 @@ mod tests { let config = TransactionMetricsConfig::default(); let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -1530,7 +1534,7 @@ mod tests { let config = TransactionMetricsConfig::default(); let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -1569,7 +1573,7 @@ mod tests { let config = TransactionMetricsConfig::default(); let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -1637,7 +1641,7 @@ mod tests { let config = TransactionMetricsConfig::default(); let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -1740,7 +1744,7 @@ mod tests { let config = TransactionMetricsConfig::default(); let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -1773,7 +1777,7 @@ mod tests { let config = TransactionMetricsConfig::default(); let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -1810,7 +1814,7 @@ mod tests { let config = TransactionMetricsConfig::default(); let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("root_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -2078,7 +2082,7 @@ mod tests { let config = TransactionMetricsConfig::default(); let extractor = TransactionExtractor { config: &config, - generic_tags: &[], + generic_config: None, transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, @@ -2170,10 +2174,16 @@ mod tests { ]"#, ) .unwrap(); + let generic_config = MetricExtractionConfig { + version: 1, + tags: generic_tags, + ..Default::default() + }; + let combined_config = CombinedMetricExtractionConfig::from(&generic_config); let extractor = TransactionExtractor { config: &config, - generic_tags: &generic_tags, + generic_config: Some(&combined_config), transaction_from_dsc: Some("test_transaction"), sampling_result: &SamplingResult::Pending, has_profile: false, diff --git a/relay-server/src/services/processor.rs b/relay-server/src/services/processor.rs index 960091fe4a..5c7660c983 100644 --- a/relay-server/src/services/processor.rs +++ b/relay-server/src/services/processor.rs @@ -1206,96 +1206,98 @@ impl EnvelopeProcessorService { Ok(()) } - /// Extract metrics from all envelope items. - /// - /// Caveats: - /// - This functionality is incomplete. At this point, extraction is implemented only for - /// transaction events. - fn extract_metrics( + /// Extract transaction metrics. + fn extract_transaction_metrics( &self, state: &mut ProcessEnvelopeState, sampling_result: &SamplingResult, ) -> Result<(), ProcessingError> { + if state.event_metrics_extracted { + return Ok(()); + } + let Some(event) = state.event.value() else { + return Ok(()); + }; + // NOTE: This function requires a `metric_extraction` in the project config. Legacy configs // will upsert this configuration from transaction and conditional tagging fields, even if - // it is not present in the actual project config payload. Once transaction metric - // extraction is moved to generic metrics, this can be converted into an early return. - let config = match &state.project_state.config.metric_extraction { - ErrorBoundary::Ok(ref config) if config.is_enabled() => Some(config), - _ => None, - }; + // it is not present in the actual project config payload. let global = self.inner.global_config.current(); - let global_config = match &global.metric_extraction { - ErrorBoundary::Ok(global_config) => global_config, - #[allow(unused_variables)] - ErrorBoundary::Err(e) => { - if_processing!(self.inner.config, { - // Config is invalid, but we will try to extract what we can with just the - // project config. - relay_log::error!("Failed to parse global extraction config {e}"); - MetricExtractionGroups::EMPTY - } else { - // If there's an error with global metrics extraction, it is safe to assume that this - // Relay instance is not up-to-date, and we should skip extraction. - relay_log::debug!("Failed to parse global extraction config {e}"); - return Ok(()); - }) - } + let combined_config = { + let config = match &state.project_state.config.metric_extraction { + ErrorBoundary::Ok(ref config) if config.is_supported() => config, + _ => return Ok(()), + }; + let global_config = match &global.metric_extraction { + ErrorBoundary::Ok(global_config) => global_config, + #[allow(unused_variables)] + ErrorBoundary::Err(e) => { + if_processing!(self.inner.config, { + // Config is invalid, but we will try to extract what we can with just the + // project config. + relay_log::error!("Failed to parse global extraction config {e}"); + MetricExtractionGroups::EMPTY + } else { + // If there's an error with global metrics extraction, it is safe to assume that this + // Relay instance is not up-to-date, and we should skip extraction. + relay_log::debug!("Failed to parse global extraction config: {e}"); + return Ok(()); + }) + } + }; + CombinedMetricExtractionConfig::new(global_config, config) }; - if let Some(event) = state.event.value() { - if state.event_metrics_extracted { + // Require a valid transaction metrics config. + let tx_config = match &state.project_state.config.transaction_metrics { + Some(ErrorBoundary::Ok(tx_config)) => tx_config, + Some(ErrorBoundary::Err(e)) => { + relay_log::debug!("Failed to parse legacy transaction metrics config: {e}"); return Ok(()); } - - if let Some(config) = config { - let combined_config = CombinedMetricExtractionConfig::new(global_config, config); - - let metrics = crate::metrics_extraction::event::extract_metrics( - event, - state.spans_extracted, - &combined_config, - self.inner - .config - .aggregator_config_for(MetricNamespace::Spans) - .max_tag_value_length, - global.options.span_extraction_sample_rate, - ); - state.event_metrics_extracted |= !metrics.is_empty(); - state.extracted_metrics.project_metrics.extend(metrics); + None => { + relay_log::debug!("Legacy transaction metrics config is missing"); + return Ok(()); } + }; - if let Some(ErrorBoundary::Ok(ref tx_config)) = - state.project_state.config.transaction_metrics - { - if tx_config.is_enabled() - && !state.project_state.has_feature(Feature::DiscardTransaction) - { - let transaction_from_dsc = state - .managed_envelope - .envelope() - .dsc() - .and_then(|dsc| dsc.transaction.as_deref()); - - let extractor = TransactionExtractor { - config: tx_config, - generic_tags: config.map(|c| c.tags.as_slice()).unwrap_or_default(), - transaction_from_dsc, - sampling_result, - has_profile: state.profile_id.is_some(), - }; - - state.extracted_metrics.extend(extractor.extract(event)?); - state.event_metrics_extracted |= true; - } - } + if !tx_config.is_enabled() { + return Ok(()); + } - if state.event_metrics_extracted { - state.managed_envelope.set_event_metrics_extracted(); - } + let metrics = crate::metrics_extraction::event::extract_metrics( + event, + state.spans_extracted, + &combined_config, + self.inner + .config + .aggregator_config_for(MetricNamespace::Spans) + .max_tag_value_length, + global.options.span_extraction_sample_rate, + ); + state.extracted_metrics.project_metrics.extend(metrics); + + if !state.project_state.has_feature(Feature::DiscardTransaction) { + let transaction_from_dsc = state + .managed_envelope + .envelope() + .dsc() + .and_then(|dsc| dsc.transaction.as_deref()); + + let extractor = TransactionExtractor { + config: tx_config, + generic_config: Some(&combined_config), + transaction_from_dsc, + sampling_result, + has_profile: state.profile_id.is_some(), + }; + + state.extracted_metrics.extend(extractor.extract(event)?); } - // NB: Other items can be added here. + state.event_metrics_extracted = true; + state.managed_envelope.set_event_metrics_extracted(); + Ok(()) } @@ -1520,7 +1522,7 @@ impl EnvelopeProcessorService { // 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. if self.inner.config.processing_enabled() || sampling_should_drop { - self.extract_metrics(state, &sampling_result)?; + self.extract_transaction_metrics(state, &sampling_result)?; } dynamic_sampling::sample_envelope_items( diff --git a/tests/integration/fixtures/histogram-outliers.yml b/tests/integration/fixtures/histogram-outliers.yml new file mode 100644 index 0000000000..70d8685e2a --- /dev/null +++ b/tests/integration/fixtures/histogram-outliers.yml @@ -0,0 +1,783 @@ +# --- +# created: "2024-05-16T13:23:17.681110+00:00" +# creator: sentry +# source: tests/sentry/api/endpoints/test_relay_globalconfig_v3.py +# --- +groups: + histogram_outliers: + isEnabled: true + tags: + - metrics: + - d:transactions/duration@millisecond + tags: + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: pageload + - name: event.platform + op: eq + value: javascript + - name: event.duration + op: gte + value: 16123.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: navigation + - name: event.platform + op: eq + value: javascript + - name: event.duration + op: gte + value: 4032.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: http.server + - name: event.platform + op: eq + value: python + - name: event.duration + op: gte + value: 383.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: http.server + - name: event.platform + op: eq + value: node + - name: event.duration + op: gte + value: 506.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: http.server + - name: event.platform + op: eq + value: php + - name: event.duration + op: gte + value: 891.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: ui.load + - name: event.platform + op: eq + value: javascript + - name: event.duration + op: gte + value: 199379.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: celery.task + - name: event.platform + op: eq + value: python + - name: event.duration + op: gte + value: 1516.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: rails.request + - name: event.platform + op: eq + value: ruby + - name: event.duration + op: gte + value: 407.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: queue.task.celery + - name: event.platform + op: eq + value: python + - name: event.duration + op: gte + value: 2637.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: function.nextjs + - name: event.platform + op: eq + value: node + - name: event.duration + op: gte + value: 505.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: ui.load + - name: event.platform + op: eq + value: cocoa + - name: event.duration + op: gte + value: 2387.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: http.server + - name: event.platform + op: eq + value: csharp + - name: event.duration + op: gte + value: 325.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: http.server + - name: event.platform + op: eq + value: ruby + - name: event.duration + op: gte + value: 347.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: ui.load + - name: event.platform + op: eq + value: java + - name: event.duration + op: gte + value: 2889.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: http.server + - name: event.platform + op: eq + value: java + - name: event.duration + op: gte + value: 246.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: awslambda.handler + - name: event.platform + op: eq + value: node + - name: event.duration + op: gte + value: 1747.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: serverless.function + - name: event.platform + op: eq + value: python + - name: event.duration + op: gte + value: 393.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: function.aws.lambda + - name: event.platform + op: eq + value: node + - name: event.duration + op: gte + value: 1633.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: default + - name: event.platform + op: eq + value: javascript + - name: event.duration + op: gte + value: 3216.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: function.aws + - name: event.platform + op: eq + value: python + - name: event.duration + op: gte + value: 1464.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: active_job + - name: event.platform + op: eq + value: ruby + - name: event.duration + op: gte + value: 1059.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: navigation + - name: event.platform + op: eq + value: other + - name: event.duration + op: gte + value: 8706.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: queue.active_job + - name: event.platform + op: eq + value: ruby + - name: event.duration + op: gte + value: 4789.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: sidekiq + - name: event.platform + op: eq + value: ruby + - name: event.duration + op: gte + value: 942.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: pageload + - name: event.platform + op: eq + value: other + - name: event.duration + op: gte + value: 3000.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: console.command + - name: event.platform + op: eq + value: php + - name: event.duration + op: gte + value: 1485.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: queue.sidekiq + - name: event.platform + op: eq + value: ruby + - name: event.duration + op: gte + value: 2262.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: transaction + - name: event.platform + op: eq + value: node + - name: event.duration + op: gte + value: 333.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: ui.action + - name: event.platform + op: eq + value: cocoa + - name: event.duration + op: gte + value: 10400.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: default + - name: event.platform + op: eq + value: node + - name: event.duration + op: gte + value: 1686.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: ui.action.click + - name: event.platform + op: eq + value: cocoa + - name: event.duration + op: gte + value: 14519.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: asgi.server + - name: event.platform + op: eq + value: python + - name: event.duration + op: gte + value: 4690.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: http.server + - name: event.platform + op: eq + value: go + - name: event.duration + op: gte + value: 16.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: sentry.test + - name: event.platform + op: eq + value: php + - name: event.duration + op: gte + value: 4.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: websocket.server + - name: event.platform + op: eq + value: ruby + - name: event.duration + op: gte + value: 16.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: ui.action.click + - name: event.platform + op: eq + value: java + - name: event.duration + op: gte + value: 13211.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: http.server + - name: event.platform + op: eq + value: other + - name: event.duration + op: gte + value: 228.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: test + - name: event.platform + op: eq + value: node + - name: event.duration + op: gte + value: 4284.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: gql + - name: event.platform + op: eq + value: node + - name: event.duration + op: gte + value: 492.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: default + - name: event.platform + op: eq + value: python + - name: event.duration + op: gte + value: 253.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: rails.action_cable + - name: event.platform + op: eq + value: ruby + - name: event.duration + op: gte + value: 20.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: queue.process + - name: event.platform + op: eq + value: php + - name: event.duration + op: gte + value: 850.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: websocket.server + - name: event.platform + op: eq + value: python + - name: event.duration + op: gte + value: 24901.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: rq.task + - name: event.platform + op: eq + value: python + - name: event.duration + op: gte + value: 1435.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: task + - name: event.platform + op: eq + value: python + - name: event.duration + op: gte + value: 1317.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: ui.action.swipe + - name: event.platform + op: eq + value: java + - name: event.duration + op: gte + value: 18818.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: queue.task.rq + - name: event.platform + op: eq + value: python + - name: event.duration + op: gte + value: 3313.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: navigation + - name: event.platform + op: eq + value: java + - name: event.duration + op: gte + value: 9647.0 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: ui.action.scroll + - name: event.platform + op: eq + value: java + - name: event.duration + op: gte + value: 7432.0 + op: and + key: histogram_outlier + value: outlier + - metrics: + - d:transactions/measurements.lcp@millisecond + tags: + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: pageload + - name: event.platform + op: eq + value: javascript + - name: event.measurements.lcp.value + op: gte + value: 7941.899538040161 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: pageload + - name: event.platform + op: eq + value: other + - name: event.measurements.lcp.value + op: gte + value: 4589.822045672948 + op: and + key: histogram_outlier + value: outlier + - metrics: + - d:transactions/measurements.fcp@millisecond + tags: + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: pageload + - name: event.platform + op: eq + value: javascript + - name: event.measurements.fcp.value + op: gte + value: 5897.500002294778 + op: and + key: histogram_outlier + value: outlier + - condition: + inner: + - name: event.contexts.trace.op + op: eq + value: pageload + - name: event.platform + op: eq + value: other + - name: event.measurements.fcp.value + op: gte + value: 3384.3555060724457 + op: and + key: histogram_outlier + value: outlier + - metrics: + - d:transactions/duration@millisecond + - d:transactions/measurements.lcp@millisecond + - d:transactions/measurements.fcp@millisecond + tags: + - condition: + inner: + - name: event.duration + op: gte + value: 0 + op: and + key: histogram_outlier + value: inlier + - metrics: + - d:transactions/duration@millisecond + - d:transactions/measurements.lcp@millisecond + - d:transactions/measurements.fcp@millisecond + tags: + - condition: + inner: [] + op: and + key: histogram_outlier + value: outlier diff --git a/tests/integration/test_metrics.py b/tests/integration/test_metrics.py index ce42c19b73..a69b6a8b8c 100644 --- a/tests/integration/test_metrics.py +++ b/tests/integration/test_metrics.py @@ -1,15 +1,17 @@ import hashlib from collections import defaultdict from datetime import UTC, datetime, timedelta, timezone +from pathlib import Path from sentry_sdk.envelope import Envelope, Item, PayloadRef import json import signal import time +import queue import pytest import requests from requests.exceptions import HTTPError -import queue +import yaml from .test_envelope import generate_transaction_item @@ -1873,3 +1875,70 @@ def test_metrics_with_denied_names( assert volume_metric["tags"]["outcome.reason"] == "denied-name" assert "d:custom/memory_usage@byte" in metrics + + +def test_histogram_outliers(mini_sentry, relay): + with open(Path(__file__).parent / "fixtures/histogram-outliers.yml") as f: + mini_sentry.global_config["metricExtraction"] = yaml.full_load(f) + project_config = mini_sentry.add_full_project_config(project_id=42)["config"] + project_config["transactionMetrics"] = { + "version": 1, + } + project_config["metricExtraction"] = { + "version": 3, + "globalGroups": {"histogram_outliers": {"isEnabled": True}}, + } + project_config["sampling"] = { # Drop everything, to trigger metrics extractino + "version": 2, + "rules": [ + { + "id": 1, + "samplingValue": {"type": "sampleRate", "value": 0.0}, + "type": "transaction", + "condition": {"op": "and", "inner": []}, + } + ], + } + + timestamp = datetime.now(tz=timezone.utc) + + event = { + "type": "transaction", + "transaction": "foo", + "transaction_info": {"source": "url"}, # 'transaction' tag not extracted + "platform": "javascript", + "contexts": { + "trace": { + "op": "pageload", + "trace_id": 32 * "b", + "span_id": 16 * "c", + "type": "trace", + } + }, + "user": {"id": 123}, + "measurements": { + "fcp": {"value": 999999999.0}, + "lcp": {"value": 0.0}, + }, + } + event["timestamp"] = timestamp.isoformat() + event["start_timestamp"] = (timestamp - timedelta(seconds=2)).isoformat() + + relay = relay(mini_sentry, TEST_CONFIG) + relay.send_event(42, event) + + tags = {} + for _ in range(2): + envelope = mini_sentry.captured_events.get() + for item in envelope: + if item.type == "metric_buckets": + buckets = item.payload.json + for bucket in buckets: + if outlier := bucket.get("tags", {}).get("histogram_outlier"): + tags[bucket["name"]] = outlier + + assert tags == { + "d:transactions/measurements.fcp@millisecond": "outlier", + "d:transactions/duration@millisecond": "inlier", + "d:transactions/measurements.lcp@millisecond": "inlier", + } diff --git a/tests/integration/test_spans.py b/tests/integration/test_spans.py index 5a6b703e66..34003b959d 100644 --- a/tests/integration/test_spans.py +++ b/tests/integration/test_spans.py @@ -74,6 +74,7 @@ def test_span_extraction( # We do not accidentally produce to the events topic: assert events_consumer.poll(timeout=2.0) is None + # We _do_ extract span metrics: assert {headers[0] for _, headers in metrics_consumer.get_metrics()} == { ("namespace", b"spans") } @@ -1507,6 +1508,9 @@ def test_rate_limit_indexed_consistent_extracted( relay = relay_with_processing() project_id = 42 project_config = mini_sentry.add_full_project_config(project_id) + # Span metrics won't be extracted without a supported transactionMetrics config. + # Without extraction, the span is treated as `Span`, not `SpanIndexed`. + project_config["config"]["transactionMetrics"] = {"version": 3} project_config["config"]["features"] = [ "projects:span-metrics-extraction", ]