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(metrics): Add ability to compute metrics summary and attach it to events #3769

Merged
merged 43 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
a80a492
feat(metrics): Add ability to compute metrics summary and attach it t…
iambriccardo Jun 27, 2024
c9b52fb
Fix
iambriccardo Jun 27, 2024
464a751
Fix
iambriccardo Jun 27, 2024
d226897
Fix
iambriccardo Jun 27, 2024
f71d90b
Fix
iambriccardo Jun 27, 2024
6a8128d
Fix
iambriccardo Jun 28, 2024
7cfbc2f
Fix
iambriccardo Jun 28, 2024
62d94e4
Fix
iambriccardo Jun 28, 2024
5f2d15a
Add changelog
iambriccardo Jun 28, 2024
bb484f4
Add integration tests
iambriccardo Jun 28, 2024
5afe699
Add integration tests
iambriccardo Jun 28, 2024
fa58eff
Extract only for custom
iambriccardo Jul 1, 2024
5f425e5
Improve
iambriccardo Jul 1, 2024
4a31446
Improve
iambriccardo Jul 1, 2024
8da0ad6
Improve
iambriccardo Jul 1, 2024
eb731ab
Improve
iambriccardo Jul 1, 2024
2bb9503
Improve
iambriccardo Jul 1, 2024
608461a
Improve
iambriccardo Jul 1, 2024
310a48e
Improve
iambriccardo Jul 1, 2024
3735f2c
Improve
iambriccardo Jul 1, 2024
e56b435
Improve
iambriccardo Jul 1, 2024
ee4d7c4
Improve
iambriccardo Jul 1, 2024
d5f3e15
Improve
iambriccardo Jul 1, 2024
e317492
Improve
iambriccardo Jul 1, 2024
f7ee62b
Improve
iambriccardo Jul 2, 2024
bb8a118
Improve
iambriccardo Jul 2, 2024
40a9a08
Improve
iambriccardo Jul 2, 2024
f13dd17
Improve
iambriccardo Jul 2, 2024
5871729
Improve
iambriccardo Jul 2, 2024
b9bdb7b
Improve
iambriccardo Jul 2, 2024
0537d4a
Improve
iambriccardo Jul 2, 2024
ff94535
Improve
iambriccardo Jul 2, 2024
7f25bea
Improve
iambriccardo Jul 2, 2024
d001732
Improve
iambriccardo Jul 2, 2024
50b0980
Improve
iambriccardo Jul 2, 2024
219380f
Improve
iambriccardo Jul 2, 2024
2e83923
Improve
iambriccardo Jul 2, 2024
070f837
Improve
iambriccardo Jul 2, 2024
923186d
Improve
iambriccardo Jul 3, 2024
19086b3
Improve
iambriccardo Jul 3, 2024
17e6bfe
Improve
iambriccardo Jul 3, 2024
e29c252
Fix
iambriccardo Jul 3, 2024
40d3949
Merge
iambriccardo Jul 4, 2024
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
30 changes: 30 additions & 0 deletions relay-event-schema/src/protocol/metrics_summary.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#[cfg(feature = "jsonschema")]
use relay_jsonschema_derive::JsonSchema;
use relay_protocol::{Annotated, Array, Empty, FromValue, IntoValue, Object};
use std::collections::BTreeMap;

use crate::processor::ProcessValue;

Expand All @@ -12,10 +13,39 @@ pub type MetricSummaryMapping = Object<Array<MetricSummary>>;
pub struct MetricsSummary(pub MetricSummaryMapping);

