Add symbol based backtrace filtering

[MilesConn]

I was really excited to use rootcause but I was working on a project where crates were vendored with a build system that wasn't cargo and I had other non rust code in my backtraces. It seemed that I couldn't get backtrace filtering to work for either of those things.

I changed the backtrace filtering logic to now filter based on demangled symbol name instead of paths. This allows filtering to work with vendored crates or other scenarios where the crates aren't located in .registry.

As a fallback, I also added a new skipped_path_patterns field that allows filtering based on path as well.

Testing

In a demo project I had given this full backtrace

 ● A
 ├ src/main.rs:71
 ╰ Backtrace
   │ trace                - /Users/***/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/backtrace-0.3.76/src/backtrace/libunwind.rs:117
   │ trace_unsynchronized - /Users/***/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/backtrace-0.3.76/src/backtrace/mod.rs:66
   │ trace                - /Users/***/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/backtrace-0.3.76/src/backtrace/mod.rs:53
   │ capture              - /Users/***/Projects/rootcause/rootcause-backtrace/src/lib.rs:763
   │ on_sendsync_creation - /Users/***/Projects/rootcause/rootcause-backtrace/src/lib.rs:686
   │ on_sendsync_creation - /Users/***/Projects/rootcause/src/hooks/report_creation.rs:289
   │ call                 - /Users/***/Projects/rootcause/src/hooks/report_creation.rs:506
   │ use_hooks            - /Users/***/Projects/rootcause/src/hooks/mod.rs:872
   │ run_creation_hooks_sendsync
   │    - /Users/***/Projects/rootcause/src/hooks/report_creation.rs:517
   │ run_creation_hooks   - /Users/***/Projects/rootcause/src/markers.rs:659
   │ from_parts           - /Users/***/Projects/rootcause/src/report/owned.rs:313
   │ new_custom           - /Users/***/Projects/rootcause/src/report/owned.rs:283
   │ new                  - /Users/***/Projects/rootcause/src/report/owned.rs:255
   │ from                 - /Users/***/Projects/rootcause/src/report/owned.rs:1753
   │ from_residual        - /Users/***/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/result.rs:2184
   │ foo                  - /Users/***/Projects/playground/src/main.rs:71
   │ main                 - /Users/***/Projects/playground/src/main.rs:195
   │ call_once            - /Users/***/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250
   │ rust_begin_short_backtrace
   │    - /Users/***/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/backtrace.rs:158
   │ {{closure}}          - /Users/***/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:206
   │ call_once            - /rustc/ded5c06cf21d2b93bffd5d884aa6e96934ee4234/library/core/src/ops/function.rs:287
   │ do_call              - /rustc/ded5c06cf21d2b93bffd5d884aa6e96934ee4234/library/std/src/panicking.rs:590
   │ catch_unwind         - /rustc/ded5c06cf21d2b93bffd5d884aa6e96934ee4234/library/std/src/panicking.rs:553
   │ catch_unwind         - /rustc/ded5c06cf21d2b93bffd5d884aa6e96934ee4234/library/std/src/panic.rs:359
   │ {{closure}}          - /rustc/ded5c06cf21d2b93bffd5d884aa6e96934ee4234/library/std/src/rt.rs:175
   │ do_call              - /rustc/ded5c06cf21d2b93bffd5d884aa6e96934ee4234/library/std/src/panicking.rs:590
   │ catch_unwind         - /rustc/ded5c06cf21d2b93bffd5d884aa6e96934ee4234/library/std/src/panicking.rs:553
   │ catch_unwind         - /rustc/ded5c06cf21d2b93bffd5d884aa6e96934ee4234/library/std/src/panic.rs:359
   │ lang_start_internal  - /rustc/ded5c06cf21d2b93bffd5d884aa6e96934ee4234/library/std/src/rt.rs:171
   │ lang_start           - /Users/***/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:205
   ╰─

with this filter

BacktraceFilter {
                skipped_initial_crates: &["backtrace"],
                skipped_middle_crates: &["rootcause", "rootcause-backtrace"],
                skipped_final_crates: &[],
                max_entry_count: usize::MAX,
                show_full_path: true,
                skipped_path_patterns: &["rustc"],
            },

I now get

 ● A
 ├ src/main.rs:71
 ╰ Backtrace
   │ capture       - /Users/***/Projects/rootcause/rootcause-backtrace/src/lib.rs:763
   │ ... omitted 10 frame(s) from crate 'rootcause' ...
   │ from_residual - /Users/***/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/result.rs:2184
   │ foo           - /Users/***/Projects/playground/src/main.rs:71
   │ main          - /Users/***/Projects/playground/src/main.rs:195
   │ call_once     - /Users/***/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250
   │ rust_begin_short_backtrace
   │    - /Users/***/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys/backtrace.rs:158
   │ {{closure}}   - /Users/***/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:206
   │ lang_start    - /Users/***/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:205
   │ note: 23 frame(s) omitted. For a complete backtrace, set RUST_BACKTRACE=full.
   ╰─

I also made another small change that RUST_BACKTRACE=full now ignores all filtering. Anyways, open to feedback and thanks for the project!

[TethysSvensson]

Can you be a bit more specific about what didn't work for you, what you tried (and failed) to do and how this fixes it?

As far as I remember from the last time I looked at this code, it's hard to get consistent results using only the symbol names, specially when using debug = "line-tables".

[MilesConn]

Can you be a bit more specific about what didn't work for you, what you tried (and failed) to do and how this fixes it?

