Skip to content

Commit

Permalink
Fixed Trim warnings in DiagnosticSource and WinUIUnhandledException i…
Browse files Browse the repository at this point in the history
…ntegrations (#3410)
  • Loading branch information
jamescrosswell committed Jun 7, 2024
1 parent 3cc2e44 commit 9f36d22
Show file tree
Hide file tree
Showing 12 changed files with 106 additions and 56 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<TargetFrameworks>net8.0;net6.0;net462</TargetFrameworks>
<PublishAot Condition="$(TargetFramework.StartsWith('net8'))">true</PublishAot>
</PropertyGroup>
<PropertyGroup Condition="$(TargetFramework.StartsWith('net8'))">
<PublishAot>true</PublishAot>
<PublishTrimmed>true</PublishTrimmed>
</PropertyGroup>

<PropertyGroup>
Expand Down Expand Up @@ -35,6 +38,9 @@
<ItemGroup>
<ProjectReference Include="..\..\src\Sentry\Sentry.csproj" />
</ItemGroup>
<ItemGroup>
<TrimmerRootAssembly Include="Sentry" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'net462'">
<PackageReference Include="System.Net.Http" Version="4.3.4" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -41,7 +36,7 @@ internal EFDiagnosticSourceHelper(IHub hub, SentryOptions options)

protected static Guid? TryGetConnectionId(ISpan span) => span.Extra.TryGetValue<string, Guid?>(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)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
using Sentry.Extensibility;

namespace Sentry.Internal.DiagnosticSource;

/// <summary>
/// Various reflection helper methods.
/// </summary>
/// <remarks>
/// 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.
/// </remarks>
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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand All @@ -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)
{
Expand All @@ -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);
}

Expand Down Expand Up @@ -248,9 +248,9 @@ public void OnNext(KeyValuePair<string, object?> kvp)
}
}

private static void TrySetConnectionStatistics(ISpan span, object? value)
private void TrySetConnectionStatistics(ISpan span, object? value)
{
if (value?.GetProperty("Statistics") is not Dictionary<object, object> statistics)
if (value?.GetProperty("Statistics", _options.DiagnosticLogger) is not Dictionary<object, object> statistics)
{
return;
}
Expand Down
11 changes: 10 additions & 1 deletion src/Sentry/Integrations/WinUIUnhandledExceptionIntegration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;

Expand Down Expand Up @@ -102,6 +110,7 @@ private void AttachEventHandler()
}
}

[UnconditionalSuppressMessage("TrimAnalyzer", "IL2075", Justification = AotHelper.SuppressionJustification)]
private void WinUIUnhandledExceptionHandler(object sender, object e)
{
bool handled;
Expand Down
11 changes: 10 additions & 1 deletion src/Sentry/Internal/AotHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -17,10 +18,18 @@ private class AotTester
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
}
25 changes: 0 additions & 25 deletions src/Sentry/Internal/Extensions/MiscExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,31 +68,6 @@ public static void CancelAfterSafe(this CancellationTokenSource cts, TimeSpan ti
/// </remarks>
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<TKey, TValue>(
this ICollection<KeyValuePair<TKey, TValue>> collection,
TKey key,
Expand Down
Original file line number Diff line number Diff line change
@@ -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
{
Expand Down
1 change: 1 addition & 0 deletions test/Sentry.Testing/BindableTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#if !NETFRAMEWORK
using Microsoft.Extensions.Configuration;
using Sentry.Internal.DiagnosticSource;

namespace Sentry.Testing;

Expand Down

0 comments on commit 9f36d22

Please sign in to comment.