diff --git a/CHANGELOG.md b/CHANGELOG.md index 111213b0d7..1975eb0e97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ **Bug Fixes**: +- Fix a bug where the replays ip-address normalization was not being applied when the user object was omitted. ([#1805](https://github.com/getsentry/relay/pull/1805)) - Improve performance for replays, especially memory usage during data scrubbing.([#1800](https://github.com/getsentry/relay/pull/1800), [#1825](https://github.com/getsentry/relay/pull/1825)) **Internal**: diff --git a/relay-general/src/protocol/replay.rs b/relay-general/src/protocol/replay.rs index d028e13632..c56851fcc5 100644 --- a/relay-general/src/protocol/replay.rs +++ b/relay-general/src/protocol/replay.rs @@ -259,13 +259,12 @@ impl Replay { } fn normalize_ip_address(&mut self, ip_address: Option) { - if let Some(addr) = ip_address { - if let Some(user) = self.user.value_mut() { - if user.ip_address.value().is_none() { - user.ip_address.set_value(Some(IpAddr(addr.to_string()))); - } - } - } + store::normalize_ip_addresses( + &mut self.request, + &mut self.user, + self.platform.as_str(), + ip_address.map(|ip| IpAddr(ip.to_string())).as_ref(), + ) } fn normalize_user_agent(&mut self, default_user_agent: &RawUserAgentInfo<&str>) { @@ -455,12 +454,27 @@ mod tests { fn test_missing_user() { let payload = include_str!("../../tests/fixtures/replays/replay_missing_user.json"); - let mut replay: Annotated = Annotated::from_json(payload).unwrap(); - let replay_value = replay.value_mut().as_mut().unwrap(); - replay_value.normalize(None, &RawUserAgentInfo::default()); + let mut annotated_replay: Annotated = Annotated::from_json(payload).unwrap(); + let replay = annotated_replay.value_mut().as_mut().unwrap(); - let user = replay_value.user.value(); + // No user object and no ip-address was provided. + replay.normalize(None, &RawUserAgentInfo::default()); + let user = replay.user.value(); assert!(user.is_none()); + + // No user object but an ip-address was provided. + let ip_address = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)); + replay.normalize(Some(ip_address), &RawUserAgentInfo::default()); + + let ip_addr = replay + .user + .value() + .unwrap() + .ip_address + .value() + .unwrap() + .as_str(); + assert!(ip_addr == "127.0.0.1"); } #[test] diff --git a/relay-general/src/store/normalize.rs b/relay-general/src/store/normalize.rs index 3aa6265ca7..a3bb558fb0 100644 --- a/relay-general/src/store/normalize.rs +++ b/relay-general/src/store/normalize.rs @@ -596,7 +596,12 @@ fn normalize_security_report( } /// Backfills IP addresses in various places. -fn normalize_ip_addresses(event: &mut Event, client_ip: Option<&IpAddr>) { +pub fn normalize_ip_addresses( + request: &mut Annotated, + user: &mut Annotated, + platform: Option<&str>, + client_ip: Option<&IpAddr>, +) { // NOTE: This is highly order dependent, in the sense that both the statements within this // function need to be executed in a certain order, and that other normalization code // (geoip lookup) needs to run after this. @@ -607,7 +612,7 @@ fn normalize_ip_addresses(event: &mut Event, client_ip: Option<&IpAddr>) { // Resolve {{auto}} if let Some(client_ip) = client_ip { - if let Some(ref mut request) = event.request.value_mut() { + if let Some(ref mut request) = request.value_mut() { if let Some(ref mut env) = request.env.value_mut() { if let Some(&mut Value::String(ref mut http_ip)) = env .get_mut("REMOTE_ADDR") @@ -620,7 +625,7 @@ fn normalize_ip_addresses(event: &mut Event, client_ip: Option<&IpAddr>) { } } - if let Some(ref mut user) = event.user.value_mut() { + if let Some(ref mut user) = user.value_mut() { if let Some(ref mut user_ip) = user.ip_address.value_mut() { if user_ip.is_auto() { *user_ip = client_ip.to_owned(); @@ -630,8 +635,7 @@ fn normalize_ip_addresses(event: &mut Event, client_ip: Option<&IpAddr>) { } // Copy IPs from request interface to user, and resolve platform-specific backfilling - let http_ip = event - .request + let http_ip = request .value() .and_then(|request| request.env.value()) .and_then(|env| env.get("REMOTE_ADDR")) @@ -639,14 +643,12 @@ fn normalize_ip_addresses(event: &mut Event, client_ip: Option<&IpAddr>) { .and_then(|ip| IpAddr::parse(ip).ok()); if let Some(http_ip) = http_ip { - let user = event.user.value_mut().get_or_insert_with(User::default); + let user = user.value_mut().get_or_insert_with(User::default); user.ip_address.value_mut().get_or_insert(http_ip); } else if let Some(client_ip) = client_ip { - let user = event.user.value_mut().get_or_insert_with(User::default); + let user = user.value_mut().get_or_insert_with(User::default); // auto is already handled above if user.ip_address.value().is_none() { - let platform = event.platform.as_str(); - // In an ideal world all SDKs would set {{auto}} explicitly. if let Some("javascript") | Some("cocoa") | Some("objc") = platform { user.ip_address = Annotated::new(client_ip.to_owned()); @@ -700,7 +702,12 @@ pub fn light_normalize_event( normalize_security_report(event, config.client_ip, &config.user_agent); // Insert IP addrs before recursing, since geo lookup depends on it. - normalize_ip_addresses(event, config.client_ip); + normalize_ip_addresses( + &mut event.request, + &mut event.user, + event.platform.as_str(), + config.client_ip, + ); // Validate the basic attributes we extract metrics from event.release.apply(|release, meta| { diff --git a/relay-general/tests/fixtures/replays/replay_missing_user.json b/relay-general/tests/fixtures/replays/replay_missing_user.json index a941f28481..7b82908aca 100644 --- a/relay-general/tests/fixtures/replays/replay_missing_user.json +++ b/relay-general/tests/fixtures/replays/replay_missing_user.json @@ -17,7 +17,7 @@ "4" ], "dist": "1.12", - "platform": "Python", + "platform": "javascript", "environment": "production", "release": "version@1.3", "tags": { diff --git a/relay-general/tests/fixtures/replays/replay_missing_user_ip_address.json b/relay-general/tests/fixtures/replays/replay_missing_user_ip_address.json index 94014c3d62..c012fdc285 100644 --- a/relay-general/tests/fixtures/replays/replay_missing_user_ip_address.json +++ b/relay-general/tests/fixtures/replays/replay_missing_user_ip_address.json @@ -17,7 +17,7 @@ "4" ], "dist": "1.12", - "platform": "Python", + "platform": "javascript", "environment": "production", "release": "version@1.3", "tags": {