feat(spans): Add inbound filters for spans #3552
Conversation
| @@ -98,6 +102,19 @@ pub fn process( | |||
| _ => return ItemAction::Keep, | |||
| }; | |||
|
|
|||
| if let Some(span) = annotated_span.value() { | |||
There was a problem hiding this comment.
After some different implementations, I settled on just putting the filtering here, even though I don't know if it should be put into the process function. The only thing I know is that separating into two functions would require quite a bit of refactoring.
| @@ -375,6 +375,7 @@ impl Getter for SpanData { | |||
| "ui\\.component_name" => self.ui_component_name.value()?.into(), | |||
| "url\\.scheme" => self.url_scheme.value()?.into(), | |||
| "transaction" => self.segment_name.as_str()?.into(), | |||
| "release" => self.release.as_str()?.into(), | |||
There was a problem hiding this comment.
Added this field since it was not there.
| @@ -108,3 +110,41 @@ impl Filterable for Replay { | |||
| self.user_agent() | |||
| } | |||
| } | |||
|
|
|||
| impl Filterable for Span { | |||
There was a problem hiding this comment.
We might want to extend the impl of the methods returning None in case we find a way to access those fields on the Span.
| @@ -3475,10 +3475,10 @@ mod tests { | |||
| processor.handle_process_metrics(&mut token, message); | |||
|
|
|||
| let value = project_cache_rx.recv().await.unwrap(); | |||
| let ProjectCache::MergeBuckets(merge_buckets) = value else { | |||
| let ProjectCache::AddMetricBuckets(add_metric_buckets) = value else { | |||
| @@ -3475,10 +3475,10 @@ mod tests { | |||
| processor.handle_process_metrics(&mut token, message); | |||
|
|
|||
| let value = project_cache_rx.recv().await.unwrap(); | |||
| let ProjectCache::MergeBuckets(merge_buckets) = value else { | |||
| let ProjectCache::AddMetricBuckets(add_metric_buckets) = value else { | |||
There was a problem hiding this comment.
I think you can just merge or rebase on latest master, see #3560.
tests/integration/test_spans.py
Outdated
| assert summarize_outcomes() == {(12, 1): 1} | ||
|
|
||
| spans_consumer.assert_empty() | ||
| outcomes_consumer.assert_empty() |
There was a problem hiding this comment.
nit: Because integration tests take a long time to run, I suggest keeping only one of these two integration tests, and cover other filters with unit tests. See for example
Lines 313 to 326 in b5fa1ff
Co-authored-by: Joris Bayer <[email protected]>
…entry/relay into riccardo/feat/inbound-filters-spans
| @@ -1594,3 +1594,71 @@ def test_span_extraction_with_tags( | |||
| assert transaction_span["tags"] == expected_tags | |||
|
|
|||
| spans_consumer.assert_empty() | |||
|
|
|||
|
|
|||
| def test_span_filtering_with_generic_inbound_filter( | |||
There was a problem hiding this comment.
Created just one integration test for the end-to-end behavior since i tested other filters directly in unit tests.
This PR adds support for inbound filters on spans which as of now should happen only on processing relays.
Closes: #3550