Skip to content

Commit

Permalink
Update based on the following plan:
Browse files Browse the repository at this point in the history
1. Mark DiagnosticSource.Write(string,object) as RequiresUnreferencedCode
2. Suppress the warnings for any .NET libraries that call the DiagnosticSource.Write() API, and annotate the .NET types being passed in to preserve their important properties.
    - This was done for HttpClient. ASP.NET and EF will need separate changes when those assemblies are made trim compatible
3. Annotate Activity and its small closure of types (ActivityLink, ActivityEvent, ActivityContext, etc) to ensure none of those properties are trimmed.
4. Suppress trim warnings inside DiagnosticSourceEventSource since the public Write method is marked with RequiresUnreferencedCode.
  • Loading branch information
eerhardt committed Mar 30, 2021
1 parent 47ce518 commit d6ef851
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 51 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
<None Include="DiagnosticSourceUsersGuide.md" />
</ItemGroup>
<ItemGroup Condition="$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) != '.NETCoreApp'">
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicDependencyAttribute.cs" />
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicallyAccessedMemberTypes.cs" />
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\RequiresUnreferencedCodeAttribute.cs" />
<Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\UnconditionalSuppressMessageAttribute.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' != 'netstandard1.1'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Threading;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace System.Diagnostics
{
Expand Down Expand Up @@ -255,6 +256,7 @@ public override bool IsEnabled(string name, object? arg1, object? arg2 = null)
/// <summary>
/// Override abstract method
/// </summary>
[RequiresUnreferencedCode(WriteRequiresUnreferencedCode)]
public override void Write(string name, object? value)
{
for (DiagnosticSubscription? curSubscription = _subscriptions; curSubscription != null; curSubscription = curSubscription.Next)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics.CodeAnalysis;

namespace System.Diagnostics
{
Expand All @@ -15,6 +16,8 @@ namespace System.Diagnostics
/// </summary>
public abstract partial class DiagnosticSource
{
internal const string WriteRequiresUnreferencedCode = "The type of object being written to DiagnosticSource cannot be discovered statically.";

/// <summary>
/// Write is a generic way of logging complex payloads. Each notification
/// is given a name, which identifies it as well as a object (typically an anonymous type)
Expand All @@ -31,6 +34,7 @@ public abstract partial class DiagnosticSource
/// <param name="name">The name of the event being written.</param>
/// <param name="value">An object that represent the value being passed as a payload for the event.
/// This is often an anonymous type which contains several sub-values.</param>
[RequiresUnreferencedCode(WriteRequiresUnreferencedCode)]
public abstract void Write(string name, object? value);

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace System.Diagnostics
{
Expand All @@ -24,6 +25,7 @@ public abstract partial class DiagnosticSource
/// <param name="args">An object that represent the value being passed as a payload for the event.</param>
/// <returns>Started Activity for convenient chaining</returns>
/// <seealso cref="Activity"/>
[RequiresUnreferencedCode(WriteRequiresUnreferencedCode)]
public Activity StartActivity(Activity activity, object? args)
{
activity.Start();
Expand All @@ -41,6 +43,7 @@ public Activity StartActivity(Activity activity, object? args)
/// <param name="activity">Activity to be stopped</param>
/// <param name="args">An object that represent the value being passed as a payload for the event.</param>
/// <seealso cref="Activity"/>
[RequiresUnreferencedCode(WriteRequiresUnreferencedCode)]
public void StopActivity(Activity activity, object? args)
{
// Stop sets the end time if it was unset, but we want it set before we issue the write
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,9 @@ public FilterAndTransform(string filterAndPayloadSpec, int startIdx, int endIdx,
if (eventNameFilter != null)
eventNameFilterPredicate = (string eventName) => eventNameFilter == eventName;

var subscription = newListener.Subscribe(new CallbackObserver<KeyValuePair<string, object?>>(delegate (KeyValuePair<string, object?> evnt)
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "DiagnosticSource.Write is marked with RequiresUnreferencedCode.")]
void OnEventWritten(KeyValuePair<string, object?> evnt)
{
// The filter given to the DiagnosticSource may not work if users don't is 'IsEnabled' as expected.
// Thus we look for any events that may have snuck through and filter them out before forwarding.
Expand All @@ -727,7 +729,9 @@ public FilterAndTransform(string filterAndPayloadSpec, int startIdx, int endIdx,
var outputArgs = this.Morph(evnt.Value);
var eventName = evnt.Key;
writeEvent(newListener.Name, eventName, outputArgs);
}), eventNameFilterPredicate);
}

var subscription = newListener.Subscribe(new CallbackObserver<KeyValuePair<string, object?>>(OnEventWritten), eventNameFilterPredicate);
_liveSubscriptions = new Subscriptions(subscription, _liveSubscriptions);
}
}));
Expand Down Expand Up @@ -948,41 +952,55 @@ internal static void CreateActivityListener(DiagnosticSourceEventSource eventSou
return false;
};

eventSource._activityListener.ActivityStarted = activity =>
eventSource._activityListener.ActivityStarted = activity => OnActivityStarted(eventSource, activity);

eventSource._activityListener.ActivityStopped = activity => OnActivityStopped(eventSource, activity);

ActivitySource.AddActivityListener(eventSource._activityListener);
}

[DynamicDependency(DynamicallyAccessedMemberTypes.PublicProperties, typeof(Activity))]
[DynamicDependency(DynamicallyAccessedMemberTypes.PublicProperties, typeof(ActivityContext))]
[DynamicDependency(DynamicallyAccessedMemberTypes.PublicProperties, typeof(ActivityEvent))]
[DynamicDependency(DynamicallyAccessedMemberTypes.PublicProperties, typeof(ActivityLink))]
[DynamicDependency(nameof(DateTime.Ticks), typeof(DateTime))]
[DynamicDependency(nameof(TimeSpan.Ticks), typeof(TimeSpan))]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "Activity's properties are being preserved with the DynamicDependencies on OnActivityStarted.")]
private static void OnActivityStarted(DiagnosticSourceEventSource eventSource, Activity activity)
{
FilterAndTransform? list = eventSource._activitySourceSpecs;
while (list != null)
{
FilterAndTransform? list = eventSource._activitySourceSpecs;
while (list != null)
if ((list.Events & ActivityEvents.ActivityStart) != 0 &&
(activity.Source.Name == list.SourceName || list.SourceName == "*") &&
(list.ActivityName == null || list.ActivityName == activity.OperationName))
{
if ((list.Events & ActivityEvents.ActivityStart) != 0 &&
(activity.Source.Name == list.SourceName || list.SourceName == "*") &&
(list.ActivityName == null || list.ActivityName == activity.OperationName))
{
eventSource.ActivityStart(activity.Source.Name, activity.OperationName, list.Morph(activity));
return;
}

list = list.Next;
eventSource.ActivityStart(activity.Source.Name, activity.OperationName, list.Morph(activity));
return;
}
};

eventSource._activityListener.ActivityStopped = activity =>
list = list.Next;
}
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "Activity's properties are being preserved with the DynamicDependencies on OnActivityStarted.")]
private static void OnActivityStopped(DiagnosticSourceEventSource eventSource, Activity activity)
{
FilterAndTransform? list = eventSource._activitySourceSpecs;
while (list != null)
{
FilterAndTransform? list = eventSource._activitySourceSpecs;
while (list != null)
if ((list.Events & ActivityEvents.ActivityStop) != 0 &&
(activity.Source.Name == list.SourceName || list.SourceName == "*") &&
(list.ActivityName == null || list.ActivityName == activity.OperationName))
{
if ((list.Events & ActivityEvents.ActivityStop) != 0 &&
(activity.Source.Name == list.SourceName || list.SourceName == "*") &&
(list.ActivityName == null || list.ActivityName == activity.OperationName))
{
eventSource.ActivityStop(activity.Source.Name, activity.OperationName, list.Morph(activity));
return;
}

list = list.Next;
eventSource.ActivityStop(activity.Source.Name, activity.OperationName, list.Morph(activity));
return;
}
};

ActivitySource.AddActivityListener(eventSource._activityListener);
list = list.Next;
}
}

