Skip to content
Merged
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
6 changes: 3 additions & 3 deletions ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ without subtle parent-resolution surprises).
| | What | Status today |
|---|------|--------------|
| 7 | ~~`FoundationsSpanContext` metadata cache eviction~~ — **shipped** as `SpanContext::on_snapshot` hook; foundations / tracing / local-scope adapters all clear their caches automatically at end of snapshot. |
| 8 | Strip capture-machinery frames in the **encoder**, not just the CLI display logic, so stock `pprof -text` shows real user code at the leaf instead of `backtrace::trace` | not started |
| 8 | ~~Strip capture-machinery frames in the **encoder**, not just the CLI display logic, so stock `pprof -text` shows real user code at the leaf instead of `backtrace::trace`~~ — **shipped** | |
| 9 | Customisable pprof label keys (currently hardcoded `"span_id"` / `"span_name"`) | not started |
| 10 | Load `culpert::Config` from foundations' `TelemetrySettings` so it's one config tree, not two | not started |
| 11 | `pprof_route` allocates a `BoxFuture` per request — cheap on the telemetry server but worth pooling for very high request rates | not started |
Expand All @@ -191,8 +191,8 @@ without subtle parent-resolution surprises).
corrected unbiased `bytes_total`; CLI dropped to two columns.
7. ~~**Metadata cache eviction**~~ — **shipped** as
`SpanContext::on_snapshot`.
8. Then opportunistically: hierarchical diff polish, JSON diff output,
async `#[culpert::span_fn]`, encoder-side machinery-frame stripping.
8. ~~**Encoder-side machinery-frame stripping**~~ — **shipped.**
9. Then opportunistically: hierarchical diff polish, JSON diff output.

The v0.2 marquee is fully in: hierarchy, diff, broader-ecosystem reach
(`tracing`), sampling-independent attribution, FP-based capture, and
Expand Down
38 changes: 1 addition & 37 deletions culpert-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
mod archive;

use clap::{Parser, Subcommand};
use culpert::pprof::{self, proto};
use culpert::pprof::{self, is_machinery, proto};
use std::collections::HashMap;
use std::path::PathBuf;

Expand Down Expand Up @@ -783,42 +783,6 @@ fn aggregate_callsites(profile: &proto::Profile, filter: &Filter) -> Vec<Callsit
}

/// Names matching any of these prefixes are stack-capture machinery (our
/// own observe/sampler path, backtrace internals, the rust allocator
/// shim) and should be skipped when picking a "leaf" callsite to show
/// the user. The first frame that isn't machinery is the real one.
/// Substrings (not just prefixes) that mark stack-capture / allocator
/// shim frames. We want to skip these and stop on the first frame that's
/// real user / framework code.
///
/// Rust 1.95+ mangles symbols as `<crate>[<build_hash>]::<path>` so we
/// match both the old (`alloc::raw_vec::...`) and new (`]::raw_vec::...`)
/// forms.
const MACHINERY_NEEDLES: &[&str] = &[
"backtrace::",
"culpert::sampler",
"culpert::stack_capture",
"culpert::allocator",
"<culpert::allocator::TrackingAllocator",
"__rust_alloc",
"__rust_realloc",
"__rust_alloc_zeroed",
"__rustc[",
// Old-form (no build-hash bracket).
"alloc::alloc::alloc",
"alloc::alloc::Global",
"alloc::alloc::realloc",
"alloc::raw_vec::",
// New-form (post-bracket).
"]::alloc::alloc",
"]::alloc::Global",
"]::alloc::realloc",
"]::raw_vec::",
];

fn is_machinery(name: &str) -> bool {
MACHINERY_NEEDLES.iter().any(|n| name.contains(n))
}

