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", ]