impl MetricsSummary {
/// Returns an empty [`MetricsSummary`].
pub fn empty() -> MetricsSummary {
MetricsSummary(BTreeMap::new())
}

/// Combinator to modify the contained metric summaries.
pub fn update_value<F: FnOnce(MetricSummaryMapping) -> MetricSummaryMapping>(&mut self, f: F) {
self.0 = f(std::mem::take(&mut self.0));
}

/// Merges a [`MetricsSummary`] into itself.
///
/// The merge operation concatenates tags of the same metric even if they are identical for the
/// sake of simplicity. This doesn't cause issues for us, under the assumption that the two
/// summaries do not have overlapping metric names (which is the case for us since summaries
/// computed by the SDKs have metric names that are different from the ones we generate in
/// Relay).
///
/// Do not use this function to merge any arbitrary [`MetricsSummary`]s.
Copy link
Member Author

Choose a reason for hiding this comment

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

I am not happy about this comment, open for suggestions.

pub fn merge(&mut self, metrics_summary: MetricsSummary) {
for (metric_name, metrics) in metrics_summary.0 {
let original_metrics = self.0.entry(metric_name).or_insert(Annotated::new(vec![]));
match (original_metrics.value_mut(), metrics.0) {
(Some(original_metrics), Some(metrics)) => {
original_metrics.extend(metrics);
}
(None, Some(metrics)) => {
original_metrics.set_value(Some(metrics));
}
_ => {}
}
}
}
}

/// The metric summary of a single metric that is emitted by the SDK.
Expand Down
155 changes: 141 additions & 14 deletions relay-server/src/metrics_extraction/event.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use relay_common::time::UnixTimestamp;
use relay_dynamic_config::CombinedMetricExtractionConfig;
use relay_event_schema::protocol::{Event, Span};
use relay_event_schema::protocol::{Event, MetricsSummary, Span};
use relay_metrics::Bucket;
use relay_quotas::DataCategory;

