Skip to content

Commit

Permalink
wip: Store sample rates instead of extrapolating
Browse files Browse the repository at this point in the history
  • Loading branch information
jan-auer committed Aug 1, 2024
1 parent 72b5d37 commit fef1dc1
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 2 deletions.
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ num-traits = "0.2.18"
num_cpus = "1.13.0"
once_cell = "1.13.1"
opentelemetry-proto = { git = "https://github.com/open-telemetry/opentelemetry-rust", rev = "dd4c13bd69ca4b24d5a8f21024a466fbb35cdd14" }
ordered-float = "4.2.2"
parking_lot = "0.12.1"
path-slash = "0.2.1"
pest = "2.1.3"
Expand Down
8 changes: 8 additions & 0 deletions relay-dynamic-config/src/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,14 @@ pub struct Options {
)]
pub extrapolation_duplication_limit: usize,

#[serde(
default,
rename = "sentry-metrics.extrapolation.propagate-rates",
deserialize_with = "default_on_error",
skip_serializing_if = "is_default"
)]
pub extrapolation_propagate_rates: bool,

/// All other unknown options.
#[serde(flatten)]
other: HashMap<String, Value>,
Expand Down
1 change: 1 addition & 0 deletions relay-metrics/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ smallvec = { workspace = true }
thiserror = { workspace = true }
tokio = { workspace = true, features = ["time"] }
unescaper = { workspace = true }
ordered-float = { workspace = true }

[dev-dependencies]
criterion = { workspace = true }
Expand Down
13 changes: 13 additions & 0 deletions relay-metrics/src/aggregator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::time::Duration;
use std::{fmt, mem};

use fnv::FnvHasher;
use ordered_float::OrderedFloat;
use priority_queue::PriorityQueue;
use relay_base_schema::project::ProjectKey;
use relay_common::time::UnixTimestamp;
Expand Down Expand Up @@ -67,6 +68,7 @@ struct BucketKey {
metric_name: MetricName,
tags: BTreeMap<String, String>,
extracted_from_indexed: bool,
sample_rate: Option<OrderedFloat<f64>>,
}

