Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add client hints for requestmeta #1802

Merged
merged 39 commits into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
1a398b0
wip
TBS1996 Jan 31, 2023
244196a
wip
TBS1996 Jan 31, 2023
9f4a092
wip
TBS1996 Feb 1, 2023
420d9ca
wip
TBS1996 Feb 1, 2023
6cb6e9c
wip
TBS1996 Feb 1, 2023
e8ccc58
savepoint
TBS1996 Feb 1, 2023
d1cbbb8
wip
TBS1996 Feb 1, 2023
f1b908a
wip
TBS1996 Feb 1, 2023
6a26595
wip
TBS1996 Feb 1, 2023
6b78a21
wip
TBS1996 Feb 1, 2023
4c2640d
wip
TBS1996 Feb 2, 2023
7c18540
wip
TBS1996 Feb 3, 2023
f626c7d
wip
TBS1996 Feb 3, 2023
1eec4c4
wip
TBS1996 Feb 3, 2023
f14d667
wip
TBS1996 Feb 3, 2023
c0f7170
wip
TBS1996 Feb 3, 2023
3272c9e
add test for request_meta serialization
TBS1996 Feb 6, 2023
b909ed4
Merge branch 'master' into feat/reqmetahints
TBS1996 Feb 6, 2023
87b54b5
wip
TBS1996 Feb 6, 2023
03a5619
Merge branch 'feat/reqmetahints' of https://github.com/getsentry/rela…
TBS1996 Feb 6, 2023
cf42d55
wip
TBS1996 Feb 6, 2023
b2e0211
wip
TBS1996 Feb 6, 2023
7255410
Merge branch 'master' into feat/reqmetahints
TBS1996 Feb 7, 2023
1419367
wip
TBS1996 Feb 7, 2023
dd8b949
wip
TBS1996 Feb 7, 2023
7c8f540
:wip
TBS1996 Feb 7, 2023
70e8dc6
wip
TBS1996 Feb 8, 2023
6b6d1f1
wip
TBS1996 Feb 8, 2023
6276a22
wip
TBS1996 Feb 8, 2023
d7667ea
wip
TBS1996 Feb 8, 2023
e5c654f
wip
TBS1996 Feb 8, 2023
4433cf3
wip
TBS1996 Feb 8, 2023
d156217
Merge branch 'master' into feat/reqmetahints
TBS1996 Feb 8, 2023
97fe2df
Update CHANGELOG.md
TBS1996 Feb 8, 2023
a0401e7
wip
TBS1996 Feb 8, 2023
6f5a012
Merge branch 'feat/reqmetahints' of https://github.com/getsentry/rela…
TBS1996 Feb 8, 2023
2cb0dd3
wip
TBS1996 Feb 9, 2023
8bab11d
wip
TBS1996 Feb 9, 2023
b075668
wip
TBS1996 Feb 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

**Features**:

- Add support for client hints. ([#1752](https://github.com/getsentry/relay/pull/1752))
- Use client hint headers instead of User-Agent when available. ([#1752](https://github.com/getsentry/relay/pull/1752), [#1802](https://github.com/getsentry/relay/pull/1802))
- Apply all configured data scrubbing rules on Replays. ([#1731](https://github.com/getsentry/relay/pull/1731))
- Add count transactions toward root project. ([#1734](https://github.com/getsentry/relay/pull/1734))
- Add or remove the profile ID on the transaction's profiling context. ([#1801](https://github.com/getsentry/relay/pull/1801))
Expand Down
6 changes: 5 additions & 1 deletion relay-cabi/src/processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use relay_general::store::{
light_normalize_event, GeoIpLookup, LightNormalizationConfig, StoreConfig, StoreProcessor,
};
use relay_general::types::{Annotated, Remark};
use relay_general::user_agent::RawUserAgentInfo;
use relay_sampling::{RuleCondition, SamplingConfig};

use crate::core::{RelayBuf, RelayStr};
Expand Down Expand Up @@ -112,7 +113,10 @@ pub unsafe extern "C" fn relay_store_normalizer_normalize_event(
let config = (*processor).config();
let light_normalization_config = LightNormalizationConfig {
client_ip: config.client_ip.as_ref(),
user_agent: config.user_agent.as_deref(),
user_agent: RawUserAgentInfo {
user_agent: config.user_agent.as_deref(),
client_hints: config.client_hints.as_deref(),
},
received_at: config.received_at,
max_secs_in_past: config.max_secs_in_past,
max_secs_in_future: config.max_secs_in_future,
Expand Down
2 changes: 2 additions & 0 deletions relay-general/benches/benchmarks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use relay_general::processor::{process_value, SelectorSpec};
use relay_general::protocol::{Event, IpAddr};
use relay_general::store::{StoreConfig, StoreProcessor};
use relay_general::types::Annotated;
use relay_general::user_agent::ClientHints;

fn load_all_fixtures() -> Vec<BenchmarkInput<String>> {
let mut rv = Vec::new();
Expand Down Expand Up @@ -93,6 +94,7 @@ fn bench_store_processor(c: &mut Criterion) {
breakdowns: None,
span_attributes: Default::default(),
client_sample_rate: None,
client_hints: ClientHints::default(),
};

let mut processor = StoreProcessor::new(config, None);
Expand Down
2 changes: 1 addition & 1 deletion relay-general/src/protocol/contexts/browser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl BrowserContext {
}

impl FromUserAgentInfo for BrowserContext {
fn from_client_hints(client_hints: &user_agent::ClientHints) -> Option<Self> {
fn from_client_hints(client_hints: &user_agent::ClientHints<&str>) -> Option<Self> {
let (browser, version) = browser_from_client_hints(client_hints.sec_ch_ua?)?;

Some(Self {
Expand Down
2 changes: 1 addition & 1 deletion relay-general/src/protocol/contexts/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ impl DeviceContext {
}

impl FromUserAgentInfo for DeviceContext {
fn from_client_hints(client_hints: &ClientHints) -> Option<Self> {
fn from_client_hints(client_hints: &ClientHints<&str>) -> Option<Self> {
let device = client_hints.sec_ch_ua_model?.to_owned();

if device.trim().is_empty() {
Expand Down
4 changes: 2 additions & 2 deletions relay-general/src/protocol/contexts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ impl Context {
/// With an automatically derived function which tries to first get the context from client hints,
/// if that fails it tries for the user agent string.
pub trait FromUserAgentInfo: Sized {
fn from_client_hints(client_hints: &ClientHints) -> Option<Self>;
fn from_client_hints(client_hints: &ClientHints<&str>) -> Option<Self>;
fn from_user_agent(user_agent: &str) -> Option<Self>;

fn from_hints_or_ua(raw_info: &RawUserAgentInfo) -> Option<Self> {
fn from_hints_or_ua(raw_info: &RawUserAgentInfo<&str>) -> Option<Self> {
Self::from_client_hints(&raw_info.client_hints)
.or_else(|| raw_info.user_agent.and_then(Self::from_user_agent))
}
Expand Down
2 changes: 1 addition & 1 deletion relay-general/src/protocol/contexts/os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl OsContext {
}

impl FromUserAgentInfo for OsContext {
fn from_client_hints(client_hints: &ClientHints) -> Option<Self> {
fn from_client_hints(client_hints: &ClientHints<&str>) -> Option<Self> {
let platform = client_hints.sec_ch_ua_platform?;
let version = client_hints.sec_ch_ua_platform_version?;

Expand Down
29 changes: 18 additions & 11 deletions relay-general/src/protocol/replay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,11 @@ impl Replay {
Ok(())
}

pub fn normalize(&mut self, client_ip: Option<RealIPAddr>, user_agent: Option<&str>) {
pub fn normalize(
&mut self,
client_ip: Option<RealIPAddr>,
user_agent: &RawUserAgentInfo<&str>,
) {
self.normalize_platform();
self.normalize_ip_address(client_ip);
self.normalize_user_agent(user_agent);
Expand All @@ -264,7 +268,7 @@ impl Replay {
}
}

fn normalize_user_agent(&mut self, default_user_agent: Option<&str>) {
fn normalize_user_agent(&mut self, default_user_agent: &RawUserAgentInfo<&str>) {
let headers = match self
.request
.value()
Expand All @@ -274,14 +278,16 @@ impl Replay {
None => return,
};

let mut user_agent_info = RawUserAgentInfo::from_headers(headers);
let user_agent_info = RawUserAgentInfo::from_headers(headers);

if user_agent_info.user_agent.is_none() {
user_agent_info.user_agent = default_user_agent;
}
let user_agent_info = if user_agent_info.is_empty() {
default_user_agent
} else {
&user_agent_info
};

let contexts = self.contexts.get_or_insert_with(|| Contexts::new());
user_agent::normalize_user_agent_info_generic(contexts, &self.platform, &user_agent_info);
user_agent::normalize_user_agent_info_generic(contexts, &self.platform, user_agent_info);
}

fn normalize_platform(&mut self) {
Expand Down Expand Up @@ -310,6 +316,7 @@ mod tests {
};
use crate::testutils::get_value;
use crate::types::Annotated;
use crate::user_agent::RawUserAgentInfo;
use chrono::{TimeZone, Utc};
use std::net::{IpAddr, Ipv4Addr};

Expand Down Expand Up @@ -414,7 +421,7 @@ mod tests {

let mut replay: Annotated<Replay> = Annotated::from_json(payload).unwrap();
let replay_value = replay.value_mut().as_mut().unwrap();
replay_value.normalize(None, None);
replay_value.normalize(None, &RawUserAgentInfo::default());

let loaded_browser_context = replay_value
.contexts
Expand Down Expand Up @@ -448,7 +455,7 @@ mod tests {

let mut replay: Annotated<Replay> = Annotated::from_json(payload).unwrap();
let replay_value = replay.value_mut().as_mut().unwrap();
replay_value.normalize(None, None);
replay_value.normalize(None, &RawUserAgentInfo::default());

let user = replay_value.user.value();
assert!(user.is_none());
Expand All @@ -464,7 +471,7 @@ mod tests {

let mut replay: Annotated<Replay> = Annotated::from_json(payload).unwrap();
let replay_value = replay.value_mut().as_mut().unwrap();
replay_value.normalize(Some(ip_address), None);
replay_value.normalize(Some(ip_address), &RawUserAgentInfo::default());

let ipaddr = replay_value
.user
Expand All @@ -484,7 +491,7 @@ mod tests {

let mut replay: Annotated<Replay> = Annotated::from_json(payload).unwrap();
let replay_value = replay.value_mut().as_mut().unwrap();
replay_value.normalize(None, None);
replay_value.normalize(None, &RawUserAgentInfo::default());

let user = replay_value.user.value().unwrap();
assert!(user.ip_address.value().unwrap().as_str() == "127.1.1.1");
Expand Down
2 changes: 2 additions & 0 deletions relay-general/src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use serde_json::Value;
use crate::processor::{ProcessingState, Processor};
use crate::protocol::{Event, IpAddr};
use crate::types::{Meta, ProcessingResult, SpanAttribute};
use crate::user_agent::ClientHints;

mod clock_drift;
mod event_error;
Expand Down Expand Up @@ -38,6 +39,7 @@ pub struct StoreConfig {
pub protocol_version: Option<String>,
pub grouping_config: Option<Value>,
pub user_agent: Option<String>,
pub client_hints: ClientHints<String>,
pub received_at: Option<DateTime<Utc>>,
pub sent_at: Option<DateTime<Utc>>,

Expand Down
84 changes: 66 additions & 18 deletions relay-general/src/store/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ use super::{schema, transactions, BreakdownsConfig, TransactionNameRule};
use crate::processor::{MaxChars, ProcessValue, ProcessingState, Processor};
use crate::protocol::{
self, AsPair, Breadcrumb, ClientSdkInfo, Context, Contexts, DebugImage, Event, EventId,
EventType, Exception, Frame, HeaderName, HeaderValue, Headers, IpAddr, Level, LogEntry,
Measurement, Measurements, Request, SpanStatus, Stacktrace, Tags, TraceContext, User,
VALID_PLATFORMS,
EventType, Exception, Frame, Headers, IpAddr, Level, LogEntry, Measurement, Measurements,
Request, SpanStatus, Stacktrace, Tags, TraceContext, User, VALID_PLATFORMS,
};
use crate::store::{ClockDriftProcessor, GeoIpLookup, StoreConfig};
use crate::types::{
Annotated, Empty, Error, ErrorKind, FromValue, Meta, Object, ProcessingAction,
ProcessingResult, Value,
};
use crate::user_agent::RawUserAgentInfo;

pub mod breakdowns;
mod contexts;
Expand Down Expand Up @@ -568,7 +568,7 @@ fn is_security_report(event: &Event) -> bool {
fn normalize_security_report(
event: &mut Event,
client_ip: Option<&IpAddr>,
user_agent: Option<&str>,
user_agent: &RawUserAgentInfo<&str>,
) {
if !is_security_report(event) {
// This event is not a security report, exit here.
Expand All @@ -582,23 +582,16 @@ fn normalize_security_report(
user.ip_address = Annotated::new(client_ip.to_owned());
}

if let Some(client) = user_agent {
let request = event
if !user_agent.is_empty() {
let headers = event
.request
.value_mut()
.get_or_insert_with(Request::default);

let headers = request
.get_or_insert_with(Request::default)
.headers
.value_mut()
.get_or_insert_with(Headers::default);

if !headers.contains("User-Agent") {
headers.insert(
HeaderName::new("User-Agent"),
Annotated::new(HeaderValue::new(client)),
);
}
user_agent.populate_event_headers(headers);
}
}

Expand Down Expand Up @@ -669,7 +662,7 @@ fn normalize_logentry(logentry: &mut Annotated<LogEntry>, _meta: &mut Meta) -> P
#[derive(Default, Debug)]
pub struct LightNormalizationConfig<'a> {
pub client_ip: Option<&'a IpAddr>,
pub user_agent: Option<&'a str>,
pub user_agent: RawUserAgentInfo<&'a str>,
pub received_at: Option<DateTime<Utc>>,
pub max_secs_in_past: Option<i64>,
pub max_secs_in_future: Option<i64>,
Expand Down Expand Up @@ -704,7 +697,7 @@ pub fn light_normalize_event(
schema::SchemaProcessor.process_event(event, meta, ProcessingState::root())?;

// Process security reports first to ensure all props.
normalize_security_report(event, config.client_ip, config.user_agent);
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);
Expand Down Expand Up @@ -1002,11 +995,12 @@ mod tests {

use crate::processor::process_value;
use crate::protocol::{
ContextInner, DebugMeta, Frame, Geo, LenientString, LogEntry, PairList, RawStacktrace,
ContextInner, Csp, DebugMeta, Frame, Geo, LenientString, LogEntry, PairList, RawStacktrace,
Span, SpanId, TagEntry, TraceId, Values,
};
use crate::testutils::{get_path, get_value};
use crate::types::{FromValue, SerializableAnnotated};
use crate::user_agent::ClientHints;

use super::*;

Expand Down Expand Up @@ -2436,4 +2430,58 @@ mod tests {
)
"###);
}

#[test]
fn test_normalize_security_report() {
let mut event = Event {
csp: Annotated::from(Csp::default()),
..Default::default()
};
let ipaddr = IpAddr("213.164.1.114".to_string());

let client_ip = Some(&ipaddr);
let user_agent = RawUserAgentInfo::new_test_dummy();

// This call should fill the event headers with info from the user_agent which is
// tested below.
normalize_security_report(&mut event, client_ip, &user_agent);

let headers = event
.request
.value_mut()
.get_or_insert_with(Request::default)
.headers
.value_mut()
.get_or_insert_with(Headers::default);

assert_eq!(
event.user.value().unwrap().ip_address,
Annotated::from(ipaddr)
);
assert_eq!(
headers.get_header(RawUserAgentInfo::USER_AGENT),
user_agent.user_agent
);
assert_eq!(
headers.get_header(ClientHints::SEC_CH_UA),
user_agent.client_hints.sec_ch_ua,
);
assert_eq!(
headers.get_header(ClientHints::SEC_CH_UA_MODEL),
user_agent.client_hints.sec_ch_ua_model,
);
assert_eq!(
headers.get_header(ClientHints::SEC_CH_UA_PLATFORM),
user_agent.client_hints.sec_ch_ua_platform,
);
assert_eq!(
headers.get_header(ClientHints::SEC_CH_UA_PLATFORM_VERSION),
user_agent.client_hints.sec_ch_ua_platform_version,
);

assert!(
std::mem::size_of_val(&ClientHints::<&str>::default()) == 64,
"If you add new fields, update the test accordingly"
);
}
}
2 changes: 1 addition & 1 deletion relay-general/src/store/normalize/user_agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub fn normalize_user_agent(event: &mut Event) {
pub fn normalize_user_agent_info_generic(
contexts: &mut Contexts,
platform: &Annotated<String>,
user_agent_info: &RawUserAgentInfo,
user_agent_info: &RawUserAgentInfo<&str>,
) {
if !contexts.contains_key(BrowserContext::default_key()) {
if let Some(browser_context) = BrowserContext::from_hints_or_ua(user_agent_info) {
Expand Down
Loading