Skip to content

Commit

Permalink
[Core] Adding attributes to support Native AOT compiling (Azure#37734)
Browse files Browse the repository at this point in the history
* part 1

* additional changes

* additional attributes

* additional attributes

* adding diagnosticscope attributes

* updating messages

* more fixes

* additional updates

* updates

* PR feedback

* API update

* removing unnecessary warnings

* reverting attribute on HttpPipelineSynchronousPolicy

* update api

* updates

* update

* API export

* WIP fix 1

* WIP

* attribute adjustments

* adding compatibility test

* WIP adding test

* WIP API

* WIP

* WIP

* updates

* add test script and remove #if statements

* fixing build

* Update Program.cs

* adding some compile statements

* Update AzureCoreEventSource.cs

* build fix attempt

* fix built attempt 2

* Update MutableJsonChange.cs

* Update MutableJsonChange.cs

* Update MutableJsonDocument.cs

* Update HttpPipelineSynchronousPolicy.cs

* adding comments

* PR fb 1

* test

* test2

* test3

* attempt

* additional build fixes

* Remove comment

* PR feedback + fixing impacts of feedback changes

* fixes

* Update GeoRedundantFallbackPolicy.cs

* Update GeoRedundantFallbackPolicy.cs

* Update GeoRedundantFallbackPolicy.cs

* fix

* add tests

* pipeline fixes

* update comments

* feedback

* adjustments

* adjustment

* pipeline fix

* PR feedback 1

* Apply suggestions from code review

Co-authored-by: Vitek Karas <10670590+vitek-karas@users.noreply.github.com>

* PR feedback updates 2

* Update sdk/core/Azure.Core/src/Shared/DiagnosticScope.cs

Co-authored-by: Vitek Karas <10670590+vitek-karas@users.noreply.github.com>

* need one other suppression

* remove 2 annotations + update suppression message

* removing all attributes from RequestContentHelper

* tweak justification

* removing test project from Azure.Core, will be adding feedback to general CI pipeline

* WIP

* feedback

* Update sdk/core/Azure.Core/src/Shared/DiagnosticScope.cs

Co-authored-by: Anne Thompson <annelo@microsoft.com>

* fix attribute and add comments

* Addressing feedback

* adding net 5 preprocessor directives

* Update sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs

Co-authored-by: Vitek Karas <10670590+vitek-karas@users.noreply.github.com>

* remove unused const

* pipeline fix

---------

Co-authored-by: Vitek Karas <10670590+vitek-karas@users.noreply.github.com>
Co-authored-by: Anne Thompson <annelo@microsoft.com>
  • Loading branch information
3 people authored and matthohn-msft committed Oct 27, 2023
1 parent 6b293a3 commit 4cc4ff3
Show file tree
Hide file tree
Showing 33 changed files with 164 additions and 16 deletions.
1 change: 1 addition & 0 deletions eng/Directory.Build.Common.targets
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@
<Compile Include="$(AzureCoreSharedSources)ChangeTrackingDictionary.cs" LinkBase="Shared/Core" />
<Compile Include="$(AzureCoreSharedSources)ChangeTrackingList.cs" LinkBase="Shared/Core" />
<Compile Include="$(AzureCoreSharedSources)ClientDiagnostics.cs" LinkBase="Shared/Core" />
<Compile Include="$(AzureCoreSharedSources)TrimmingAttribute.cs" LinkBase="Shared/Core" />
<Compile Include="$(AzureCoreSharedSources)DiagnosticScope.cs" LinkBase="Shared/Core" />
<Compile Include="$(AzureCoreSharedSources)DiagnosticScopeFactory.cs" LinkBase="Shared/Core" />
<Compile Include="$(AzureCoreSharedSources)FixedDelayWithNoJitterStrategy.cs" LinkBase="Shared/Core" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
<Compile Include="$(AzureCoreSharedSources)PropertyReferenceTypeAttribute.cs" LinkBase="Shared\Core" />
<Compile Include="$(AzureCoreSharedSources)InitializationConstructorAttribute.cs" LinkBase="Shared\Core" />
<Compile Include="$(AzureCoreSharedSources)SerializationConstructorAttribute.cs" LinkBase="Shared\Core" />
<Compile Include="$(AzureCoreSharedSources)TrimmingAttribute.cs" LinkBase="Shared\Core" />
</ItemGroup>

</Project>
3 changes: 3 additions & 0 deletions sdk/core/Azure.Core/api/Azure.Core.net5.0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -544,8 +544,11 @@ protected RequestContent() { }
public static Azure.Core.RequestContent Create(byte[] bytes) { throw null; }
public static Azure.Core.RequestContent Create(byte[] bytes, int index, int length) { throw null; }
public static Azure.Core.RequestContent Create(System.IO.Stream stream) { throw null; }
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("This method uses reflection-based serialization which is incompatible with trimming. Try using one of the 'Create' overloads that doesn't wrap a serialized version of an object.")]
public static Azure.Core.RequestContent Create(object serializable) { throw null; }
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("This method uses reflection-based serialization which is incompatible with trimming. Try using one of the 'Create' overloads that doesn't wrap a serialized version of an object.")]
public static Azure.Core.RequestContent Create(object serializable, Azure.Core.Serialization.JsonPropertyNames propertyNameFormat, string dateTimeFormat = "o") { throw null; }
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("This method uses reflection-based serialization which is incompatible with trimming. Try using one of the 'Create' overloads that doesn't wrap a serialized version of an object.")]
public static Azure.Core.RequestContent Create(object serializable, Azure.Core.Serialization.ObjectSerializer? serializer) { throw null; }
public static Azure.Core.RequestContent Create(System.ReadOnlyMemory<byte> bytes) { throw null; }
public static Azure.Core.RequestContent Create(string content) { throw null; }
Expand Down
6 changes: 6 additions & 0 deletions sdk/core/Azure.Core/api/Azure.Core.net6.0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -544,8 +544,11 @@ protected RequestContent() { }
public static Azure.Core.RequestContent Create(byte[] bytes) { throw null; }
public static Azure.Core.RequestContent Create(byte[] bytes, int index, int length) { throw null; }
public static Azure.Core.RequestContent Create(System.IO.Stream stream) { throw null; }
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("This method uses reflection-based serialization which is incompatible with trimming. Try using one of the 'Create' overloads that doesn't wrap a serialized version of an object.")]
public static Azure.Core.RequestContent Create(object serializable) { throw null; }
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("This method uses reflection-based serialization which is incompatible with trimming. Try using one of the 'Create' overloads that doesn't wrap a serialized version of an object.")]
public static Azure.Core.RequestContent Create(object serializable, Azure.Core.Serialization.JsonPropertyNames propertyNameFormat, string dateTimeFormat = "o") { throw null; }
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("This method uses reflection-based serialization which is incompatible with trimming. Try using one of the 'Create' overloads that doesn't wrap a serialized version of an object.")]
public static Azure.Core.RequestContent Create(object serializable, Azure.Core.Serialization.ObjectSerializer? serializer) { throw null; }
public static Azure.Core.RequestContent Create(System.ReadOnlyMemory<byte> bytes) { throw null; }
public static Azure.Core.RequestContent Create(string content) { throw null; }
Expand Down Expand Up @@ -1024,6 +1027,7 @@ protected HttpPipelinePolicy() { }
protected static void ProcessNext(Azure.Core.HttpMessage message, System.ReadOnlyMemory<Azure.Core.Pipeline.HttpPipelinePolicy> pipeline) { }
protected static System.Threading.Tasks.ValueTask ProcessNextAsync(Azure.Core.HttpMessage message, System.ReadOnlyMemory<Azure.Core.Pipeline.HttpPipelinePolicy> pipeline) { throw null; }
}
[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicMethods)]
public abstract partial class HttpPipelineSynchronousPolicy : Azure.Core.Pipeline.HttpPipelinePolicy
{
protected HttpPipelineSynchronousPolicy() { }
Expand Down Expand Up @@ -1075,6 +1079,7 @@ public ServerCertificateCustomValidationArgs(System.Security.Cryptography.X509Ce
}
namespace Azure.Core.Serialization
{
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("This class utilizes reflection-based JSON serialization and deserialization which is not compatible with trimming.")]
[System.Diagnostics.DebuggerDisplayAttribute("{DebuggerDisplay,nq}")]
public sealed partial class DynamicData : System.Dynamic.IDynamicMetaObjectProvider, System.IDisposable
{
Expand Down Expand Up @@ -1109,6 +1114,7 @@ public partial interface IMemberNameConverter
{
string? ConvertMemberName(System.Reflection.MemberInfo member);
}
[System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("This class uses reflection-based JSON serialization and deserialization that is not compatible with trimming.")]
public partial class JsonObjectSerializer : Azure.Core.Serialization.ObjectSerializer, Azure.Core.Serialization.IMemberNameConverter
{
public JsonObjectSerializer() { }
Expand Down
2 changes: 2 additions & 0 deletions sdk/core/Azure.Core/src/Diagnostics/AzureCoreEventSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.Tracing;
using System.Diagnostics.CodeAnalysis;
using System.Text;

namespace Azure.Core.Diagnostics
Expand Down Expand Up @@ -231,6 +232,7 @@ public void ErrorResponseContentTextBlock(string requestId, int blockNumber, str
}

[Event(RequestRetryingEvent, Level = EventLevel.Informational, Message = "Request [{0}] attempt number {1} took {2:00.0}s")]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", Justification = "WriteEvent is used with primitive types.")]
public void RequestRetrying(string requestId, int retryNumber, double seconds)
{
WriteEvent(RequestRetryingEvent, requestId, retryNumber, seconds);
Expand Down
14 changes: 14 additions & 0 deletions sdk/core/Azure.Core/src/DynamicData/DynamicData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Text.Json;
using System.Text.Json.Serialization;
using Azure.Core.Json;
Expand All @@ -20,6 +22,10 @@ namespace Azure.Core.Serialization
/// This and related types are not intended to be mocked.
/// </summary>
[DebuggerDisplay("{DebuggerDisplay,nq}")]
#if !NET5_0 // RequiresUnreferencedCode in net5.0 doesn't have AttributeTargets.Class as a target, but it was added in net6.0
[RequiresUnreferencedCode(MutableJsonDocument.SerializationRequiresUnreferencedCodeClass)]
[RequiresDynamicCode(MutableJsonDocument.SerializationRequiresUnreferencedCodeClass)]
#endif
[JsonConverter(typeof(DynamicDataJsonConverter))]
public sealed partial class DynamicData : IDisposable
{
Expand All @@ -33,6 +39,8 @@ public sealed partial class DynamicData : IDisposable
private readonly DynamicDataOptions _options;
private readonly JsonSerializerOptions _serializerOptions;

internal const string SerializationRequiresUnreferencedCodeClass = "This class utilizes reflection-based JSON serialization and deserialization which is not compatible with trimming.";

internal DynamicData(MutableJsonElement element, DynamicDataOptions options)
{
_element = element;
Expand Down Expand Up @@ -471,8 +479,14 @@ public override int GetHashCode()
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
private string DebuggerDisplay => _element.DebuggerDisplay;

#if !NET5_0 // RequiresUnreferencedCode in net5.0 doesn't have AttributeTargets.Class as a target, but it was added in net6.0
[RequiresUnreferencedCode(ClassIsIncompatibleWithTrimming)]
#endif
[RequiresDynamicCode(ClassIsIncompatibleWithTrimming)]
private class DynamicDataJsonConverter : JsonConverter<DynamicData>
{
public const string ClassIsIncompatibleWithTrimming = "Using DynamicData or DynamicDataConverter is not compatible with trimming due to reflection-based serialization.";

public override DynamicData Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
MutableJsonDocument mdoc = MutableJsonDocument.Parse(ref reader);
Expand Down
1 change: 1 addition & 0 deletions sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System;
using System.Diagnostics.CodeAnalysis;
using System.Text.Json;

#nullable enable
Expand Down
16 changes: 16 additions & 0 deletions sdk/core/Azure.Core/src/DynamicData/MutableJsonDocument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Buffers;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Text;
using System.Text.Json;
Expand All @@ -15,6 +16,13 @@ namespace Azure.Core.Json
/// <summary>
/// A mutable representation of a JSON value.
/// </summary>
#if !NET5_0 // RequiresUnreferencedCode in net5.0 doesn't have AttributeTargets.Class as a target, but it was added in net6.0
// This class is marked as RequiresUnreferencedCode and RequiresDynamic code for two reasons. First, for the usage of MutableJsonElement in RootElement and WriteTo. Second, the method WriteTo
// is used in the MutableJsonDocumentConverter, which causes MutableJsonDocumentConverter to be incompatible with trimming. Since the class is used in the attribute on this class, the entire
// class must be annotated.
[RequiresUnreferencedCode(SerializationRequiresUnreferencedCodeClass)]
[RequiresDynamicCode(SerializationRequiresUnreferencedCodeClass)]
#endif
[JsonConverter(typeof(MutableJsonDocumentConverter))]
internal sealed partial class MutableJsonDocument : IDisposable
{
Expand All @@ -26,6 +34,8 @@ internal sealed partial class MutableJsonDocument : IDisposable
private readonly JsonSerializerOptions _serializerOptions;
internal JsonSerializerOptions SerializerOptions => _serializerOptions;

internal const string SerializationRequiresUnreferencedCodeClass = "This class utilizes reflection-based JSON serialization and deserialization which is not compatible with trimming.";

private ChangeTracker? _changeTracker;
internal ChangeTracker Changes
{
Expand Down Expand Up @@ -210,8 +220,14 @@ private MutableJsonDocument(JsonDocument document, ReadOnlyMemory<byte> utf8Json
_serializerOptions = serializerOptions ?? DefaultSerializerOptions;
}

#if !NET5_0 // RequiresUnreferencedCode in net5.0 doesn't have AttributeTargets.Class as a target, but it was added in net6.0
[RequiresUnreferencedCode(classIsIncompatibleWithTrimming)]
#endif
[RequiresDynamicCode(classIsIncompatibleWithTrimming)]
private class MutableJsonDocumentConverter : JsonConverter<MutableJsonDocument>
{
public const string classIsIncompatibleWithTrimming = "Using MutableJsonDocument or MutableJsonDocumentConverter is not compatible with trimming due to reflection-based serialization.";

public override MutableJsonDocument Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
return Parse(ref reader);
Expand Down
21 changes: 21 additions & 0 deletions sdk/core/Azure.Core/src/DynamicData/MutableJsonElement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Buffers;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Text;
using System.Text.Json;
Expand All @@ -17,6 +18,12 @@ namespace Azure.Core.Json
/// A mutable representation of a JSON element.
/// </summary>
[DebuggerDisplay("{DebuggerDisplay,nq}")]
// This struct is incompatible with trimming for a couple reasons:
// 1. MutableJsonElementConverter is not compatible with trimming, since it is used in an attribute, the whole struct is incompatible with trimming.
// 2. The internal static method SerializeToJsonElement uses reflection-based serialization. This method is used throughout the non-static methods within this struct.
// In order to avoid annotating every single method, we are instead annotating the constructor, and the problematic static method.
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", Justification = "The constructor is marked as RequiresUnreferencedCode, since the whole struct is incompatible with trimming.")]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL3050", Justification = "The constructor is marked as RequiresDynamicCode, since the whole struct is incompatible with AOT.")]
[JsonConverter(typeof(MutableJsonElementConverter))]
internal readonly partial struct MutableJsonElement
{
Expand All @@ -27,8 +34,15 @@ internal readonly partial struct MutableJsonElement
private readonly string _path;
private readonly int _highWaterMark;

internal const string SerializationRequiresUnreferencedCodeConstructor = "This struct utilizes reflection-based JSON serialization which is not compatible with trimming.";
internal const string SerializationRequiresUnreferencedCodeMethod = "This method utilizes reflection-based JSON serialization which is not compatible with trimming.";

private readonly MutableJsonDocument.ChangeTracker Changes => _root.Changes;

// The default constructor generated by the compiler should also be annotated - but since this struct cannot function if the default constructor is used, it is okay that
// only this constructor is annotated.
[RequiresUnreferencedCode(SerializationRequiresUnreferencedCodeConstructor)]
[RequiresDynamicCode(SerializationRequiresUnreferencedCodeConstructor)]
internal MutableJsonElement(MutableJsonDocument root, JsonElement element, string path, int highWaterMark = -1)
{
_element = element;
Expand Down Expand Up @@ -1302,6 +1316,10 @@ public override string ToString()
return _element.ToString() ?? "null";
}

#if !NET5_0 // RequiresUnreferencedCode in net5.0 doesn't have AttributeTargets.Class as a target, but it was added in net6.0
[RequiresUnreferencedCode(SerializationRequiresUnreferencedCodeMethod)]
[RequiresDynamicCode(SerializationRequiresUnreferencedCodeMethod)]
#endif
internal static JsonElement SerializeToJsonElement(object? value, JsonSerializerOptions? options = default)
{
byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(value, options);
Expand Down Expand Up @@ -1395,6 +1413,9 @@ private void EnsureValid()
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
internal string DebuggerDisplay => $"ValueKind = {ValueKind} : \"{ToString()}\"";

#if !NET5_0 // RequiresUnreferencedCode in net5.0 doesn't have AttributeTargets.Class as a target, but it was added in net6.0
[RequiresUnreferencedCode("Using MutableJsonElement or MutableJsonElementConverter is not compatible with trimming due to reflection-based serialization.")]
#endif
private class MutableJsonElementConverter : JsonConverter<MutableJsonElement>
{
public override MutableJsonElement Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Threading.Tasks;

Expand All @@ -11,6 +12,9 @@ namespace Azure.Core.Pipeline
/// <summary>
/// Represents a <see cref="HttpPipelinePolicy"/> that doesn't do any asynchronous or synchronously blocking operations.
/// </summary>
#if !NET5_0 // DynamicallyAccessedMembers in net5.0 doesn't have AttributeTargets.Class as a target, but it was added in net6.0
[DynamicallyAccessedMembers(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicMethods)]
#endif
public abstract class HttpPipelineSynchronousPolicy : HttpPipelinePolicy
{
private static Type[] _onReceivedResponseParameters = new[] { typeof(HttpMessage) };
Expand Down
17 changes: 12 additions & 5 deletions sdk/core/Azure.Core/src/Pipeline/Internal/RequestActivityPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Threading.Tasks;

Expand Down Expand Up @@ -58,11 +59,7 @@ public override void Process(HttpMessage message, ReadOnlyMemory<HttpPipelinePol

private async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPipelinePolicy> pipeline, bool async)
{
#if NETCOREAPP2_1
using var scope = new DiagnosticScope("Azure.Core.Http.Request", s_diagnosticSource, message, s_activitySource, DiagnosticScope.ActivityKind.Client, false);
#else
using var scope = new DiagnosticScope("Azure.Core.Http.Request", s_diagnosticSource, message, s_activitySource, System.Diagnostics.ActivityKind.Client, false);
#endif
using var scope = CreateDiagnosticScope(message);

bool isActivitySourceEnabled = IsActivitySourceEnabled;

Expand Down Expand Up @@ -136,6 +133,16 @@ private async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPip
}
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", Justification = "The values being passed into Write have the commonly used properties being preserved with DynamicallyAccessedMembers.")]
private DiagnosticScope CreateDiagnosticScope<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>(T sourceArgs)
{
#if NETCOREAPP2_1
return new DiagnosticScope("Azure.Core.Http.Request", s_diagnosticSource, sourceArgs, s_activitySource, DiagnosticScope.ActivityKind.Client, false);
#else
return new DiagnosticScope("Azure.Core.Http.Request", s_diagnosticSource, sourceArgs, s_activitySource, System.Diagnostics.ActivityKind.Client, false);
#endif
}

private static ValueTask ProcessNextAsync(HttpMessage message, ReadOnlyMemory<HttpPipelinePolicy> pipeline, bool async)
{
Activity? currentActivity = Activity.Current;
Expand Down
Loading

0 comments on commit 4cc4ff3

Please sign in to comment.