Skip to content

Commit a6da660

Browse files
Dav1ddeDav1dde
committed
fix(filters): Improve inbound filters for EAP items
1 parent 2db307f commit a6da660

4 files changed

Lines changed: 200 additions & 43 deletions

File tree

‎relay-filter/src/interface.rs‎

Lines changed: 54 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
//! This module contains the trait for items that can be filtered by Inbound Filters, plus
22
//! the implementation for [`Event`].
3+
use std::sync::LazyLock;
4+
35
use relay_conventions::attributes::{
46
BROWSER__NAME, BROWSER__VERSION, CLIENT__ADDRESS, SENTRY__RELEASE, SENTRY__SEGMENT__NAME,
57
URL__FULL, USER_AGENT__ORIGINAL,
68
};
9+
use relay_conventions::interpolate::http__request__header__key;
710
use url::Url;
811

912
use relay_event_schema::protocol::{
@@ -196,26 +199,6 @@ impl Filterable for Span {
196199
}
197200
}
198201

199-
impl Filterable for SpanV2 {
200-
fn release(&self) -> Option<&str> {
201-
self.attributes
202-
.value()?
203-
.get_value(SENTRY__RELEASE)?
204-
.as_str()
205-
}
206-
207-
fn transaction(&self) -> Option<&str> {
208-
self.attributes
209-
.value()?
210-
.get_value(SENTRY__SEGMENT__NAME)?
211-
.as_str()
212-
}
213-
214-
fn user_agent(&self) -> UserAgent<'_> {
215-
user_agent_from_attributes(&self.attributes)
216-
}
217-
}
218-
219202
impl Filterable for SessionUpdate {
220203
fn ip_addr(&self) -> Option<&str> {
221204
self.attributes
@@ -256,32 +239,51 @@ impl Filterable for SessionAggregates {
256239
}
257240
}
258241

259-
impl Filterable for OurLog {
260-
fn release(&self) -> Option<&str> {
261-
self.attributes
262-
.value()?
263-
.get_value(SENTRY__RELEASE)?
264-
.as_str()
265-
}
242+
macro_rules! impl_for_attributes {
243+
($ty:ty) => {
244+
impl Filterable for $ty {
245+
fn ip_addr(&self) -> Option<&str> {
246+
self.attributes
247+
.value()?
248+
.get_value(CLIENT__ADDRESS)?
249+
.as_str()
250+
}
266251

267-
fn user_agent(&self) -> UserAgent<'_> {
268-
user_agent_from_attributes(&self.attributes)
269-
}
270-
}
252+
fn release(&self) -> Option<&str> {
253+
self.attributes
254+
.value()?
255+
.get_value(SENTRY__RELEASE)?
256+
.as_str()
257+
}
271258

272-
impl Filterable for TraceMetric {
273-
fn release(&self) -> Option<&str> {
274-
self.attributes
275-
.value()?
276-
.get_value(SENTRY__RELEASE)?
277-
.as_str()
278-
}
259+
fn transaction(&self) -> Option<&str> {
260+
self.attributes
261+
.value()?
262+
.get_value(SENTRY__SEGMENT__NAME)?
263+
.as_str()
264+
}
279265

280-
fn user_agent(&self) -> UserAgent<'_> {
281-
user_agent_from_attributes(&self.attributes)
282-
}
266+
fn url(&self) -> Option<Url> {
267+
let url = self.attributes.value()?.get_value(URL__FULL)?.as_str()?;
268+
Url::parse(url).ok()
269+
}
270+
271+
fn user_agent(&self) -> UserAgent<'_> {
272+
user_agent_from_attributes(&self.attributes)
273+
}
274+
275+
fn header(&self, header_name: &str) -> Option<&str> {
276+
let key = http__request__header__key(header_name);
277+
self.attributes.value()?.get_value(&key)?.as_str()
278+
}
279+
}
280+
};
283281
}
284282