use crate::metrics_extraction::generic::{self, Extractable};
use crate::metrics_extraction::metrics_summary;
use crate::services::processor::extract_transaction_span;
use crate::statsd::RelayTimers;
use crate::utils::sample;
Expand Down Expand Up @@ -46,7 +47,7 @@ impl Extractable for Span {
///
/// If this is a transaction event with spans, metrics will also be extracted from the spans.
pub fn extract_metrics(
event: &Event,
event: &mut Event,
spans_extracted: bool,
config: CombinedMetricExtractionConfig<'_>,
max_tag_value_size: usize,
Expand All @@ -64,20 +65,33 @@ pub fn extract_metrics(
}

fn extract_span_metrics_for_event(
event: &Event,
event: &mut Event,
config: CombinedMetricExtractionConfig<'_>,
max_tag_value_size: usize,
output: &mut Vec<Bucket>,
) {
relay_statsd::metric!(timer(RelayTimers::EventProcessingSpanMetricsExtraction), {
if let Some(transaction_span) = extract_transaction_span(event, max_tag_value_size) {
output.extend(generic::extract_metrics(&transaction_span, config));
let metrics = generic::extract_metrics(&transaction_span, config);
if let Some(metrics_summary) = metrics_summary::compute(&metrics) {
event
._metrics_summary
.get_or_insert_with(MetricsSummary::empty)
.merge(metrics_summary);
}
output.extend(metrics);
}

if let Some(spans) = event.spans.value() {
if let Some(spans) = event.spans.value_mut() {
for annotated_span in spans {
if let Some(span) = annotated_span.value() {
output.extend(generic::extract_metrics(span, config));
if let Some(span) = annotated_span.value_mut() {
let metrics = generic::extract_metrics(span, config);
if let Some(metrics_summary) = metrics_summary::compute(&metrics) {
span._metrics_summary
.get_or_insert_with(MetricsSummary::empty)
.merge(metrics_summary);
}
output.extend(metrics);
}
}
}
Expand Down Expand Up @@ -1181,7 +1195,7 @@ mod tests {
);

extract_metrics(
event.value().unwrap(),
event.value_mut().as_mut().unwrap(),
false,
combined_config(features).combined(),
200,
Expand Down Expand Up @@ -1383,7 +1397,7 @@ mod tests {
);

let metrics = extract_metrics(
event.value().unwrap(),
event.value_mut().as_mut().unwrap(),
false,
combined_config([Feature::ExtractCommonSpanMetricsFromEvent]).combined(),
200,
Expand Down Expand Up @@ -1437,10 +1451,10 @@ mod tests {
]
}
"#;
let event = Annotated::from_json(json).unwrap();
let mut event = Annotated::from_json(json).unwrap();

let metrics = extract_metrics(
event.value().unwrap(),
event.value_mut().as_mut().unwrap(),
false,
combined_config([Feature::ExtractCommonSpanMetricsFromEvent]).combined(),
200,
Expand Down Expand Up @@ -1472,7 +1486,7 @@ mod tests {
);

let metrics = extract_metrics(
event.value().unwrap(),
event.value_mut().as_mut().unwrap(),
false,
combined_config([Feature::ExtractCommonSpanMetricsFromEvent]).combined(),
200,
Expand Down Expand Up @@ -1692,10 +1706,10 @@ mod tests {
}
}
"#;
let event = Annotated::<Event>::from_json(event).unwrap();
let mut event = Annotated::<Event>::from_json(event).unwrap();

let metrics = extract_metrics(
event.value().unwrap(),
event.value_mut().as_mut().unwrap(),
false,
combined_config([Feature::ExtractCommonSpanMetricsFromEvent]).combined(),
200,
Expand All @@ -1722,4 +1736,117 @@ mod tests {

assert_eq!(&*metrics[3].name, "d:spans/duration@millisecond");
}

#[test]
fn test_metrics_summaries_on_transaction_and_spans() {
let mut event = Annotated::from_json(
r#"
{
"type": "transaction",
"sdk": {"name": "sentry.javascript.react-native"},
"start_timestamp": "2021-04-26T07:59:01+0100",
"timestamp": "2021-04-26T08:00:00+0100",
"release": "1.2.3",
"transaction": "gEt /api/:version/users/",
"transaction_info": {"source": "custom"},
"platform": "cocoa",
"contexts": {
"trace": {
"trace_id": "ff62a8b040f340bda5d830223def1d81",
"span_id": "bd429c44b67a3eb4",
"op": "ui.load"
},
"device": {
"family": "iOS",
"model": "iPhone1,1"
},
"app": {
"app_identifier": "org.reactjs.native.example.RnDiffApp",
"app_name": "RnDiffApp"
},
"os": {
"name": "iOS",
"version": "16.2"
}
},
"measurements": {
"app_start_warm": {
"value": 1.0,
"unit": "millisecond"
}
},
"spans": [
{
"op": "ui.load.initial_display",
"span_id": "bd429c44b67a3eb2",
"start_timestamp": 1597976300.0000000,
"timestamp": 1597976303.0000000,
"trace_id": "ff62a8b040f340bda5d830223def1d81",
"data": {
"frames.slow": 1,
"frames.frozen": 2,
"frames.total": 9,
"frames.delay": 0.1
},
"_metrics_summary": {
"d:spans/duration@millisecond": [
{
"min": 50.0,
"max": 60.0,
"sum": 100.0,
"count": 2,
"tags": {
"app_start_type": "warm",
"device.class": "1"
}
}
]
}
}
],
"_metrics_summary": {
"d:spans/duration@millisecond": [
{
"min": 50.0,
"max": 100.0,
"sum": 150.0,
"count": 2,
"tags": {
"app_start_type": "warm",
"device.class": "1"
}
}
]
}
}
"#,
)
.unwrap();

// Normalize first, to make sure that all things are correct as in the real pipeline:
normalize_event(
&mut event,
&NormalizationConfig {
enrich_spans: true,
device_class_synthesis_config: true,
..Default::default()
},
);

let _ = extract_metrics(
event.value_mut().as_mut().unwrap(),
false,
combined_config([Feature::ExtractCommonSpanMetricsFromEvent]).combined(),
200,
None,
);

insta::assert_debug_snapshot!(&event.value().unwrap()._metrics_summary);
insta::assert_debug_snapshot!(
&event.value().unwrap().spans.value().unwrap()[0]
.value()
.unwrap()
._metrics_summary
);
}
}
Loading
Loading