Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
150 changes: 148 additions & 2 deletions relay-event-normalization/src/eap/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use chrono::{DateTime, Utc};
use relay_common::time::UnixTimestamp;
use relay_conventions::attributes::*;
use relay_conventions::{AttributeInfo, WriteBehavior};
use relay_event_schema::protocol::{AttributeType, Attributes, BrowserContext, Geo};
use relay_protocol::{Annotated, ErrorKind, Meta, Remark, RemarkType, Value};
use relay_event_schema::protocol::{Attribute, AttributeType, Attributes, BrowserContext, Geo};
use relay_protocol::{Annotated, Error, ErrorKind, Meta, Remark, RemarkType, Value};
use relay_sampling::DynamicSamplingContext;
use relay_spans::derive_op_for_v2_span;

Expand Down Expand Up @@ -387,6 +387,35 @@ pub fn normalize_dsc(
}
}

/// Normalizes the client sample rate attribute to be in the range `[0, 1]`.
Comment thread
Dav1dde marked this conversation as resolved.
Outdated
Comment thread
Dav1dde marked this conversation as resolved.
Outdated
///
/// This is only relevant for spans as other eap types re not sampled.
pub fn normalize_client_sample_rate(attributes: &mut Annotated<Attributes>) {
let Some(attributes) = attributes.value_mut() else {
return;
};

// This is fine if normalizations like this stay one-offs. If at some point we end up with more
// of these structural validations or normalizations based on attributes, they should be
// outsourced to conventions and enforced with a dedicated processor.
fn normalize_sample_rate(sr: &Annotated<Attribute>) -> Option<Annotated<Attribute>> {
match sr.value()?.value.value.value()?.as_f64() {
Some(v) if v > 0.0 && v <= 1.0 => None,
// This is an invalid sample rate, either by type or value.
_ => Some(Annotated::from_error(
Error::expected("sample rate > 0.0, <= 1.0"),
None,
)),
}
}

if let Some(sr) = attributes.0.get_mut(SENTRY__CLIENT_SAMPLE_RATE)
&& let Some(new_sr) = normalize_sample_rate(sr)
{
*sr = new_sr;
}
}

/// Normalizes deprecated attributes according to `sentry-conventions`.
///
/// Attributes with a status of `"normalize"` will be moved to their replacement name.
Expand Down Expand Up @@ -2525,4 +2554,121 @@ mod tests {
}
"#);
}

#[test]
fn test_normalize_client_sample_rate_valid() {
let mut attributes = Annotated::from_json(
r#"{
"sentry.client_sample_rate": {
"type": "double",
"value": 1.0
}
}"#,
)
.unwrap();

normalize_client_sample_rate(&mut attributes);

assert_annotated_snapshot!(attributes, @r#"
{
"sentry.client_sample_rate": {
"type": "double",
"value": 1.0
}
}
"#);
}

#[test]
fn test_normalize_client_sample_rate_invalid_too_small() {
let mut attributes = {
let mut attrs = Attributes::new();
attrs.insert(SENTRY__CLIENT_SAMPLE_RATE, 0.0);
Annotated::new(attrs)
};

normalize_client_sample_rate(&mut attributes);

assert_annotated_snapshot!(attributes, @r#"
{
"sentry.client_sample_rate": null,
"_meta": {
"sentry.client_sample_rate": {
"": {
"err": [
[
"invalid_data",
{
"reason": "expected sample rate > 0.0, <= 1.0"
}
]
]
}
}
}
}
"#);
}

#[test]
fn test_normalize_client_sample_rate_invalid_too_large() {
let mut attributes = {
let mut attrs = Attributes::new();
attrs.insert(SENTRY__CLIENT_SAMPLE_RATE, 1.1);
Annotated::new(attrs)
};

normalize_client_sample_rate(&mut attributes);

assert_annotated_snapshot!(attributes, @r#"
{
"sentry.client_sample_rate": null,
"_meta": {
"sentry.client_sample_rate": {
"": {
"err": [
[
"invalid_data",
{
"reason": "expected sample rate > 0.0, <= 1.0"
}
]
]
}
}
}
}
"#);
}

#[test]
fn test_normalize_client_sample_rate_invalid_type() {
let mut attributes = {
let mut attrs = Attributes::new();
attrs.insert(SENTRY__CLIENT_SAMPLE_RATE, "foobar");
Annotated::new(attrs)
};

normalize_client_sample_rate(&mut attributes);

assert_annotated_snapshot!(attributes, @r#"
{
"sentry.client_sample_rate": null,
"_meta": {
"sentry.client_sample_rate": {
"": {
"err": [
[
"invalid_data",
{
"reason": "expected sample rate > 0.0, <= 1.0"
}
]
]
}
}
}
}
"#);
}
}
1 change: 1 addition & 0 deletions relay-server/src/processing/spans/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ fn normalize_span(
relay_event_normalization::normalize_performance_score(span, performance_score);
eap::normalize_attribute_values(&mut span.attributes, allowed_hosts);
eap::write_legacy_attributes(&mut span.attributes);
eap::normalize_client_sample_rate(&mut span.attributes);
};

// Set a max_bytes value on the root state if it's defined in the project config.
Expand Down
5 changes: 4 additions & 1 deletion relay-server/src/processing/spans/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,8 @@ fn inject_server_sample_rate(
};

let attributes = attributes.get_or_insert_with(Default::default);
attributes.insert("sentry.server_sample_rate", server_sample_rate.to_f64());
attributes.insert(
"sentry.server_sample_rate",
server_sample_rate.to_f64().clamp(1e-9, 1.0),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since measurements conversion is at the front of my brain right now: should the same thing happen during v1 -> v2 span conversion?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it would be good if we could normalize in all places, it's just a bit of a pain.

);
}
Loading