Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Add `gen_ai.tool.call.result.size` to AI tool spans, measuring serialized results in UTF-8 bytes before Relay truncation and preserving SDK-provided sizes. ([#6534](https://github.com/getsentry/relay/pull/6534))
- Derive the `object` span category from `object.*` span operations, such as Cloudflare R2 and Amazon S3 object storage calls. ([#6514](https://github.com/getsentry/relay/pull/6514))
- Process `event.flamegraph` attachments associated with events. ([#6473](https://github.com/getsentry/relay/pull/6473))
- Derive representative exception stack traces from flame graph attachments on the first Relay in the chain, weighting sampled call paths by frequency, depth, and application frames. ([#6520](https://github.com/getsentry/relay/pull/6520))

**Bug Fixes**:

Expand Down
276 changes: 272 additions & 4 deletions relay-profiling/src/flamegraph.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! Validation of version 1 [flame graph attachments].
//! Validation and representative call-path selection for version 1 [flame graph attachments].
//!
//! [flame graph attachments]: https://develop.sentry.dev/sdk/telemetry/attachments/#flame-graph

use std::collections::HashSet;
use std::collections::{HashMap, HashSet};
Comment thread
sentry-warden[bot] marked this conversation as resolved.

use bytes::Bytes;
use relay_event_schema::protocol::{DebugId, VALID_PLATFORMS};
Expand Down Expand Up @@ -51,6 +51,10 @@ pub enum Error {
SampleCount(u64),
#[error("children exceed parent sample count")]
ChildSampleCount,
#[error("cyclic call path")]
CyclicCallPath,
#[error("call-path traversal limit exceeded")]
TraversalLimit,
}

#[derive(Deserialize, Serialize)]
Expand Down Expand Up @@ -83,8 +87,8 @@ struct Tree {
roots: Vec<usize>,
#[serde(rename = "thread_id", skip_serializing_if = "Option::is_none")]
_thread_id: Option<String>,
#[serde(rename = "thread_attributed", skip_serializing_if = "Option::is_none")]
_thread_attributed: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
thread_attributed: Option<bool>,
}

#[derive(Deserialize, Serialize)]
Expand Down Expand Up @@ -186,11 +190,271 @@ fn parse(payload: &[u8]) -> Result<Graph, Error> {
Ok(graph)
}

/// Identify a representative caller-to-callee path, expressed as frame-table indices.
///
/// Like the `AnrCulpritIdentifier` in the Android SDK, this scores identical root-to-frame prefixes by
/// `sample_count * (depth + application_frames)`. Node counts are inclusive, so they already
/// include the observations ending in descendants. Explicit `in_app: true` supplies the app boost.
///
/// Sample populations from different trees are never added together. Prefer trees attributed to
/// the diagnostic when present; otherwise choose the strongest path across all trees. Ties retain
/// the first path in input order. The attachment itself remains unchanged. Graphs whose expanded
/// paths exceed a traversal budget linear in nodes, child edges, and roots are rejected.
pub fn identify_culprit(payload: &[u8]) -> Result<Vec<usize>, Error> {
let graph = parse(payload)?;

// A producer may emit duplicate frame-table entries. Intern their known frame data once,
// rather than copying or hashing entire paths at every node.
let mut frames = HashMap::new();
let mut frame_ids = Vec::with_capacity(graph.frames.len());
for (index, frame) in graph.frames.iter().enumerate() {
let key = (&frame.frame, frame.addr_mode.as_deref().unwrap_or("abs"));
frame_ids.push(*frames.entry(key).or_insert(index));
}

struct Prefix {
parent: Option<usize>,
frame_id: usize,
depth: usize,
app_frames: usize,
samples: u128,
}

let has_attributed_tree = graph
.trees
.iter()
.any(|tree| tree.thread_attributed == Some(true));
// Shared nodes and repeated edges can expand exponentially when materialized as paths.
// Bound total visits across all trees by the size of the serialized graph structure.
let mut remaining_visits = graph.nodes.len()
+ graph
.nodes
.iter()
.map(|node| node.children.len())
.sum::<usize>()
+ graph
.trees
.iter()
.map(|tree| tree.roots.len())
.sum::<usize>();
let mut best_score = 0;
let mut best_path = Vec::new();
for tree in &graph.trees {
if has_attributed_tree && tree.thread_attributed != Some(true) {
continue;
}
remaining_visits = remaining_visits
.checked_sub(tree.roots.len())
.ok_or(Error::TraversalLimit)?;
let mut prefixes = HashMap::new();
let mut candidates: Vec<Prefix> = Vec::new();
let mut pending: Vec<_> = tree.roots.iter().rev().map(|node| (node, None)).collect();
while let Some((node_index, parent)) = pending.pop() {
let node = graph.node(*node_index)?;
let frame_id = frame_ids[node.frame_id];
let prefix = *prefixes.entry((parent, frame_id)).or_insert_with(|| {
let (depth, app_frames) = parent
.map(|parent: usize| {
let prefix = &candidates[parent];
(prefix.depth, prefix.app_frames)
})
.unwrap_or_default();
let index = candidates.len();
candidates.push(Prefix {
parent,
frame_id,
depth: depth + 1,
app_frames: app_frames
+ usize::from(graph.frames[frame_id].frame.in_app == Some(true)),
samples: 0,
});
index
});
if candidates[prefix].depth > graph.nodes.len() {
return Err(Error::CyclicCallPath);
}
candidates[prefix].samples += u128::from(node.sample_count);
remaining_visits = remaining_visits
.checked_sub(node.children.len())
.ok_or(Error::TraversalLimit)?;
pending.extend(
node.children
.iter()
.rev()
.map(|child| (child, Some(prefix))),
);
}

// Integer scores retain differences near the maximum safe JSON sample count. The input
// byte limit bounds the number of nodes and path depth, so u128 also bounds aggregation.
let mut best_prefix = None;
for (index, prefix) in candidates.iter().enumerate() {
let score = prefix.samples * (prefix.depth + prefix.app_frames) as u128;
if score > best_score {
best_score = score;
best_prefix = Some(index);
}
}
if let Some(index) = best_prefix {
let mut path = Vec::new();
let mut current = Some(index);
while let Some(index) = current {
let prefix = &candidates[index];
path.push(prefix.frame_id);
current = prefix.parent;
}
path.reverse();
best_path = path;
}
}
Ok(best_path)
}

#[cfg(test)]
mod tests {
use super::*;
use serde_json::json;

fn identify(graph: serde_json::Value) -> Result<Vec<usize>, Error> {
identify_culprit(&serde_json::to_vec(&graph).unwrap())
}

fn culprit_graph() -> serde_json::Value {
json!({
"version": "1", "platform": "cocoa",
"frames": [
{"instruction_addr": "0x1000", "in_app": false},
{"instruction_addr": "0x2000", "in_app": true},
{"instruction_addr": "0x3000", "in_app": false},
{"instruction_addr": "0x4000", "in_app": false}
],
"nodes": [
{"frame_id": 0, "sample_count": 100, "children": [1]},
{"frame_id": 1, "sample_count": 100, "children": [2, 3]},
{"frame_id": 2, "sample_count": 60},
{"frame_id": 3, "sample_count": 40}
],
"trees": [{"roots": [0]}]
})
}

#[test]
fn test_culprit_shared_application_prefix() {
// Shared caller: 100 * (2 + 1) = 300, versus 60 * (3 + 1) = 240.
assert_eq!(identify(culprit_graph()).unwrap(), vec![0, 1]);
}

#[test]
fn test_culprit_duplicate_prefixes_and_frame_entries() {
let mut graph = culprit_graph();
let duplicate = graph["frames"][0].clone();
graph["frames"].as_array_mut().unwrap().push(duplicate);
graph["nodes"] = json!([
{"frame_id": 0, "sample_count": 60, "children": [1]},
{"frame_id": 1, "sample_count": 60, "children": [2]},
{"frame_id": 2, "sample_count": 60},
{"frame_id": 4, "sample_count": 40, "children": [4]},
{"frame_id": 1, "sample_count": 40, "children": [5]},
{"frame_id": 3, "sample_count": 40}
]);
graph["trees"][0]["roots"] = json!([0, 3]);
assert_eq!(identify(graph).unwrap(), vec![0, 1]);
}

#[test]
fn test_culprit_application_weight_and_stable_ties() {
let mut graph = culprit_graph();
graph["nodes"] = json!([
{"frame_id": 2, "sample_count": 10},
{"frame_id": 1, "sample_count": 10},
{"frame_id": 3, "sample_count": 10}
]);
graph["trees"][0]["roots"] = json!([0, 1, 2]);
assert_eq!(identify(graph.clone()).unwrap(), vec![1]);
graph["frames"][1]["in_app"] = json!(false);
assert_eq!(identify(graph.clone()).unwrap(), vec![2]);
graph["frames"][1].as_object_mut().unwrap().remove("in_app");
assert_eq!(identify(graph).unwrap(), vec![2]);
}

#[test]
fn test_culprit_inclusive_counts_and_recursion() {
let mut graph = culprit_graph();
graph["nodes"] = json!([
{"frame_id": 1, "sample_count": 100, "children": [1]},
{"frame_id": 1, "sample_count": 60, "children": [2]},
{"frame_id": 1, "sample_count": 30}
]);
// Inclusive counts must not be counted again from descendant samples.
assert_eq!(identify(graph).unwrap(), vec![1, 1]);
}

#[test]
fn test_culprit_does_not_merge_independent_tree_populations() {
let mut graph = culprit_graph();
graph["nodes"] = json!([
{"frame_id": 0, "sample_count": 10},
{"frame_id": 0, "sample_count": 10},
{"frame_id": 1, "sample_count": 9}
]);
graph["trees"] = json!([
{"roots": [0]}, {"roots": [1]}, {"roots": [2]}
]);
assert_eq!(identify(graph.clone()).unwrap(), vec![1]);
graph["trees"][0]["thread_attributed"] = json!(true);
assert_eq!(identify(graph).unwrap(), vec![0]);
}

#[test]
fn test_culprit_exact_large_sample_counts() {
let mut graph = culprit_graph();
graph["frames"][0]["in_app"] = json!(true);
graph["nodes"] = json!([
{"frame_id": 0, "sample_count": MAX_SAMPLE_COUNT - 1},
{"frame_id": 1, "sample_count": MAX_SAMPLE_COUNT}
]);
graph["trees"][0]["roots"] = json!([0, 1]);
assert_eq!(identify(graph).unwrap(), vec![1]);
}

#[test]
fn test_culprit_rejects_invalid_graph() {
let mut graph = culprit_graph();
graph["nodes"][0]["sample_count"] = json!(1);
assert!(identify(graph).is_err());
assert!(identify(json!({"version": "2"})).is_err());
}

#[test]
fn test_culprit_bounds_repeated_child_edges() {
let depth = 12;
let mut graph = culprit_graph();
graph["nodes"] = (0..depth)
.map(|index| {
json!({
"frame_id": 0,
"sample_count": 1_u64 << (depth - index - 1),
"children": if index + 1 < depth {
vec![index + 1, index + 1]
} else {
vec![]
}
})
})
.collect();
validate(&serde_json::to_vec(&graph).unwrap()).unwrap();
assert!(matches!(identify(graph), Err(Error::TraversalLimit)));
}

#[test]
fn test_culprit_rejects_cyclic_call_path() {

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.

@romtsn as you brought up cyclic frames in #6473 (comment) here's one case where we need to think about it. The other case will be in backend, but I wouldn't trust input data in any case, so while early validation might be useful I would do it in each consumer instead.

let mut graph = culprit_graph();
graph["nodes"] = json!([
{"frame_id": 0, "sample_count": 1, "children": [0]}
]);
assert!(matches!(identify(graph), Err(Error::CyclicCallPath)));
}

#[test]
fn test_flamegraph_deep_call_tree() {
let depth = 1000;
Expand All @@ -210,6 +474,10 @@ mod tests {
"trees": [{"roots": [0]}]
});
validate(&serde_json::to_vec(&graph).unwrap()).unwrap();
assert_eq!(
identify_culprit(&serde_json::to_vec(&graph).unwrap()).unwrap(),
vec![0; depth]
);

graph["nodes"][depth - 1]["sample_count"] = json!(0);
assert!(matches!(
Expand Down
2 changes: 1 addition & 1 deletion relay-profiling/src/sample/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub enum Version {
///
/// Each object should contain **at least** a `filename`, `function` or `instruction_addr`
/// attribute. All values are optional, but recommended.
#[derive(Clone, Debug, Default, Deserialize, Serialize)]
#[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq, Eq, Hash)]
pub struct Frame {
/// Absolute path to the source file.
#[serde(skip_serializing_if = "Option::is_none")]
Expand Down
2 changes: 1 addition & 1 deletion relay-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ sentry = { workspace = true, features = ["tower-http"] }
sentry_protos = { workspace = true, optional = true }
serde = { workspace = true }
serde_bytes = { workspace = true }
serde_json = { workspace = true }
serde_json = { workspace = true, features = ["raw_value"] }
serde_path_to_error = { workspace = true }
serde_urlencoded = { workspace = true }
smallvec = { workspace = true, features = ["drain_filter"] }
Expand Down
Loading
Loading