impl BucketKey {
Expand Down Expand Up @@ -712,6 +714,7 @@ impl Aggregator {
metric_name: bucket.name,
tags: bucket.tags,
extracted_from_indexed: bucket.metadata.extracted_from_indexed,
sample_rate: bucket.metadata.sample_rate.map(OrderedFloat),
};
let key = validate_bucket_key(key, &self.config)?;

Expand Down Expand Up @@ -1029,6 +1032,7 @@ mod tests {
("answer".to_owned(), "42".to_owned()),
]),
extracted_from_indexed: false,
sample_rate: None,
};
assert_eq!(
bucket_key.cost(),
Expand Down Expand Up @@ -1213,6 +1217,7 @@ mod tests {
metric_name: "c:transactions/foo@none".into(),
tags: BTreeMap::new(),
extracted_from_indexed: false,
sample_rate: None,
};
let fixed_cost = bucket_key.cost() + mem::size_of::<BucketValue>();
for (metric_name, metric_value, expected_added_cost) in [
Expand Down Expand Up @@ -1448,6 +1453,7 @@ mod tests {
tags
},
extracted_from_indexed: false,
sample_rate: None,
};

let mut bucket_key = validate_bucket_key(bucket_key, &test_config()).unwrap();
Expand All @@ -1474,6 +1480,7 @@ mod tests {
metric_name: "c:transactions/a_short_metric".into(),
tags: BTreeMap::new(),
extracted_from_indexed: false,
sample_rate: None,
};
assert!(validate_bucket_key(short_metric, &test_config()).is_ok());

Expand All @@ -1483,6 +1490,7 @@ mod tests {
metric_name: "c:transactions/long_name_a_very_long_name_its_super_long_really_but_like_super_long_probably_the_longest_name_youve_seen_and_even_the_longest_name_ever_its_extremly_long_i_cant_tell_how_long_it_is_because_i_dont_have_that_many_fingers_thus_i_cant_count_the_many_characters_this_long_name_is".into(),
tags: BTreeMap::new(),
extracted_from_indexed: false,
sample_rate: None,
};
let validation = validate_bucket_key(long_metric, &test_config());

Expand All @@ -1499,6 +1507,7 @@ mod tests {
metric_name: "c:transactions/a_short_metric_with_long_tag_key".into(),
tags: BTreeMap::from([("i_run_out_of_creativity_so_here_we_go_Lorem_Ipsum_is_simply_dummy_text_of_the_printing_and_typesetting_industry_Lorem_Ipsum_has_been_the_industrys_standard_dummy_text_ever_since_the_1500s_when_an_unknown_printer_took_a_galley_of_type_and_scrambled_it_to_make_a_type_specimen_book".into(), "tag_value".into())]),
extracted_from_indexed: false,
sample_rate: None,
};
let validation = validate_bucket_key(short_metric_long_tag_key, &test_config()).unwrap();
assert_eq!(validation.tags.len(), 0);
Expand All @@ -1509,6 +1518,7 @@ mod tests {
metric_name: "c:transactions/a_short_metric_with_long_tag_value".into(),
tags: BTreeMap::from([("tag_key".into(), "i_run_out_of_creativity_so_here_we_go_Lorem_Ipsum_is_simply_dummy_text_of_the_printing_and_typesetting_industry_Lorem_Ipsum_has_been_the_industrys_standard_dummy_text_ever_since_the_1500s_when_an_unknown_printer_took_a_galley_of_type_and_scrambled_it_to_make_a_type_specimen_book".into())]),
extracted_from_indexed: false,
sample_rate: None,
};
let validation = validate_bucket_key(short_metric_long_tag_value, &test_config()).unwrap();
assert_eq!(validation.tags.len(), 0);
Expand All @@ -1527,6 +1537,7 @@ mod tests {
metric_name: "c:transactions/a_short_metric".into(),
tags: BTreeMap::from([("foo".into(), tag_value.clone())]),
extracted_from_indexed: false,
sample_rate: None,
};
let validated_bucket = validate_metric_tags(short_metric, &test_config());
assert_eq!(validated_bucket.tags["foo"], tag_value);
Expand Down Expand Up @@ -1665,6 +1676,7 @@ mod tests {
metric_name: "c:transactions/foo".into(),
tags: BTreeMap::new(),
extracted_from_indexed: false,
sample_rate: None,
};

// Second bucket has a timestamp in this hour.
Expand All @@ -1675,6 +1687,7 @@ mod tests {
metric_name: "c:transactions/foo".into(),
tags: BTreeMap::new(),
extracted_from_indexed: false,
sample_rate: None,
};

let flush_time_1 = get_flush_time(&config, reference_time, &bucket_key_1);
Expand Down
11 changes: 11 additions & 0 deletions relay-metrics/src/bucket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,8 @@ pub struct BucketMetadata {
/// received in the outermost internal Relay.
pub received_at: Option<UnixTimestamp>,

pub sample_rate: Option<f64>,

/// Is `true` if this metric was extracted from a sampled/indexed envelope item.
///
/// The final dynamic sampling decision is always made in processing Relays.
Expand All @@ -781,6 +783,7 @@ impl BucketMetadata {
merges: 1,
received_at: Some(received_at),
extracted_from_indexed: false,
sample_rate: None,
}
}

Expand All @@ -798,6 +801,10 @@ impl BucketMetadata {
(left, right) => left.min(right),
};
}

pub fn set_sample_rate(&mut self, sample_rate: f64) {
self.sample_rate = Some(sample_rate.clamp(0.0, 1.0));
}
}

impl Default for BucketMetadata {
Expand All @@ -806,6 +813,7 @@ impl Default for BucketMetadata {
merges: 1,
received_at: None,
extracted_from_indexed: false,
sample_rate: None,
}
}
}
Expand Down Expand Up @@ -1482,6 +1490,7 @@ mod tests {
merges: 2,
received_at: None,
extracted_from_indexed: false,
sample_rate: None,
}
);

Expand All @@ -1493,6 +1502,7 @@ mod tests {
merges: 3,
received_at: Some(UnixTimestamp::from_secs(10)),
extracted_from_indexed: false,
sample_rate: None,
}
);

Expand All @@ -1504,6 +1514,7 @@ mod tests {
merges: 4,
received_at: Some(UnixTimestamp::from_secs(10)),
extracted_from_indexed: false,
sample_rate: None,
}
);
}
Expand Down

0 comments on commit fef1dc1

Please sign in to comment.