// Move all wildcard nodes at the end of the list.
Expand Down Expand Up @@ -1067,6 +1085,7 @@ private void Dispose()
}
}

[RequiresUnreferencedCode(DiagnosticSource.WriteRequiresUnreferencedCode)]
public List<KeyValuePair<string, string?>> Morph(object? args)
{
// Transform the args into a bag of key-value strings.
Expand Down Expand Up @@ -1105,7 +1124,11 @@ private void Dispose()
Interlocked.CompareExchange(ref _implicitTransformsTable,
new ConcurrentDictionary<Type, TransformSpec?>(1, 8), null);
}
implicitTransforms = _implicitTransformsTable.GetOrAdd(argType, type => MakeImplicitTransforms(type));
implicitTransforms = _implicitTransformsTable.GetOrAdd(argType, type => MakeImplicitTransformsWrapper(type));

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Justification = "The Morph method has RequiresUnreferencedCode, but the trimmer can't see through lamdba calls.")]
static TransformSpec? MakeImplicitTransformsWrapper(Type transformType) => MakeImplicitTransforms(transformType);
}

// implicitTransformas now fetched from cache or constructed, use it to Fetch all the implicit fields.
Expand Down Expand Up @@ -1145,6 +1168,7 @@ private void Dispose()

// Given a type generate all the implicit transforms for type (that is for every field
// generate the spec that fetches it).
[RequiresUnreferencedCode(DiagnosticSource.WriteRequiresUnreferencedCode)]
private static TransformSpec? MakeImplicitTransforms(Type type)
{
TransformSpec? newSerializableArgs = null;
Expand Down Expand Up @@ -1239,6 +1263,7 @@ public TransformSpec(string transformSpec, int startIdx, int endIdx, TransformSp
/// if the spec is OUTSTR=EVENT_VALUE.PROP1.PROP2.PROP3 and the ultimate value of PROP3 is
/// 10 then the return key value pair is KeyValuePair("OUTSTR","10")
/// </summary>
[RequiresUnreferencedCode(DiagnosticSource.WriteRequiresUnreferencedCode)]
public KeyValuePair<string, string?> Morph(object? obj)
{
for (PropertySpec? cur = _fetches; cur != null; cur = cur.Next)
Expand Down Expand Up @@ -1289,6 +1314,7 @@ public PropertySpec(string propertyName, PropertySpec? next)
/// Given an object fetch the property that this PropertySpec represents.
/// obj may be null when IsStatic is true, otherwise it must be non-null.
/// </summary>
[RequiresUnreferencedCode(DiagnosticSource.WriteRequiresUnreferencedCode)]
public object? Fetch(object? obj)
{
PropertyFetch? fetch = _fetchForExpectedType;
Expand Down Expand Up @@ -1331,6 +1357,7 @@ public PropertyFetch(Type? type)
/// <summary>
/// Create a property fetcher for a propertyName
/// </summary>
[RequiresUnreferencedCode(DiagnosticSource.WriteRequiresUnreferencedCode)]
public static PropertyFetch FetcherForProperty(Type? type, string propertyName)
{
if (propertyName == null)
Expand Down Expand Up @@ -1379,11 +1406,7 @@ public static PropertyFetch FetcherForProperty(Type? type, string propertyName)
}
else
{
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:RequiresUnreferencedCode",
Justification = "If the property is trimmed, a message gets logged to inform the user.")]
static PropertyInfo? GetProperty(TypeInfo type, string name) => type.GetDeclaredProperty(name);

PropertyInfo? propertyInfo = GetProperty(typeInfo, propertyName);
PropertyInfo? propertyInfo = typeInfo.GetDeclaredProperty(propertyName);
if (propertyInfo == null)
{
Logger.Message($"Property {propertyName} not found on {type}. Ensure the name is spelled correctly. If you published the application with PublishTrimmed=true, ensure the property was not trimmed away.");
Expand Down
Loading

0 comments on commit d6ef851

Please sign in to comment.