-
-
Notifications
You must be signed in to change notification settings - Fork 226
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
Changes from all commits
2dff37f
02e0a99
d9dd0aa
d52b2ad
9796a17
703c19f
357ad20
9a83f63
4e47a58
16a54ee
400f5ba
da2f702
0223cf9
45b3258
e4120a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,106 @@ | ||||
| using Sentry.Infrastructure; | ||||
|
|
||||
| namespace Sentry.Extensibility; | ||||
|
|
||||
| #if NET8_0_OR_GREATER | ||||
|
|
||||
| /// <summary> | ||||
| /// A rudimentary implementation of <see cref="ISentryStackTraceFactory"/> that simply parses the | ||||
| /// string representation of the stack trace from an exception. This lacks many of the features | ||||
| /// off the full <see cref="SentryStackTraceFactory"/>. However, it may be useful in AOT compiled | ||||
| /// applications where the full factory is not returning a useful stack trace. | ||||
| /// <remarks> | ||||
| /// <para> | ||||
| /// This class is currently EXPERIMENTAL | ||||
| /// </para> | ||||
| /// <para> | ||||
| /// This factory is designed for AOT scenarios, so only available for net8.0+ | ||||
| /// </para> | ||||
| /// </remarks> | ||||
| /// </summary> | ||||
| [Experimental(DiagnosticId.ExperimentalFeature)] | ||||
| public partial class StringStackTraceFactory : ISentryStackTraceFactory | ||||
| { | ||||
| private readonly SentryOptions _options; | ||||
| private const string FullStackTraceLinePattern = @"at (?<Module>[^\.]+)\.(?<Function>.*?) in (?<FileName>.*?):line (?<LineNo>\d+)"; | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
I'm hoping that between the two of those we can extract something sensible.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||||
| private const string StackTraceLinePattern = @"at (.+)\.(.+) \+"; | ||||
|
|
||||
| #if NET9_0_OR_GREATER | ||||
| [GeneratedRegex(FullStackTraceLinePattern)] | ||||
| internal static partial Regex FullStackTraceLine { get; } | ||||
| #else | ||||
| internal static readonly Regex FullStackTraceLine = FullStackTraceLineRegex(); | ||||
|
|
||||
| [GeneratedRegex(FullStackTraceLinePattern)] | ||||
| private static partial Regex FullStackTraceLineRegex(); | ||||
| #endif | ||||
|
|
||||
| #if NET9_0_OR_GREATER | ||||
| [GeneratedRegex(StackTraceLinePattern)] | ||||
| private static partial Regex StackTraceLine { get; } | ||||
| #else | ||||
| private static readonly Regex StackTraceLine = StackTraceLineRegex(); | ||||
|
|
||||
| [GeneratedRegex(StackTraceLinePattern)] | ||||
| private static partial Regex StackTraceLineRegex(); | ||||
| #endif | ||||
|
|
||||
| /// <summary> | ||||
| /// Creates a new instance of <see cref="StringStackTraceFactory"/>. | ||||
| /// </summary> | ||||
| /// <param name="options">The sentry options</param> | ||||
| public StringStackTraceFactory(SentryOptions options) | ||||
| { | ||||
| _options = options; | ||||
| } | ||||
|
|
||||
| /// <inheritdoc /> | ||||
| public SentryStackTrace? Create(Exception? exception = null) | ||||
| { | ||||
| _options.LogDebug("Source Stack Trace: {0}", exception?.StackTrace); | ||||
|
|
||||
| var trace = new SentryStackTrace(); | ||||
| var frames = new List<SentryStackFrame>(); | ||||
|
|
||||
| var lines = exception?.StackTrace?.Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries) ?? []; | ||||
| foreach (var line in lines) | ||||
| { | ||||
| var fullMatch = FullStackTraceLine.Match(line); | ||||
| if (fullMatch.Success) | ||||
| { | ||||
| frames.Add(new SentryStackFrame() | ||||
| { | ||||
| Module = fullMatch.Groups[1].Value, | ||||
| Function = fullMatch.Groups[2].Value, | ||||
| FileName = fullMatch.Groups[3].Value, | ||||
| LineNumber = int.Parse(fullMatch.Groups[4].Value), | ||||
| }); | ||||
| continue; | ||||
| } | ||||
|
|
||||
| _options.LogDebug("Full stack frame match failed for: {0}", line); | ||||
| var lineMatch = StackTraceLine.Match(line); | ||||
| if (lineMatch.Success) | ||||
| { | ||||
| frames.Add(new SentryStackFrame() | ||||
| { | ||||
| Module = lineMatch.Groups[1].Value, | ||||
| Function = lineMatch.Groups[2].Value | ||||
| }); | ||||
| continue; | ||||
| } | ||||
|
|
||||
| _options.LogDebug("Stack frame match failed for: {0}", line); | ||||
| frames.Add(new SentryStackFrame() | ||||
| { | ||||
| Function = line | ||||
| }); | ||||
| } | ||||
|
|
||||
| trace.Frames = frames; | ||||
| _options.LogDebug("Created {0} with {1} frames.", "StringStackTrace", trace.Frames.Count); | ||||
| return trace.Frames.Count != 0 ? trace : null; | ||||
| } | ||||
| } | ||||
|
|
||||
| #endif | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| { | ||
| FileName: {ProjectDirectory}Internals/StringStackTraceFactoryTests.cs, | ||
| Function: Tests.Internals.StringStackTraceFactoryTests.GenericMethodThatThrows[T](T value), | ||
| Module: Other | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| // ReSharper disable once CheckNamespace | ||
| // Stack trace filters out Sentry frames by namespace | ||
| namespace Other.Tests.Internals; | ||
|
|
||
| #if PLATFORM_NEUTRAL && NET8_0_OR_GREATER | ||
|
|
||
| public class StringStackTraceFactoryTests | ||
| { | ||
| [Fact] | ||
| public Task MethodGeneric() | ||
| { | ||
| // Arrange | ||
| const int i = 5; | ||
| var exception = Record.Exception(() => GenericMethodThatThrows(i)); | ||
|
|
||
| var options = new SentryOptions | ||
| { | ||
| AttachStacktrace = true | ||
| }; | ||
| var factory = new StringStackTraceFactory(options); | ||
|
|
||
| // Act | ||
| var stackTrace = factory.Create(exception); | ||
|
|
||
| // Assert; | ||
| var frame = stackTrace!.Frames.Single(x => x.Function!.Contains("GenericMethodThatThrows")); | ||
| return Verify(frame) | ||
| .IgnoreMembers<SentryStackFrame>( | ||
| x => x.Package, | ||
| x => x.LineNumber, | ||
| x => x.ColumnNumber, | ||
| x => x.InstructionAddress, | ||
| x => x.FunctionId) | ||
| .AddScrubber(x => x.Replace(@"\", @"/")); | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.NoInlining)] | ||
| private static void GenericMethodThatThrows<T>(T value) => | ||
| throw new Exception(); | ||
|
|
||
| [Theory] | ||
| [InlineData("at MyNamespace.MyClass.MyMethod in /path/to/file.cs:line 42", "MyNamespace", "MyClass.MyMethod", "/path/to/file.cs", "42")] | ||
| [InlineData("at Foo.Bar.Baz in C:\\code\\foo.cs:line 123", "Foo", "Bar.Baz", "C:\\code\\foo.cs", "123")] | ||
| public void FullStackTraceLine_ValidInput_Matches( | ||
| string input, string expectedModule, string expectedFunction, string expectedFile, string expectedLine) | ||
| { | ||
| var match = StringStackTraceFactory.FullStackTraceLine.Match(input); | ||
| Assert.True(match.Success); | ||
| Assert.Equal(expectedModule, match.Groups["Module"].Value); | ||
| Assert.Equal(expectedFunction, match.Groups["Function"].Value); | ||
| Assert.Equal(expectedFile, match.Groups["FileName"].Value); | ||
| Assert.Equal(expectedLine, match.Groups["LineNo"].Value); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData("at MyNamespace.MyClass.MyMethod +")] | ||
| [InlineData("random text")] | ||
| [InlineData("at . in :line ")] | ||
| public void FullStackTraceLine_InvalidInput_DoesNotMatch(string input) | ||
| { | ||
| var match = StringStackTraceFactory.FullStackTraceLine.Match(input); | ||
| Assert.False(match.Success); | ||
| } | ||
| } | ||
|
|
||
| #endif |
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
Troubleshootingwhen 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
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.