-
-
Notifications
You must be signed in to change notification settings - Fork 221
Added StringStackTraceFactory #4362
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
Conversation
public partial class StringStackTraceFactory : ISentryStackTraceFactory | ||
{ | ||
private readonly SentryOptions _options; | ||
private const string FullStackTraceLinePattern = @"at (?<Module>[^\.]+)\.(?<Function>.*?) in (?<FileName>.*?):line (?<LineNo>\d+)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this regex coming from another file in the repo or it's a new regex? curious how tested/vetted this is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a new regex and the short answer is "not very"... which is one reason this is marked as Experimental.
We do have these unit tests but I'm not sure what this string is likely to look like in all the different real world scenarios.
This one (the fallback) is the regex that @filipnavara is using here:
private const string StackTraceLinePattern = @"at (.+)\.(.+) \+"; |
I'm hoping that between the two of those we can extract something sensible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have been using this regex in production since October 2024. It may not be fool-proof but it's somewhat battle-tested.
@@ -2,6 +2,10 @@ | |||
|
|||
## Unreleased | |||
|
|||
### Features | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it on by default on AOT then?
How do I turn it on?
Would be nice to have a mention of that in the changelog too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps something we could also document in Troubleshooting
when released?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah when @Flash0ver and I discussed we were thinking we wouldn't promote this too widely, since it's quite experimental. However there are some additional docs on SentryOptions.UseStackTraceFactory
(which is how you'd add this):
sentry-dotnet/src/Sentry/SentryOptions.cs
Lines 1637 to 1643 in e4120a9
/// <para> | |
/// By default, Sentry uses the <see cref="SentryStackTraceFactory"/> to create stack traces and this implementation | |
/// offers the most comprehensive functionality. However, full stack traces are not available in AOT compiled | |
/// applications. If you are compiling your applications AOT and the stack traces that you see in Sentry are not | |
/// informative enough, you could consider using the StringStackTraceFactory instead. This is not as functional but | |
/// is guaranteed to provide at least _something_ useful in AOT compiled applications. | |
/// </para> |
And we figured we'd add something to our troubleshooting docs as well...
I'll add a blurb in the description of this PR as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some notes on changelog otherwise lgtm
Resolves #3439
Description and Usage
This PR introduces a new stack trace factory that SDK users can enable explicitly for AOT compiled applications only if they're not getting useful stack traces from the default
SentryStackTraceFactory
.It can be enabled via the
SentryOptions
when initialising Sentry:If this proves useful enough, we can simplify how that gets wired up.
Initial testing
I tested this out with the sample code from the related issue and the resulting stack trace is worse than what you get from the
SentryStackTraceFactory
.For folks who aren't getting any stack trace at all, this might be an improvement... although I'm unable to reproduce a scenario where I don't have any stack trace.
Examples
SentryStackTraceFactory
SimpleStackTraceFactory