Skip to content
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

Trimming for .NET 7+ #2699

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
8f81603
Conditionally include Ben.Demystifier only if IsTrimmable != true
jamescrosswell Oct 5, 2023
8c8615e
Review feedback
jamescrosswell Oct 9, 2023
edb9e38
Removed automatic registration of WinUIUnhandledExceptionIntegration …
jamescrosswell Oct 9, 2023
48c3f0a
Merge branch 'feat/4.0.0' into feat/4.0.0-sentry-trimmable
jamescrosswell Oct 9, 2023
5d2a51b
Merged DebugStackTrace from feat/4.0.0
jamescrosswell Oct 10, 2023
9ccc529
Checkpoint
jamescrosswell Oct 10, 2023
dcb7441
Compiles (serialization verify tests still fail)
jamescrosswell Oct 10, 2023
4f6644c
Added some tests for JsonText source generated serializers
jamescrosswell Oct 11, 2023
5479f1e
Added SentryOptions.AddJsonSerializerContext
jamescrosswell Oct 11, 2023
98a51c7
Update SerializationTests.verify.cs
jamescrosswell Oct 11, 2023
dc2c522
Update DebugStackTrace.cs
jamescrosswell Oct 11, 2023
a216116
Fixed JsonSerializerContext with CustomConverters
jamescrosswell Oct 11, 2023
3b24c7b
Update ContextsTests.cs
jamescrosswell Oct 11, 2023
ae56436
Update JsonExtensions.cs
jamescrosswell Oct 11, 2023
bfb0818
Added support for a SentryJsonContext as well as a user defined JsonS…
jamescrosswell Oct 11, 2023
7322b6a
Fixed almost all of the JsonTests
jamescrosswell Oct 11, 2023
bb41fbb
Resolved deserialization issues for GraphQLRequestContent
jamescrosswell Oct 12, 2023
c37926c
Fixed MS Build issues
jamescrosswell Oct 12, 2023
29818c7
Update SentryClientTests.CaptureEvent_BeforeEventThrows_ErrorToEventB…
jamescrosswell Oct 12, 2023
adabd5d
Fixed CapturesEventWithContextKey_Implementation test
jamescrosswell Oct 12, 2023
89684a2
Update JsonExtensions.cs
jamescrosswell Oct 12, 2023
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 Sentry.sln.DotSettings
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=UI/@EntryIndexedValue">UI</s:String>
<s:Boolean x:Key="/Default/UserDictionary/Words/=Enricher/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=enrichers/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=instrumenter/@EntryIndexedValue">True</s:Boolean>
Expand Down
8 changes: 8 additions & 0 deletions src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ public void Register(IHub hub, SentryOptions options)
_hub = hub;
_options = options;

#if !TRIMMABLE
// Hook the main event handler
AttachEventHandler();
#endif

// First part of workaround for https://github.com/microsoft/microsoft-ui-xaml/issues/7160
AppDomain.CurrentDomain.FirstChanceException += (_, e) => _lastFirstChanceException = e.Exception;
Expand All @@ -73,6 +75,11 @@ public void Register(IHub hub, SentryOptions options)
});
}

#if !TRIMMABLE
/// <summary>
/// This method uses reflection to hook up an UnhandledExceptionHandler. When <IsTrimmed> is true, users will have
/// follow our guidance to perform this initialization manually.
/// </summary>
private void AttachEventHandler()
{
try
Expand All @@ -92,6 +99,7 @@ private void AttachEventHandler()
_options.LogError("Could not attach WinUIUnhandledExceptionHandler.", ex);
}
}
#endif

