From 9f36d22f3b216351d5957b74e831e8b92636e6ba Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Fri, 7 Jun 2024 21:46:09 +1200 Subject: [PATCH] Fixed Trim warnings in DiagnosticSource and WinUIUnhandledException integrations (#3410) --- CHANGELOG.md | 1 + .../Sentry.Samples.Console.Basic.csproj | 8 +++- .../EFCommandDiagnosticSourceHelper.cs | 2 +- .../EFDiagnosticSourceHelper.cs | 21 ++++----- .../DiagnosticSource/ReflectionHelper.cs | 46 +++++++++++++++++++ .../SentryDiagnosticListenerIntegration.cs | 6 +++ .../DiagnosticSource/SentrySqlListener.cs | 24 +++++----- .../WinUIUnhandledExceptionIntegration.cs | 11 ++++- src/Sentry/Internal/AotHelper.cs | 11 ++++- .../Internal/Extensions/MiscExtensions.cs | 25 ---------- .../ReflectionHelperTests.cs} | 6 ++- test/Sentry.Testing/BindableTests.cs | 1 + 12 files changed, 106 insertions(+), 56 deletions(-) create mode 100644 src/Sentry.DiagnosticSource/Internal/DiagnosticSource/ReflectionHelper.cs rename test/{Sentry.Tests/Internals/Extensions/MiscExtensionsTests.cs => Sentry.DiagnosticSource.Tests/ReflectionHelperTests.cs} (93%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6571ae80c2..6d20f92079 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Fixed null IServiceProvider in anonymous routes with OpenTelemetry ([#3401](https://github.com/getsentry/sentry-dotnet/pull/3401)) +- Fixed Trim warnings in Sentry.DiagnosticSource and WinUIUnhandledException integrations ([#3410](https://github.com/getsentry/sentry-dotnet/pull/3410)) ### Dependencies diff --git a/samples/Sentry.Samples.Console.Basic/Sentry.Samples.Console.Basic.csproj b/samples/Sentry.Samples.Console.Basic/Sentry.Samples.Console.Basic.csproj index 7e30da951d..f86d175453 100644 --- a/samples/Sentry.Samples.Console.Basic/Sentry.Samples.Console.Basic.csproj +++ b/samples/Sentry.Samples.Console.Basic/Sentry.Samples.Console.Basic.csproj @@ -5,7 +5,10 @@ enable enable net8.0;net6.0;net462 - true + + + true + true @@ -35,6 +38,9 @@ + + + diff --git a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFCommandDiagnosticSourceHelper.cs b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFCommandDiagnosticSourceHelper.cs index 14d67caf69..baccd1366d 100644 --- a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFCommandDiagnosticSourceHelper.cs +++ b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFCommandDiagnosticSourceHelper.cs @@ -13,7 +13,7 @@ internal EFCommandDiagnosticSourceHelper(IHub hub, SentryOptions options) : base protected override string GetDescription(object? diagnosticSourceValue) => FilterNewLineValue(diagnosticSourceValue) ?? string.Empty; - private static Guid? GetCommandId(object? diagnosticSourceValue) => diagnosticSourceValue?.GetGuidProperty("CommandId"); + private Guid? GetCommandId(object? diagnosticSourceValue) => diagnosticSourceValue?.GetGuidProperty("CommandId", Options.DiagnosticLogger); private static void SetCommandId(ISpan span, Guid? commandId) { diff --git a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFDiagnosticSourceHelper.cs b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFDiagnosticSourceHelper.cs index 9dcf190e6b..4b57fd4657 100644 --- a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFDiagnosticSourceHelper.cs +++ b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFDiagnosticSourceHelper.cs @@ -11,27 +11,22 @@ internal abstract class EFDiagnosticSourceHelper protected abstract string? GetDescription(object? diagnosticSourceValue); - protected static string? GetDatabaseName(object? diagnosticSourceValue) => - diagnosticSourceValue?.GetStringProperty("Connection.Database"); + protected string? GetDatabaseName(object? diagnosticSourceValue) => + diagnosticSourceValue?.GetStringProperty("Connection.Database", Options.DiagnosticLogger); - protected static string? GetDatabaseSystem(object? diagnosticSourceValue) + protected string? GetDatabaseSystem(object? diagnosticSourceValue) { - var providerName = diagnosticSourceValue?.GetStringProperty("Context.Database.ProviderName"); + var providerName = diagnosticSourceValue?.GetStringProperty("Context.Database.ProviderName", Options.DiagnosticLogger); if (providerName is null) { return null; } - if (DatabaseProviderSystems.ProviderSystems.TryGetValue(providerName, out var dbSystem)) - { - return dbSystem; - } - - return null; + return DatabaseProviderSystems.ProviderSystems.GetValueOrDefault(providerName); } - protected static string? GetDatabaseServerAddress(object? diagnosticSourceValue) => - diagnosticSourceValue?.GetStringProperty("Connection.DataSource"); + protected string? GetDatabaseServerAddress(object? diagnosticSourceValue) => + diagnosticSourceValue?.GetStringProperty("Connection.DataSource", Options.DiagnosticLogger); internal EFDiagnosticSourceHelper(IHub hub, SentryOptions options) { @@ -41,7 +36,7 @@ internal EFDiagnosticSourceHelper(IHub hub, SentryOptions options) protected static Guid? TryGetConnectionId(ISpan span) => span.Extra.TryGetValue(EFKeys.DbConnectionId); - protected static Guid? GetConnectionId(object? diagnosticSourceValue) => diagnosticSourceValue?.GetGuidProperty("ConnectionId"); + protected Guid? GetConnectionId(object? diagnosticSourceValue) => diagnosticSourceValue?.GetGuidProperty("ConnectionId", Options.DiagnosticLogger); protected static void SetConnectionId(ISpan span, Guid? connectionId) { diff --git a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/ReflectionHelper.cs b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/ReflectionHelper.cs new file mode 100644 index 0000000000..f569c885f3 --- /dev/null +++ b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/ReflectionHelper.cs @@ -0,0 +1,46 @@ +using Sentry.Extensibility; + +namespace Sentry.Internal.DiagnosticSource; + +/// +/// Various reflection helper methods. +/// +/// +/// Note that the methods in this class are incompatible with Trimming. They are +/// used exclusively in the `Sentry.Internal.DiagnosticSource` integration, which +/// we disable if trimming is enabled to avoid problems at runtime. +/// +internal static class ReflectionHelper +{ + [UnconditionalSuppressMessage("TrimAnalyzer", "IL2075", Justification = AotHelper.SuppressionJustification)] + public static object? GetProperty(this object obj, string name, IDiagnosticLogger? logger = null) + { + if (AotHelper.IsTrimmed) + { + logger?.LogInfo("ReflectionHelper.GetProperty should not be used when trimming is enabled"); + return null; + } + + var propertyNames = name.Split('.'); + var currentObj = obj; + + foreach (var propertyName in propertyNames) + { + var property = currentObj?.GetType().GetProperty(propertyName); + if (property == null) + { + return null; + } + + currentObj = property.GetValue(currentObj); + } + + return currentObj; + } + + public static Guid? GetGuidProperty(this object obj, string name, IDiagnosticLogger? logger = null) => + obj.GetProperty(name, logger) as Guid?; + + public static string? GetStringProperty(this object obj, string name, IDiagnosticLogger? logger = null) => + obj.GetProperty(name, logger) as string; +} diff --git a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentryDiagnosticListenerIntegration.cs b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentryDiagnosticListenerIntegration.cs index 68bff6a399..39ced41aa5 100644 --- a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentryDiagnosticListenerIntegration.cs +++ b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentryDiagnosticListenerIntegration.cs @@ -13,6 +13,12 @@ public void Register(IHub hub, SentryOptions options) return; } + if (AotHelper.IsTrimmed) + { + options.Log(SentryLevel.Info, "DiagnosticSource Integration is disabled because trimming is enabled."); + return; + } + var subscriber = new SentryDiagnosticSubscriber(hub, options); DiagnosticListener.AllListeners.Subscribe(subscriber); } diff --git a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentrySqlListener.cs b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentrySqlListener.cs index 17edf945b6..641e307afe 100644 --- a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentrySqlListener.cs +++ b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentrySqlListener.cs @@ -81,8 +81,8 @@ private void AddSpan(string operation, object? value) var parent = transaction.GetDbParentSpan(); var span = parent.StartChild(operation); span.SetExtra(OTelKeys.DbSystem, "sql"); - SetOperationId(span, value?.GetGuidProperty("OperationId")); - SetConnectionId(span, value?.GetGuidProperty("ConnectionId")); + SetOperationId(span, value?.GetGuidProperty("OperationId", _options.DiagnosticLogger)); + SetConnectionId(span, value?.GetGuidProperty("ConnectionId", _options.DiagnosticLogger)); } private ISpan? GetSpan(SentrySqlSpanType type, object? value) @@ -96,7 +96,7 @@ private void AddSpan(string operation, object? value) switch (type) { case SentrySqlSpanType.Execution: - var operationId = value?.GetGuidProperty("OperationId"); + var operationId = value?.GetGuidProperty("OperationId", _options.DiagnosticLogger); if (TryGetQuerySpan(transaction, operationId) is { } querySpan) { return querySpan; @@ -108,7 +108,7 @@ private void AddSpan(string operation, object? value) return null; case SentrySqlSpanType.Connection: - var connectionId = value?.GetGuidProperty("ConnectionId"); + var connectionId = value?.GetGuidProperty("ConnectionId", _options.DiagnosticLogger); if (TryGetConnectionSpan(transaction, connectionId) is { } connectionSpan) { return connectionSpan; @@ -150,7 +150,7 @@ private void UpdateConnectionSpan(object? value) return; } - var operationId = value.GetGuidProperty("OperationId"); + var operationId = value.GetGuidProperty("OperationId", _options.DiagnosticLogger); if (operationId == null) { return; @@ -159,18 +159,18 @@ private void UpdateConnectionSpan(object? value) var spans = transaction.Spans.Where(span => span.Operation is "db.connection").ToList(); if (spans.Find(span => !span.IsFinished && TryGetOperationId(span) == operationId) is { } connectionSpan) { - if (value.GetGuidProperty("ConnectionId") is { } connectionId) + if (value.GetGuidProperty("ConnectionId", _options.DiagnosticLogger) is { } connectionId) { SetConnectionId(connectionSpan, connectionId); } - if (value.GetStringProperty("Connection.Database") is { } dbName) + if (value.GetStringProperty("Connection.Database", _options.DiagnosticLogger) is { } dbName) { connectionSpan.Description = dbName; SetDatabaseName(connectionSpan, dbName); } - if (value.GetStringProperty("Connection.DataSource") is { } dbSource) + if (value.GetStringProperty("Connection.DataSource", _options.DiagnosticLogger) is { } dbSource) { SetDatabaseAddress(connectionSpan, dbSource); } @@ -193,7 +193,7 @@ private void FinishCommandSpan(object? value, SpanStatus spanStatus) // Try to lookup the associated connection span so that we can store the db.name in // the command span as well. This will be easier for users to read/identify than the // ConnectionId (which is a Guid) - var connectionId = value.GetGuidProperty("ConnectionId"); + var connectionId = value.GetGuidProperty("ConnectionId", _options.DiagnosticLogger); var transaction = _hub.GetTransactionIfSampled(); if (TryGetConnectionSpan(transaction!, connectionId) is { } connectionSpan) { @@ -203,7 +203,7 @@ private void FinishCommandSpan(object? value, SpanStatus spanStatus) } } - commandSpan.Description = value.GetStringProperty("Command.CommandText"); + commandSpan.Description = value.GetStringProperty("Command.CommandText", _options.DiagnosticLogger); commandSpan.Finish(spanStatus); } @@ -248,9 +248,9 @@ when GetSpan(SentrySqlSpanType.Connection, kvp.Value) is { } closeSpan: } } - private static void TrySetConnectionStatistics(ISpan span, object? value) + private void TrySetConnectionStatistics(ISpan span, object? value) { - if (value?.GetProperty("Statistics") is not Dictionary statistics) + if (value?.GetProperty("Statistics", _options.DiagnosticLogger) is not Dictionary statistics) { return; } diff --git a/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs b/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs index a52f3951c1..3341abc2dd 100644 --- a/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs +++ b/src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs @@ -19,7 +19,9 @@ namespace Sentry.Integrations; // See: https://github.com/microsoft/microsoft-ui-xaml/issues/5221 // // Note that we use reflection in this integration to get at WinUI code. -// If we ever add a Windows platform target (net6.0-windows, etc.), we could refactor to avoid reflection. +// If we ever add a Windows platform target (net6.0-windows, etc.), we could refactor +// to avoid reflection (which would also allow us to support trimming with this +// integration). // // This integration is for WinUI 3. It does NOT work for UWP (WinUI 2). // For UWP, the calling application will need to hook the event handler. @@ -45,6 +47,12 @@ public void Register(IHub hub, SentryOptions options) return; } + if (AotHelper.IsTrimmed) + { + options.Log(SentryLevel.Info, "WinUIUnhandledExceptionIntegration Integration is disabled because trimming is enabled."); + return; + } + _hub = hub; _options = options; @@ -102,6 +110,7 @@ private void AttachEventHandler() } } + [UnconditionalSuppressMessage("TrimAnalyzer", "IL2075", Justification = AotHelper.SuppressionJustification)] private void WinUIUnhandledExceptionHandler(object sender, object e) { bool handled; diff --git a/src/Sentry/Internal/AotHelper.cs b/src/Sentry/Internal/AotHelper.cs index 6de2729f6c..5c3387b535 100644 --- a/src/Sentry/Internal/AotHelper.cs +++ b/src/Sentry/Internal/AotHelper.cs @@ -3,6 +3,7 @@ namespace Sentry.Internal; internal static class AotHelper { internal const string SuppressionJustification = "Non-trimmable code is avoided at runtime"; + internal static bool IsTrimmed { get; } private class AotTester { @@ -17,10 +18,18 @@ public void Test() { } static AotHelper() { var stackTrace = new StackTrace(false); - IsNativeAot = stackTrace.GetFrame(0)?.GetMethod() is null; + IsTrimmed = stackTrace.GetFrame(0)?.GetMethod() is null; + IsNativeAot = IsTrimmed; } #else // This is a compile-time const so that the irrelevant code is removed during compilation. internal const bool IsNativeAot = false; + + [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = AotHelper.SuppressionJustification)] + static AotHelper() + { + var stackTrace = new StackTrace(false); + IsTrimmed = stackTrace.GetFrame(0)?.GetMethod() is null; + } #endif } diff --git a/src/Sentry/Internal/Extensions/MiscExtensions.cs b/src/Sentry/Internal/Extensions/MiscExtensions.cs index 25bce31157..6831fccbcc 100644 --- a/src/Sentry/Internal/Extensions/MiscExtensions.cs +++ b/src/Sentry/Internal/Extensions/MiscExtensions.cs @@ -68,31 +68,6 @@ public static void CancelAfterSafe(this CancellationTokenSource cts, TimeSpan ti /// public static bool IsNull(this object? o) => o is null; - public static object? GetProperty(this object obj, string name) - { - var propertyNames = name.Split('.'); - var currentObj = obj; - - foreach (var propertyName in propertyNames) - { - var property = currentObj?.GetType().GetProperty(propertyName); - if (property == null) - { - return null; - } - - currentObj = property.GetValue(currentObj); - } - - return currentObj; - } - - public static Guid? GetGuidProperty(this object obj, string name) => - obj.GetProperty(name) as Guid?; - - public static string? GetStringProperty(this object obj, string name) => - obj.GetProperty(name) as string; - public static void Add( this ICollection> collection, TKey key, diff --git a/test/Sentry.Tests/Internals/Extensions/MiscExtensionsTests.cs b/test/Sentry.DiagnosticSource.Tests/ReflectionHelperTests.cs similarity index 93% rename from test/Sentry.Tests/Internals/Extensions/MiscExtensionsTests.cs rename to test/Sentry.DiagnosticSource.Tests/ReflectionHelperTests.cs index 088ea23b3b..b1ba997c14 100644 --- a/test/Sentry.Tests/Internals/Extensions/MiscExtensionsTests.cs +++ b/test/Sentry.DiagnosticSource.Tests/ReflectionHelperTests.cs @@ -1,6 +1,8 @@ -namespace Sentry.Tests.Internals.Extensions; +using Sentry.Internal.DiagnosticSource; -public class MiscExtensionsTests +namespace Sentry.DiagnosticSource.Tests; + +public class ReflectionHelperTests { public class MyClass { diff --git a/test/Sentry.Testing/BindableTests.cs b/test/Sentry.Testing/BindableTests.cs index c802f11f84..199eb481da 100644 --- a/test/Sentry.Testing/BindableTests.cs +++ b/test/Sentry.Testing/BindableTests.cs @@ -1,5 +1,6 @@ #if !NETFRAMEWORK using Microsoft.Extensions.Configuration; +using Sentry.Internal.DiagnosticSource; namespace Sentry.Testing;