fn format_leaf_callsite(
sample: &proto::Sample,
profile: &proto::Profile,
Expand Down
128 changes: 128 additions & 0 deletions culpert/src/pprof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,38 @@ use crate::Profile;
use prost::Message;
use std::collections::HashMap;

/// Frame name substrings that identify culpert's own capture machinery —
/// `backtrace::trace`, the allocator shim, Rust's alloc glue, etc.
///
/// Rust 1.95+ mangles symbols as `<crate>[<build_hash>]::<path>`, so we
/// match both the old (`alloc::raw_vec::...`) and new (`]::raw_vec::...`)
/// forms.
pub const MACHINERY_NEEDLES: &[&str] = &[
"backtrace::",
"culpert::sampler",
"culpert::stack_capture",
"culpert::allocator",
"<culpert::allocator::TrackingAllocator",
"__rust_alloc",
"__rust_realloc",
"__rust_alloc_zeroed",
"__rustc[",
// Old-form (no build-hash bracket).
"alloc::alloc::alloc",
"alloc::alloc::Global",
"alloc::alloc::realloc",
"alloc::raw_vec::",
// New-form (post-bracket).
"]::alloc::alloc",
"]::alloc::Global",
"]::alloc::realloc",
"]::raw_vec::",
];

pub fn is_machinery(name: &str) -> bool {
MACHINERY_NEEDLES.iter().any(|n| name.contains(n))
}

/// pprof protobuf types, hand-written from `profile.proto`.
///
/// We carry the messages directly (with `#[derive(prost::Message)]`) rather
Expand Down Expand Up @@ -318,6 +350,11 @@ fn build_proto(profile: &Profile) -> proto::Profile {
for entry in &profile.entries {
let mut location_ids = Vec::with_capacity(entry.frames.len());
for frame in &entry.frames {
// Strip capture-machinery frames so stock `go tool pprof` /
// `pprof -text` shows real user code at the leaf.
if frame.name.as_deref().is_some_and(is_machinery) {
continue;
}
let loc_id = match locs.get(&frame.ip) {
Some(&id) => id,
None => {
Expand Down Expand Up @@ -701,6 +738,97 @@ mod tests {
assert_eq!(recovered, md);
}

#[test]
fn machinery_frames_stripped_from_encoder() {
let mut profile = fake_profile();
// Prepend machinery frames to the first entry's stack.
profile.entries[0].frames.insert(
0,
Frame {
ip: 0xdead,
name: Some("backtrace::capture::Backtrace::new_unresolved".into()),
filename: Some("backtrace/src/capture.rs".into()),
lineno: Some(1),
},
);
profile.entries[0].frames.insert(
0,
Frame {
ip: 0xbeef,
name: Some("culpert::sampler::on_alloc".into()),
filename: None,
lineno: None,
},
);

let bytes = encode(&profile);
let decoded = proto::Profile::decode(&bytes[..]).expect("decode");

// No location should carry a machinery function name.
for loc in &decoded.location {
for line in &loc.line {
let func = decoded
.function
.iter()
.find(|f| f.id == line.function_id)
.unwrap();
let name = &decoded.string_table[func.name as usize];
assert!(
!is_machinery(name),
"machinery frame leaked into encoded profile: {name}"
);
}
}

// The non-machinery user frames are still present.
let all_names: Vec<&str> = decoded
.function
.iter()
.map(|f| decoded.string_table[f.name as usize].as_str())
.collect();
assert!(all_names.contains(&"render_template"));
assert!(all_names.contains(&"build_response"));
}

#[test]
fn all_machinery_stack_emits_empty_location_list() {
// A sample whose entire stack is machinery should still appear in the
// output (its bytes count) but with an empty location_id list rather
// than disappearing entirely.
let profile = Profile {
entries: vec![crate::ProfileEntry {
span: None,
frames: vec![
Frame {
ip: 0x1,
name: Some("alloc::alloc::alloc".into()),
filename: None,
lineno: None,
},
Frame {
ip: 0x2,
name: Some("culpert::allocator::on_alloc".into()),
filename: None,
lineno: None,
},
],
bytes_total: 64,
samples: 1,
}],
spans: std::collections::HashMap::new(),
dropped_samples: 0,
config: crate::config::Config::default(),
};

let bytes = encode(&profile);
let decoded = proto::Profile::decode(&bytes[..]).expect("decode");

assert_eq!(decoded.sample.len(), 1);
assert!(decoded.sample[0].location_id.is_empty());
assert_eq!(decoded.sample[0].value, vec![1, 64]);
assert_eq!(decoded.location.len(), 0);
}

#[test]
fn metadata_skips_malformed_comments() {
// Hand-craft a proto::Profile whose `comment` field includes both
Expand Down
Loading