Skip to content

Commit

Permalink
fix(spans): Use user_agent.original for browser name (#3834)
Browse files Browse the repository at this point in the history
Use `user_agent.original` to derive the browser name, and only use
request headers if that field has not been set.

This PR also refactors span normalization, such that there is no more
fake event necessary to derive user agent information or performance
scores.
  • Loading branch information
jjbayer committed Jul 22, 2024
1 parent 4d5941a commit 1c49b90
Show file tree
Hide file tree
Showing 6 changed files with 339 additions and 132 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
**Features**:

- "Cardinality limit" outcomes now report which limit was exceeded. ([#3825](https://github.com/getsentry/relay/pull/3825))
- Derive span browser name from user agent. ([#3834](https://github.com/getsentry/relay/pull/3834))

**Internal**:

Expand Down
107 changes: 65 additions & 42 deletions relay-event-normalization/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ use relay_event_schema::processor::{self, ProcessingAction, ProcessingState, Pro
use relay_event_schema::protocol::{
AsPair, ClientSdkInfo, Context, ContextInner, Contexts, DebugImage, DeviceClass, Event,
EventId, EventType, Exception, Headers, IpAddr, Level, LogEntry, Measurement, Measurements,
MetricSummaryMapping, NelContext, PerformanceScoreContext, ReplayContext, Request, SpanStatus,
Tags, Timestamp, TraceContext, User, VALID_PLATFORMS,
MetricSummaryMapping, NelContext, PerformanceScoreContext, ReplayContext, Request, Span,
SpanStatus, Tags, Timestamp, TraceContext, User, VALID_PLATFORMS,
};
use relay_protocol::{
Annotated, Empty, Error, ErrorKind, FromValue, IntoValue, Meta, Object, Remark, RemarkType,
Value,
Annotated, Empty, Error, ErrorKind, FromValue, Getter, IntoValue, Meta, Object, Remark,
RemarkType, Value,
};
use smallvec::SmallVec;
use uuid::Uuid;
Expand Down Expand Up @@ -297,7 +297,13 @@ fn normalize(event: &mut Event, meta: &mut Meta, config: &NormalizationConfig) {
config.measurements.clone(),
config.max_name_and_unit_len,
); // Measurements are part of the metric extraction
normalize_performance_score(event, config.performance_score);
if let Some(version) = normalize_performance_score(event, config.performance_score) {
event
.contexts
.get_or_insert_with(Contexts::new)
.get_or_default::<PerformanceScoreContext>()
.score_profile_version = Annotated::new(version);
}
normalize_ai_measurements(event, config.ai_model_costs);
normalize_breakdowns(event, config.breakdowns_config); // Breakdowns are part of the metric extraction too
normalize_default_attributes(event, meta, config);
Expand Down Expand Up @@ -852,23 +858,28 @@ pub fn normalize_measurements(
}
}

pub trait MutMeasurements {
fn measurements(&mut self) -> &mut Annotated<Measurements>;
}

/// Computes performance score measurements for an event.
///
/// This computes score from vital measurements, using config options to define how it is
/// calculated.
pub fn normalize_performance_score(
event: &mut Event,
event: &mut (impl Getter + MutMeasurements),
performance_score: Option<&PerformanceScoreConfig>,
) {
) -> Option<String> {
let mut version = None;
let Some(performance_score) = performance_score else {
return;
return version;
};
for profile in &performance_score.profiles {
if let Some(condition) = &profile.condition {
if !condition.matches(event) {
continue;
}
if let Some(measurements) = event.measurements.value_mut() {
if let Some(measurements) = event.measurements().value_mut() {
let mut should_add_total = false;
if profile.score_components.iter().any(|c| {
!measurements.contains_key(c.measurement.as_str())
Expand Down Expand Up @@ -929,13 +940,7 @@ pub fn normalize_performance_score(
);
}
if should_add_total {
if let Some(version) = &profile.version {
event
.contexts
.get_or_insert_with(Contexts::new)
.get_or_default::<PerformanceScoreContext>()
.score_profile_version = version.clone().into();
}
version.clone_from(&profile.version);
measurements.insert(
"score.total".to_owned(),
Measurement {
Expand All @@ -949,6 +954,19 @@ pub fn normalize_performance_score(
break; // Stop after the first matching profile.
}
}
version
}

impl MutMeasurements for Event {
fn measurements(&mut self) -> &mut Annotated<Measurements> {
&mut self.measurements
}
}

impl MutMeasurements for Span {
fn measurements(&mut self) -> &mut Annotated<Measurements> {
&mut self.measurements
}
}

/// Compute additional measurements derived from existing ones.
Expand Down Expand Up @@ -3315,35 +3333,40 @@ mod tests {
}))
.unwrap();

normalize_performance_score(&mut event, Some(&performance_score));
normalize(
&mut event,
&mut Meta::default(),
&NormalizationConfig {
performance_score: Some(&performance_score),
..Default::default()
},
);

insta::assert_ron_snapshot!(SerializableAnnotated(&Annotated::new(event)), {}, @r###"
insta::assert_ron_snapshot!(SerializableAnnotated(&event.contexts), {}, @r###"
{
"type": "transaction",
"timestamp": 1619420405.0,
"start_timestamp": 1619420400.0,
"contexts": {
"performance_score": {
"score_profile_version": "beta",
"type": "performancescore",
},
"performance_score": {
"score_profile_version": "beta",
"type": "performancescore",
},
"measurements": {
"inp": {
"value": 120.0,
},
"score.inp": {
"value": 0.0,
"unit": "ratio",
},
"score.total": {
"value": 0.0,
"unit": "ratio",
},
"score.weight.inp": {
"value": 1.0,
"unit": "ratio",
},
}
"###);
insta::assert_ron_snapshot!(SerializableAnnotated(&event.measurements), {}, @r###"
{
"inp": {
"value": 120.0,
"unit": "millisecond",
},
"score.inp": {
"value": 0.0,
"unit": "ratio",
},
"score.total": {
"value": 0.0,
"unit": "ratio",
},
"score.weight.inp": {
"value": 1.0,
"unit": "ratio",
},
}
"###);
Expand Down
7 changes: 5 additions & 2 deletions relay-event-normalization/src/normalize/user_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,14 @@ impl ClientHints<String> {
}

/// Computes a [`Context`] from either a user agent string and client hints.
trait FromUserAgentInfo: Sized {
pub trait FromUserAgentInfo: Sized {
/// Tries to populate the context from client hints.
fn parse_client_hints(client_hints: &ClientHints<&str>) -> Option<Self>;

/// Tries to populate the context from a user agent header string.
fn parse_user_agent(user_agent: &str) -> Option<Self>;

/// Tries to populate the context from client hints or a user agent header string.
fn from_hints_or_ua(raw_info: &RawUserAgentInfo<&str>) -> Option<Self> {
Self::parse_client_hints(&raw_info.client_hints)
.or_else(|| raw_info.user_agent.and_then(Self::parse_user_agent))
Expand Down Expand Up @@ -362,7 +365,7 @@ impl FromUserAgentInfo for BrowserContext {
/// to be a browser and gets returned as such.
///
/// Returns None if no browser field detected.
fn browser_from_client_hints(s: &str) -> Option<(String, String)> {
pub fn browser_from_client_hints(s: &str) -> Option<(String, String)> {
for item in s.split(',') {
// if it contains one of these then we can know it isn't a browser field. atm chromium
// browsers are the only ones supporting client hints.
Expand Down
1 change: 1 addition & 0 deletions relay-event-schema/src/protocol/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ impl Getter for Span {
"release" => self.data.value()?.release.as_str()?.into(),
"environment" => self.data.value()?.environment.as_str()?.into(),
"transaction" => self.data.value()?.segment_name.as_str()?.into(),
"contexts.browser.name" => self.data.value()?.browser_name.as_str()?.into(),
// TODO: we might want to add additional fields once they are added to the span.
_ => return None,
})
Expand Down
Loading

0 comments on commit 1c49b90

Please sign in to comment.