-
Notifications
You must be signed in to change notification settings - Fork 103
ref(processing): Use new transactions processor #5379
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.
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
base: master
Are you sure you want to change the base?
Changes from 49 commits
9fd743a
4fd0055
cea5ff1
989b031
b962ef8
f063ad6
add0dd8
40b1326
ce20017
4ccc557
d8a903a
4737ed3
9c27ac4
a1b42a2
6f242b8
fb2c132
799103e
0ce93c7
1ebc03f
0120631
a2600b2
a9beb7c
5acad09
8031075
976c7cd
886dbe7
c095090
b1a4136
a336c2c
dd6e913
c23617f
11861ed
06410b2
6991a3a
27f059c
49d5fcb
753c2e9
0263642
747a9a2
6f1c486
e785f23
cf08b4f
5256ccf
50c95a7
b54d161
ef89349
c432508
c0f78f9
903f969
c180a9f
cbc73f4
72062fe
1030517
7c61e54
ef1e88f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,12 +6,14 @@ use crate::processing::logs::LogsProcessor; | |||||
| use crate::processing::sessions::SessionsProcessor; | ||||||
| use crate::processing::spans::SpansProcessor; | ||||||
| use crate::processing::trace_metrics::TraceMetricsProcessor; | ||||||
| use crate::processing::transactions::TransactionProcessor; | ||||||
| use crate::processing::{Forward, Processor}; | ||||||
|
|
||||||
| macro_rules! outputs { | ||||||
| ($($variant:ident => $ty:ty,)*) => { | ||||||
| /// All known [`Processor`] outputs. | ||||||
| #[derive(Debug)] | ||||||
| #[allow(clippy::large_enum_variant)] | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Alternatively we can also box the transaction output, I think that kinda depends why the lint triggers, if it's just because of a small-ish amount it's fine, if there is a large discrepancy maybe it's worth boxing. |
||||||
| pub enum Outputs { | ||||||
| $( | ||||||
| $variant(<$ty as Processor>::Output) | ||||||
|
|
@@ -57,4 +59,5 @@ outputs!( | |||||
| TraceMetrics => TraceMetricsProcessor, | ||||||
| Spans => SpansProcessor, | ||||||
| Sessions => SessionsProcessor, | ||||||
| Transactions => TransactionProcessor, | ||||||
| ); | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,12 +57,16 @@ pub fn extract_metrics( | |
| metrics_extracted, | ||
| spans_extracted, | ||
| } = ctx; | ||
| // TODO(follow-up): this function should always extract metrics. Dynamic sampling should validate | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wanna link to a GH issue here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done: #5403 |
||
| // the full metrics extraction config and skip sampling if it is incomplete. | ||
|
|
||
| if metrics_extracted { | ||
| return Ok(EventMetricsExtracted(metrics_extracted)); | ||
| debug_assert!(false, "metrics extraction called twice"); | ||
| return Ok(EventMetricsExtracted(true)); | ||
| } | ||
| let Some(event) = event.value_mut() else { | ||
| return Ok(EventMetricsExtracted(metrics_extracted)); | ||
| // Nothing to extract, but metrics extraction was called. | ||
| return Ok(EventMetricsExtracted(true)); | ||
| }; | ||
|
|
||
| // NOTE: This function requires a `metric_extraction` in the project config. Legacy configs | ||
|
|
@@ -71,7 +75,7 @@ pub fn extract_metrics( | |
| let combined_config = { | ||
| let config = match &ctx.project_info.config.metric_extraction { | ||
| ErrorBoundary::Ok(config) if config.is_supported() => config, | ||
| _ => return Ok(EventMetricsExtracted(metrics_extracted)), | ||
| _ => return Ok(EventMetricsExtracted(false)), | ||
| }; | ||
| let global_config = match &ctx.global_config.metric_extraction { | ||
| ErrorBoundary::Ok(global_config) => global_config, | ||
|
|
@@ -86,7 +90,7 @@ pub fn extract_metrics( | |
| // 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(EventMetricsExtracted(metrics_extracted)); | ||
| return Ok(EventMetricsExtracted(false)); | ||
| } | ||
| } | ||
| }; | ||
|
|
@@ -98,11 +102,11 @@ pub fn extract_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(EventMetricsExtracted(metrics_extracted)); | ||
| return Ok(EventMetricsExtracted(false)); | ||
| } | ||
| None => { | ||
| relay_log::debug!("Legacy transaction metrics config is missing"); | ||
| return Ok(EventMetricsExtracted(metrics_extracted)); | ||
| return Ok(EventMetricsExtracted(false)); | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -117,7 +121,7 @@ pub fn extract_metrics( | |
| } | ||
| }); | ||
|
|
||
| return Ok(EventMetricsExtracted(metrics_extracted)); | ||
| return Ok(EventMetricsExtracted(false)); | ||
| } | ||
|
|
||
| // If spans were already extracted for an event, we rely on span processing to extract metrics. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.