283+
impl_for_attributes!(SpanV2);
284+
impl_for_attributes!(OurLog);
285+
impl_for_attributes!(TraceMetric);
286+
285287
fn user_agent_from_attributes(attributes: &relay_protocol::Annotated<Attributes>) -> UserAgent<'_> {
286288
let parsed = (|| {
287289
let attributes = attributes.value()?;
@@ -298,10 +300,20 @@ fn user_agent_from_attributes(attributes: &relay_protocol::Annotated<Attributes>
298300
})
299301
})();
300302

301-
let raw = attributes
303+
static HTTP_USER_AGENT: LazyLock<String> =
304+
LazyLock::new(|| http__request__header__key("user-agent"));
305+
306+
let ua_original = attributes
302307
.value()
303308
.and_then(|attr| attr.get_value(USER_AGENT__ORIGINAL))
304309
.and_then(|ua| ua.as_str());
305310

311+
let ua_header = attributes
312+
.value()
313+
.and_then(|attr| attr.get_value(&*HTTP_USER_AGENT))
314+
.and_then(|ua| ua.as_str());
315+
316+
let raw = ua_original.or(ua_header);
317+
306318
UserAgent { raw, parsed }
307319
}

‎tests/integration/test_ourlogs.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -937,6 +937,55 @@ def test_browser_name_version_extraction(
937937
{},
938938
id="release",
939939
),
940+
pytest.param(
941+
"filtered-transaction",
942+
{"ignoreTransactions": {"isEnabled": True, "patterns": ["*health*"]}},
943+
{
944+
"attributes": {
945+
"sentry.segment.name": {
946+
"value": "/foo/healthz",
947+
"type": "string",
948+
}
949+
}
950+
},
951+
id="transaction",
952+
),
953+
pytest.param(
954+
"localhost",
955+
{"localhost": {"isEnabled": True}},
956+
{
957+
"attributes": {
958+
"client.address": {"value": "127.0.0.1", "type": "string"}
959+
}
960+
},
961+
id="localhost-ip",
962+
),
963+
pytest.param(
964+
"localhost",
965+
{"localhost": {"isEnabled": True}},
966+
{
967+
"attributes": {
968+
"url.full": {
969+
"value": "http://localhost:8000/foo",
970+
"type": "string",
971+
}
972+
}
973+
},
974+
id="localhost-url",
975+
),
976+
pytest.param(
977+
"localhost",
978+
{"localhost": {"isEnabled": True}},
979+
{
980+
"attributes": {
981+
"http.request.header.Host": {
982+
"value": "localhost:8000",
983+
"type": "string",
984+
}
985+
}
986+
},
987+
id="localhost-header",
988+
),
940989
pytest.param(
941990
"legacy-browsers",
942991
{"legacyBrowsers": {"isEnabled": True, "options": ["ie9"]}},
@@ -1021,6 +1070,14 @@ def test_filters_are_applied_to_logs(
10211070
"attributes": {
10221071
"some_integer": {"value": 123, "type": "integer"},
10231072
"sentry.release": {"value": "foobar@1.0", "type": "string"},
1073+
**args.get("attributes", {}),
1074+
},
1075+
},
1076+
metadata={
1077+
"version": 2,
1078+
"ingest_settings": {
1079+
"infer_ip": "never",
1080+
"infer_user_agent": "auto",
10241081
},
10251082
},
10261083
)

