bug(replays): Add default user object and fill its ip-address

[cmanallen]

If the User object is null we currently skip the ip normalization step. Now (if we have an ip-address) we will add a default User if one does not exist and add the ip-address to that User. This fixes a bug where a large number of replays were being ingested without an ip-address.

---

Edit: out of date. Left for historical purposes.

I also observed this functionality:

relay/relay-general/src/store/normalize.rs

Lines 593 to 650 in 0151447

	fn normalize_ip_addresses(event: &mut Event, 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.
	//
	// After a series of regressions over the old Python spaghetti code we decided to put it
	// back into one function. If a desire to split this code up overcomes you, put this in a
	// new processor and make sure all of it runs before the rest of normalization.
	// Resolve {{auto}}
	if let Some(client_ip) = client_ip {
	if let Some(ref mut request) = event.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")
	.and_then(|annotated| annotated.value_mut().as_mut())
	{
	if http_ip == "{{auto}}" {
	*http_ip = client_ip.to_string();
	}
	}
	}
	}
	if let Some(ref mut user) = event.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();
	}
	}
	}
	}
	// Copy IPs from request interface to user, and resolve platform-specific backfilling
	let http_ip = event
	.request
	.value()
	.and_then(|request| request.env.value())
	.and_then(|env| env.get("REMOTE_ADDR"))
	.and_then(Annotated::<Value>::as_str)
	.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);
	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);
	// 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());
	}
	}
	}
	}

Obviously my additions are much less than what this function encompasses. Some of this seems related to back-end SDKs which may be outside of scope. I did notice lines 617-620 seem to handle the case where the user's ip was submitted as auto. Is this something the JS SDKs do?

[cmanallen]

I decided to make the ip-address normalization function generic and implement it for event and replay.

[jan-auer]

Thinking out loud, instead of a function with a large signature like this, we could decompose it into smaller functions. For instance, I'm noticing that we don't have to pass the user in. Instead, this function could return an Option<IpAddr> and then the caller runs get_or_insert_with(|| infer_ip_address(/* ... */)).

[cmanallen]

@jan-auer I did break out some of the functionality into different units but I kept this function with the large signature. I didn't want to duplicate implementation details across two modules. If there's a better way to do this let me know. I'd also like to expedite this particular pull if possible. Its preventing ip-address ingest in prod currently.

[jan-auer]

Similarly, here we could have two separate functions. One that resolves {{auto}}, and another one that copies IP addresses over.

[jjbayer]

@cmanallen I found it hard to judge by the diff whether your refactor had any functional changes that could negatively impact event processing, so I reverted parts of the refactor to make it easier to understand what actually changes. Let me know if I missed something.