Skip to content

Commit

Permalink
ref(normalization): Remove duplicated functions (#2994)
Browse files Browse the repository at this point in the history 
Currently, there are two normalization functions that are exactly the
same for the following steps:

- IP addresses.
- GeoInfo.
- App start spans.

This PR removes one of them and uses the other.
  • Loading branch information
@iker-barriocanal
iker-barriocanal authored Jan 24, 2024
1 parent 13696a7 commit f437f20
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 117 deletions.
38 changes: 3 additions & 35 deletions relay-event-normalization/src/event.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ fn normalize(event: &mut Event, meta: &mut Meta, config: &NormalizationConfig) -

if let Some(geoip_lookup) = config.geoip_lookup {
if let Some(user) = event.user.value_mut() {
normalize_user_geoinfo(geoip_lookup, user)
normalize_user_geoinfo(geoip_lookup, user);
}
}

Expand Down Expand Up @@ -284,7 +284,7 @@ fn normalize(event: &mut Event, meta: &mut Meta, config: &NormalizationConfig) -
// transactions don't have many spans, but if this is no longer the
// case and we roll this flag out for most projects, we may want to
// reconsider this approach.
normalize_app_start_spans(event);
crate::normalize::normalize_app_start_spans(event);
span::attributes::normalize_spans(event, &BTreeSet::from([SpanAttribute::ExclusiveTime]));
}

Expand Down Expand Up @@ -419,7 +419,7 @@ pub fn normalize_ip_addresses(
}

/// Sets the user's GeoIp info based on user's IP address.
fn normalize_user_geoinfo(geoip_lookup: &GeoIpLookup, user: &mut User) {
pub fn normalize_user_geoinfo(geoip_lookup: &GeoIpLookup, user: &mut User) {
// Infer user.geo from user.ip_address
if user.geo.value().is_none() {
if let Some(ip_address) = user.ip_address.value() {
Expand Down Expand Up @@ -890,38 +890,6 @@ fn normalize_breakdowns(event: &mut Event, breakdowns_config: Option<&Breakdowns
}
}

/// Replaces snake_case app start spans op with dot.case op.
///
/// This is done for the affected React Native SDK versions (from 3 to 4.4).
fn normalize_app_start_spans(event: &mut Event) {
if !event.sdk_name().eq("sentry.javascript.react-native")
|| !(event.sdk_version().starts_with("4.4")
|| event.sdk_version().starts_with("4.3")
|| event.sdk_version().starts_with("4.2")
|| event.sdk_version().starts_with("4.1")
|| event.sdk_version().starts_with("4.0")
|| event.sdk_version().starts_with('3'))
{
return;
}

if let Some(spans) = event.spans.value_mut() {
for span in spans {
if let Some(span) = span.value_mut() {
if let Some(op) = span.op.value() {
if op == "app_start_cold" {
span.op.set_value(Some("app.start.cold".to_string()));
break;
} else if op == "app_start_warm" {
span.op.set_value(Some("app.start.warm".to_string()));
break;
}
}
}
}
}
}

/// Normalizes incoming contexts for the downstream metric extraction.
fn normalize_contexts(contexts: &mut Annotated<Contexts>) {
let _ = processor::apply(contexts, |contexts, _meta| {
Expand Down
87 changes: 7 additions & 80 deletions relay-event-normalization/src/normalize/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use relay_event_schema::processor::{
};
use relay_event_schema::protocol::{
Breadcrumb, ClientSdkInfo, Context, Contexts, DebugImage, Event, EventId, EventType, Exception,
Frame, IpAddr, Level, MetricSummaryMapping, NelContext, ReplayContext, Request, Stacktrace,
TraceContext, User, VALID_PLATFORMS,
Frame, Level, MetricSummaryMapping, NelContext, ReplayContext, Stacktrace, TraceContext, User,
VALID_PLATFORMS,
};
use relay_protocol::{
Annotated, Empty, Error, ErrorKind, FromValue, IntoValue, Meta, Object, Remark, RemarkType,
Expand Down Expand Up @@ -184,7 +184,7 @@ impl<'a> StoreNormalizeProcessor<'a> {
/// Replaces snake_case app start spans op with dot.case op.
///
/// This is done for the affected React Native SDK versions (from 3 to 4.4).
fn normalize_app_start_spans(event: &mut Event) {
pub fn normalize_app_start_spans(event: &mut Event) {
if !event.sdk_name().eq("sentry.javascript.react-native")
|| !(event.sdk_version().starts_with("4.4")
|| event.sdk_version().starts_with("4.3")
Expand Down Expand Up @@ -252,80 +252,6 @@ fn normalize_all_metrics_summaries(event: &mut Event) {
}
}

/// Backfills IP addresses in various places.
pub fn normalize_ip_addresses(
request: &mut Annotated<Request>,
user: &mut Annotated<User>,
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.
//
// 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) = 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) = 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 = 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 = 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 = user.value_mut().get_or_insert_with(User::default);
// auto is already handled above
if user.ip_address.value().is_none() {
// 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());
}
}
}
}

// Sets the user's GeoIp info based on user's IP address.
fn normalize_user_geoinfo(geoip_lookup: &GeoIpLookup, user: &mut User) {
// Infer user.geo from user.ip_address
if user.geo.value().is_none() {
if let Some(ip_address) = user.ip_address.value() {
if let Ok(Some(geo)) = geoip_lookup.lookup(ip_address.as_str()) {
user.geo.set_value(Some(geo));
}
}
}
}

/// Container for global and project level [`MeasurementsConfig`]. The purpose is to handle
/// the merging logic.
#[derive(Clone, Debug)]
Expand Down Expand Up @@ -524,7 +450,7 @@ impl<'a> Processor for StoreNormalizeProcessor<'a> {

// Infer user.geo from user.ip_address
if let Some(geoip_lookup) = self.geoip_lookup {
normalize_user_geoinfo(geoip_lookup, user)
crate::event::normalize_user_geoinfo(geoip_lookup, user)
}

Ok(())
Expand Down Expand Up @@ -759,8 +685,9 @@ mod tests {
use relay_base_schema::spans::SpanStatus;
use relay_event_schema::processor::process_value;
use relay_event_schema::protocol::{
ContextInner, DebugMeta, Frame, Geo, LenientString, LogEntry, MetricSummary,
MetricsSummary, PairList, RawStacktrace, Span, SpanId, TagEntry, Tags, TraceId, Values,
ContextInner, DebugMeta, Frame, Geo, IpAddr, LenientString, LogEntry, MetricSummary,
MetricsSummary, PairList, RawStacktrace, Request, Span, SpanId, TagEntry, Tags, TraceId,
Values,
};
use relay_protocol::{
assert_annotated_snapshot, get_path, get_value, FromValue, SerializableAnnotated,
Expand Down
4 changes: 2 additions & 2 deletions relay-event-normalization/src/replay.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,12 @@ fn normalize_array_fields(replay: &mut Replay) {
}

fn normalize_ip_address(replay: &mut Replay, ip_address: Option<StdIpAddr>) {
crate::normalize_ip_addresses(
crate::event::normalize_ip_addresses(
&mut replay.request,
&mut replay.user,
replay.platform.as_str(),
ip_address.map(|ip| IpAddr(ip.to_string())).as_ref(),
)
);
}

fn normalize_user_agent(replay: &mut Replay, default_user_agent: &RawUserAgentInfo<&str>) {
Expand Down

0 comments on commit f437f20

Please sign in to comment.