private void WinUIUnhandledExceptionHandler(object sender, object e)
{
Expand Down
24 changes: 14 additions & 10 deletions src/Sentry/Internal/DebugStackTrace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,9 @@ private IEnumerable<SentryStackFrame> CreateFrames(StackTrace stackTrace, bool i
{
var frames = _options.StackTraceMode switch
{
#if !TRIMMABLE
StackTraceMode.Enhanced => EnhancedStackTrace.GetFrames(stackTrace).Select(p => new RealStackFrame(p)),
#endif
_ => stackTrace.GetFrames()
// error CS8619: Nullability of reference types in value of type 'StackFrame?[]' doesn't match target type 'IEnumerable<StackFrame>'.
#if NETCOREAPP3_0
Expand Down Expand Up @@ -196,7 +198,7 @@ private IEnumerable<SentryStackFrame> CreateFrames(StackTrace stackTrace, bool i

/// <summary>
/// Native AOT implementation of CreateFrame.
/// Native frames have only limited method information at runtime (and even that can be disabled).
/// Native frames have only limited method information at runtime (and even that can be disabled).
/// We try to parse that and also add addresses for server-side symbolication.
/// </summary>
private SentryStackFrame? TryCreateNativeAOTFrame(IStackFrame stackFrame)
Expand All @@ -213,7 +215,7 @@ private IEnumerable<SentryStackFrame> CreateFrames(StackTrace stackTrace, bool i
}

// Method info is currently only exposed by ToString(), see https://github.com/dotnet/runtime/issues/92869
// We only care about the case where the method is available (`StackTraceSupport` property is the default `true`):
// We only care about the case where the method is available (`StackTraceSupport` property is the default `true`):
// https://github.com/dotnet/runtime/blob/254230253da143a082f47cfaf8711627c0bf2faf/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/DeveloperExperience/DeveloperExperience.cs#L42
internal static SentryStackFrame ParseNativeAOTToString(string info)
{
Expand Down Expand Up @@ -243,7 +245,7 @@ internal static SentryStackFrame ParseNativeAOTToString(string info)
Module = method.DeclaringType?.FullName ?? unknownRequiredField,
Package = method.DeclaringType?.Assembly.FullName
};

#if !TRIMMABLE
if (stackFrame.Frame is EnhancedStackFrame enhancedStackFrame)
{
var stringBuilder = new StringBuilder();
Expand All @@ -267,6 +269,9 @@ internal static SentryStackFrame ParseNativeAOTToString(string info)
{
frame.Function = method.Name;
}
#else
frame.Function = method.Name;
#endif

// Originally we didn't skip methods from dynamic assemblies, so not to break compatibility:
if (_options.StackTraceMode != StackTraceMode.Original && method.Module.Assembly.IsDynamic)
Expand Down Expand Up @@ -345,22 +350,21 @@ internal static SentryStackFrame ParseNativeAOTToString(string info)
frame.ColumnNumber = colNo;
}

if (stackFrame.Frame is not EnhancedStackFrame)
{
DemangleAsyncFunctionName(frame);
DemangleAnonymousFunction(frame);
DemangleLambdaReturnType(frame);
}

#if !TRIMMABLE
if (stackFrame.Frame is EnhancedStackFrame)
{
// In Enhanced mode, Module (which in this case is the Namespace)
// is already prepended to the function, after return type.
// Removing here at the end because this is used to resolve InApp=true/false
// TODO what is this really about? we have already run ConfigureAppFrame() at this time...
frame.Module = null;
return frame;
}
#endif

DemangleAsyncFunctionName(frame);
DemangleAnonymousFunction(frame);
DemangleLambdaReturnType(frame);
return frame;
}

Expand Down
32 changes: 18 additions & 14 deletions src/Sentry/Sentry.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
<NoWarn Condition="'$(TargetFramework)' == 'netstandard2.0'">$(NoWarn);RS0017</NoWarn>
<CLSCompliant Condition="'$(TargetPlatformIdentifier)' == ''">true</CLSCompliant>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<IsTrimmable>true</IsTrimmable>
<DefineConstants>$(DefineConstants);TRIMMABLE</DefineConstants>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this add the TRIMMABLE constant unconditionally? That effectively removes enhanced stack frames at all times.

Copy link
Collaborator Author

@jamescrosswell jamescrosswell Oct 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I does yeah... I didn't intend for this to be merged into the feat/4.0.0 branch - it was just a quick/easy hack to enable trimming so that I could get a sense for the challenges and start to try to work through those.

</PropertyGroup>

<PropertyGroup Condition="'$(SolutionName)' != 'Sentry.Unity'">
Expand All @@ -22,21 +24,23 @@
<Import Project="Platforms\Android\Sentry.Android.props" Condition="'$(TargetPlatformIdentifier)' == 'android'" />
<Import Project="Platforms\iOS\Sentry.iOS.props" Condition="'$(TargetPlatformIdentifier)' == 'ios' Or '$(TargetPlatformIdentifier)' == 'maccatalyst'" />

<!--
Ben.Demystifier is compiled directly into Sentry.
Note: It uses Microsoft.Bcl.AsyncInterfaces, which we get transitively from System.Text.Json.
-->
<ItemGroup>
<Compile Include="..\..\modules\Ben.Demystifier\src\**\*.cs">
<Link>%(RecursiveDir)\%(Filename)%(Extension)</Link>
</Compile>
<Compile Remove="..\..\modules\Ben.Demystifier\**\obj\**" />
</ItemGroup>
<Target Name="IncludeBenDemystifier" Condition="'$(IsTrimmable)' != 'true'">
<!--
Ben.Demystifier is compiled directly into Sentry.
Note: It uses Microsoft.Bcl.AsyncInterfaces, which we get transitively from System.Text.Json.
-->
<ItemGroup>
<Compile Include="..\..\modules\Ben.Demystifier\src\**\*.cs">
<Link>%(RecursiveDir)\%(Filename)%(Extension)</Link>
</Compile>
<Compile Remove="..\..\modules\Ben.Demystifier\**\obj\**" />
</ItemGroup>

<!-- Ben.Demystifier also needs System.Reflection.Metadata 5.0.0 or higher on all platforms. -->
<ItemGroup Condition="$(TargetFramework.StartsWith('netstandard')) or $(TargetFramework.StartsWith('net4')) or $(TargetFramework.StartsWith('netcoreapp'))">
<PackageReference Include="System.Reflection.Metadata" Version="5.0.0" />
</ItemGroup>
<!-- Ben.Demystifier also needs System.Reflection.Metadata 5.0.0 or higher on all platforms. -->
<ItemGroup Condition="$(TargetFramework.StartsWith('netstandard')) or $(TargetFramework.StartsWith('net4')) or $(TargetFramework.StartsWith('netcoreapp'))">
<PackageReference Include="System.Reflection.Metadata" Version="5.0.0" />
</ItemGroup>
</Target>

<!-- Sentry.DiagnosticSource is compiled directly into Sentry for .NET Core and .NET targets only. -->
<PropertyGroup Condition="!$(TargetFramework.StartsWith('netstandard')) and !$(TargetFramework.StartsWith('net4'))">
Expand Down
4 changes: 4 additions & 0 deletions src/Sentry/SentryClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,10 @@ public void CaptureTransaction(Transaction transaction, Hint? hint)
}
catch (Exception e)
{
#if !TRIMMABLE
// Attempt to demystify exceptions before adding them as breadcrumbs.
e.Demystify();
#endif

_options.LogError("The BeforeSendTransaction callback threw an exception. It will be added as breadcrumb and continue.", e);

Expand Down Expand Up @@ -369,8 +371,10 @@ private bool CaptureEnvelope(Envelope envelope)
}
catch (Exception e)
{
#if !TRIMMABLE
// Attempt to demystify exceptions before adding them as breadcrumbs.
e.Demystify();
#endif

_options.LogError("The BeforeSend callback threw an exception. It will be added as breadcrumb and continue.", e);
var data = new Dictionary<string, string>
Expand Down
2 changes: 1 addition & 1 deletion src/Sentry/SentryOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1085,7 +1085,7 @@ public SentryOptions()
#endif
};

#if NET5_0_OR_GREATER
#if NET5_0_OR_GREATER && !TRIMMABLE
if (WinUIUnhandledExceptionIntegration.IsApplicable)
{
this.AddIntegration(new WinUIUnhandledExceptionIntegration());
Expand Down