‎tests/integration/test_spansv2.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,42 @@ def test_spansv2_ds_root_in_different_org(
917917
{},
918918
id="transaction",
919919
),
920+
pytest.param(
921+
"localhost",
922+
{"localhost": {"isEnabled": True}},
923+
{
924+
"attributes": {
925+
"client.address": {"value": "127.0.0.1", "type": "string"}
926+
}
927+
},
928+
id="localhost-ip",
929+
),
930+
pytest.param(
931+
"localhost",
932+
{"localhost": {"isEnabled": True}},
933+
{
934+
"attributes": {
935+
"url.full": {
936+
"value": "http://localhost:8000/foo",
937+
"type": "string",
938+
}
939+
}
940+
},
941+
id="localhost-url",
942+
),
943+
pytest.param(
944+
"localhost",
945+
{"localhost": {"isEnabled": True}},
946+
{
947+
"attributes": {
948+
"http.request.header.Host": {
949+
"value": "localhost:8000",
950+
"type": "string",
951+
}
952+
}
953+
},
954+
id="localhost-header",
955+
),
920956
pytest.param(
921957
"legacy-browsers",
922958
{"legacyBrowsers": {"isEnabled": True, "options": ["ie9"]}},
@@ -998,11 +1034,13 @@ def test_spanv2_inbound_filters(
9981034
"some_integer": {"value": 123, "type": "integer"},
9991035
"sentry.release": {"value": "foobar@1.0", "type": "string"},
10001036
"sentry.segment.name": {"value": "/foo/healthz", "type": "string"},
1037+
**args.get("attributes", {}),
10011038
},
10021039
},
10031040
metadata={
10041041
"version": 2,
10051042
"ingest_settings": {
1043+
"infer_ip": "never",
10061044
"infer_user_agent": "auto",
10071045
},
10081046
},

‎tests/integration/test_trace_metrics.py‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1028,6 +1028,55 @@ def test_trace_metric_container_metadata(
10281028
{},
10291029
id="release",
10301030
),
1031+
pytest.param(
1032+
"filtered-transaction",
1033+
{"ignoreTransactions": {"isEnabled": True, "patterns": ["*health*"]}},
1034+
{
1035+
"attributes": {
1036+
"sentry.segment.name": {
1037+
"value": "/foo/healthz",
1038+
"type": "string",
1039+
}
1040+
}
1041+
},
1042+
id="transaction",
1043+
),
1044+
pytest.param(
1045+
"localhost",
1046+
{"localhost": {"isEnabled": True}},
1047+
{
1048+
"attributes": {
1049+
"client.address": {"value": "127.0.0.1", "type": "string"}
1050+
}
1051+
},
1052+
id="localhost-ip",
1053+
),
1054+
pytest.param(
1055+
"localhost",
1056+
{"localhost": {"isEnabled": True}},
1057+
{
1058+
"attributes": {
1059+
"url.full": {
1060+
"value": "http://localhost:8000/foo",
1061+
"type": "string",
1062+
}
1063+
}
1064+
},
1065+
id="localhost-url",
1066+
),
1067+
pytest.param(
1068+
"localhost",
1069+
{"localhost": {"isEnabled": True}},
1070+
{
1071+
"attributes": {
1072+
"http.request.header.Host": {
1073+
"value": "localhost:8000",
1074+
"type": "string",
1075+
}
1076+
}
1077+
},
1078+
id="localhost-header",
1079+
),
10311080
pytest.param(
10321081
"legacy-browsers",
10331082
{"legacyBrowsers": {"isEnabled": True, "options": ["ie9"]}},
@@ -1101,7 +1150,7 @@ def test_filters_are_applied_to_trace_metrics(
11011150

11021151
metadata = {
11031152
"version": 2,
1104-
"ingest_settings": {"infer_ip": "auto", "infer_user_agent": "auto"},
1153+
"ingest_settings": {"infer_ip": "never", "infer_user_agent": "auto"},
11051154
}
11061155

11071156
envelope = envelope_with_trace_metrics(
@@ -1114,6 +1163,7 @@ def test_filters_are_applied_to_trace_metrics(
11141163
"attributes": {
11151164
"http.status_code": {"value": 500, "type": "integer"},
11161165
"sentry.release": {"value": "foobar@1.0", "type": "string"},
1166+
**args.get("attributes", {}),
11171167
},
11181168
},
11191169
metadata=metadata,

0 commit comments

Comments
 (0)