Sure thing. I think there's a few problems I'm trying to solve that are all different.

1. I'm using vendored crates which means I never have a path like .cargo/registry/src/... or /rustc/{hash}/library/. My paths instead are third-party/... or whatever my vendor location is. There's no guarantee it's just third-party/.

2. My testing framework isn't rust. Because of this, in my backtraces I have frames from non rust code I'd like to filter out. You can imagine this is a problem for all foreign code.

3. Finally, I'm not using cargo to build I'm using buck which makes Location::caller()); not work as dependably. This problem I think is more ultra specific to me so it's not the main problem to fix.

What I like about the symbol based approach is I think it handles 1 and 3 together. Alternatively, the API could allow users to provide additional paths to search for ie third-party/... OR .cargo/registry/src or custom_path/.... I just chose the symbol based approach because it seemed like it was more in line with the original API which is, specify some crates and they get filtered out.

Finally, something that does work is if I map my vendored crates to match what the current cargo registry regex expects. For instance, say I have a vendored crate called dummy

RUSTFLAGS="--remap-path-prefix=/Users/***/Projects/dummy/=/Users/***/.cargo/registry/src/fake-0000000000000000/dummy-0.1.0/" cargo run

this works in that dummy now gets filtered out. However, because the current regex expects a hash and a version in the path, you have to map to this fake directory. While this works, it doesn't seem very scalable.

The skipped_path_patterns solves issue 2 and is more a general catchall solution.

As far as I remember from the last time I looked at this code, it's hard to get consistent results using only the symbol names, specially when using debug = "line-tables".

I knew there was going to be cases I'd miss. Seems like line-tables-only does in fact break my approach unfortunately.

[TethysSvensson]

I don't think I can accept this as is, if it doesn't work with line-tables-only. Are you interested in improving the PR to support this case as well? Maybe by making this new behavior optional?

I am wondering though if this might be slightly too specific to your use case? Would you consider this more or less useful to the average user than the current approach?

Rootcause has been designed to make it easy to make your own extensions or customize existing ones. If this indeed specific to your use case, then you can still inline the backtrace collector into your own code change the behavior like that.

[MilesConn]

I don't think I can accept this as is, if it doesn't work with line-tables-only. Are you interested in improving the PR to support this case as well? Maybe by making this new behavior optional?

I can definitely improve it. One thing could be we just keep the current behavior and add the skipped_path_patterns field. Alternatively, we allow users to override the base regex so instead of .cargo/registry they can specify the path and then we use the same path based filtering. Any opinions?

Also thanks so much for your time.

[MilesConn]

Alright I updated it to just work with paths and supports "line-tables-only". Let me know what you think.

edit: Had a fmt issue whoopsies. Do you also want me to bump semver version?

[TethysSvensson]

Sorry for leaving this hanging for so long. I know having a PR open for this long is frustrating, but unfortunately I haven't been able to work on rootcause for the last few months and I am only now catching back up.

After re-reading this issue, I would like to start by saying that I think the problems you've mentioned are very valid. As I see it, there are three connected problems here:

• Problem 1: It would be nice with better support for vendored crates. This seems like an obvious place where we could do better
• Problem 2: It would be nice with better support for filtering non-Rust frames. This also seems like something where we could definitely improve.
• Problem 3: It would be nice with better support for buck. I am not sure whether this is something we will be able to do, especially since I don't understand the problem at hand, but if it's something we can easily fix in rootcause, then I don't see why we shouldn't.

I think reviving this PR to fix some of these issues would definitely be worthwhile. As I understand this PR, it contains a few different changes bundled together:

• Change 1: The crate_root_paths feature
• Change 2: The skipped_path_patterns feature
• Change 3: A mechanism for crate name normalization with regards to dashes and underscores.
• Change 4: Moving the environment variable capture directly into Backtrace::capture
• Change 5: The RUST_SRC/CARGO -> CRATE collapse and crate_name field removal
• Change 6: A significant refactoring of the frame filtering internals

To my eyes, Change 1 looks like a useful improvement on Problem 1. Similarly Change 2 looks like a useful improvement on Problem 2.

I am a bit more ambivalent on Change 3. This is partially because I don't understand the motivation for it. Is it a prerequisite for some of the other changes?

Change 4 makes capturing non-deterministic and I don't think I am very likely to accept that.

I don't think I am likely to accept Change 5 without a good motivation. I think the current features are useful, and I prefer not removing them.

I am uncertain about Change 6, partially because I don't quite understand it. It might be useful on its own or it might be prerequisites for some of the other changes? Right now I don't think I have the context to be able to accept this, but I am very open to learning more about it.

I know it's been a long time since you opened this PR and I am grateful for your patience on this. If you still have motivation to work on this, then I think the next step would be to isolate one of the changes you've made and make a PR for that one change.

Note that a rebase would be necessary in any case as #175 and #177 made changes to this code as well. I am planning to release version 0.13 in a few days and if you are fast I would be okay with waiting for you to land some or all of these. Otherwise we can add them as part of the 1.0 release.

If you no longer have the motivation or time to work on this, then I completely understand that as well. In that case, I think we should close this PR. If you want, feel free open a feature request with the features you would like to see implemented.

[MilesConn]

No worries and I totally understand maintenance can be time consuming. I really appreciate the feedback. Sometime this week I'll see if I can address these points and clean it up along with a rebase.

Change 6: A significant refactoring of the frame filtering internals

I think I was trying to simplify how frame filtering is done but I'll go back to the drawing board on it. Thanks for the review!