Trim warnings coming from TryGetAzureRoleInstanceIdNoThrow

[eerhardt]

In trying to use Microsoft.Extensions.Caching.StackExchangeRedis in a Native AOT app (see dotnet/aspnetcore#45910), I'm getting the following warnings from this code:

StackExchange.Redis/src/StackExchange.Redis/Configuration/DefaultOptionsProvider.cs

Lines 208 to 242 in b1fddf3

	internal static string? TryGetAzureRoleInstanceIdNoThrow()
	{
	string? roleInstanceId;
	try
	{
	Assembly? asm = null;
	foreach (var asmb in AppDomain.CurrentDomain.GetAssemblies())
	{
	if (asmb.GetName()?.Name?.Equals("Microsoft.WindowsAzure.ServiceRuntime") == true)
	{
	asm = asmb;
	break;
	}
	}
	if (asm == null)
	return null;
	var type = asm.GetType("Microsoft.WindowsAzure.ServiceRuntime.RoleEnvironment");
	// https://msdn.microsoft.com/en-us/library/microsoft.windowsazure.serviceruntime.roleenvironment.isavailable.aspx
	if (type?.GetProperty("IsAvailable") is not PropertyInfo isAvailableProp
	|| isAvailableProp.GetValue(null, null) is not bool isAvailableVal
	|| !isAvailableVal)
	{
	return null;
	}
	var currentRoleInstanceProp = type.GetProperty("CurrentRoleInstance");
	var currentRoleInstanceId = currentRoleInstanceProp?.GetValue(null, null);
	roleInstanceId = currentRoleInstanceId?.GetType().GetProperty("Id")?.GetValue(currentRoleInstanceId, null)?.ToString();
	if (roleInstanceId.IsNullOrEmpty())
	{
	roleInstanceId = null;
	}

/_/src/StackExchange.Redis/Configuration/DefaultOptionsProvider.cs(225): Trim analysis warning IL2026: StackExchange.Redis.Configuration.DefaultOptionsProvider.TryGetAzureRoleInstanceIdNoThrow(): Using member 'System.Reflection.Assembly.GetType(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/StackExchange.Redis/Configuration/DefaultOptionsProvider.cs(228): Trim analysis warning IL2075: StackExchange.Redis.Configuration.DefaultOptionsProvider.TryGetAzureRoleInstanceIdNoThrow(): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperty(String)'. The return value of method 'System.Reflection.Assembly.GetType(String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/StackExchange.Redis/Configuration/DefaultOptionsProvider.cs(235): Trim analysis warning IL2075: StackExchange.Redis.Configuration.DefaultOptionsProvider.TryGetAzureRoleInstanceIdNoThrow(): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperty(String)'. The return value of method 'System.Reflection.Assembly.GetType(String)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/StackExchange.Redis/Configuration/DefaultOptionsProvider.cs(237): Trim analysis warning IL2075: StackExchange.Redis.Configuration.DefaultOptionsProvider.TryGetAzureRoleInstanceIdNoThrow(): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperty(String)'. The return value of method 'System.Object.GetType()' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]

I'm not sure exactly what this code is for. If it isn't necessary from a trimmed/AOT app, maybe the warnings could just be suppressed? Either way, it would be good to address these warnings so devs using Redis caching in Native AOT apps don't get the warnings.

cc @mgravell

[NickCraver]

Agreed we'd throw on this...but if we removed it wouldn't it just complain about the next 5 reflection usages (or other things?)

This is reflected so we don't have a dependency on Azure SDKs, so I'm not sure what the alternative is - is there an AOT friendly way to do an equivalent thing?

[eerhardt]

You can use a [DynamicDependency] attribute if we need these APIs to be preserved. See https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming#dynamicdependency.

If we did this, we would probably need to suppress the warnings that come when the APIs aren't there.

FYI - @vitek-karas @agocke in case they have a better idea.

BTW - is Microsoft.WindowsAzure.ServiceRuntime.RoleEnvironment.IsAvailable and CurrentRoleInstance the modern ways of checking for "am I in Azure"?

[eerhardt]

if we removed it wouldn't it just complain about the next 5 reflection usages (or other things?)

The way I got these warnings (and the warnings in mgravell/Pipelines.Sockets.Unofficial#72) was following https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming#show-all-warnings-with-sample-application for Microsoft.Extensions.Caching.StackExchangeRedis. There may be more reflection usages in StackExchange.Redis that Microsoft.Extensions.Caching.StackExchangeRedis doesn't need. To get those warnings, you can follow those steps explicitly for the StackExchange.Redis library.

I did it really quick and I got the following extra warnings:

/_/src/StackExchange.Redis/ChannelMessageQueue.cs(132): Trim analysis warning IL2075: StackExchange.Redis.ChannelMessageQueue.TryGetCount(Int32&): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.NonPublicProperties' in call to 'System.Type.GetProperty(String,BindingFlags)'. The return value of method 'System.Object.GetType()' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/StackExchange.Redis/ScriptParameterMapper.cs(173): Trim analysis warning IL2070: StackExchange.Redis.ScriptParameterMapper.IsValidParameterHash(Type,LuaScript,String&,String&): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.PublicNestedTypes', 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.PublicEvents' in call to 'System.Type.GetMember(String)'. The parameter 't' of method 'StackExchange.Redis.ScriptParameterMapper.IsValidParameterHash(Type,LuaScript,String&,String&)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/StackExchange.Redis/ScriptParameterMapper.cs(227): Trim analysis warning IL2070: StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.PublicMethods', 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.PublicNestedTypes', 'DynamicallyAccessedMemberTypes.PublicProperties', 'DynamicallyAccessedMemberTypes.PublicEvents' in call to 'System.Type.GetMember(String)'. The parameter 't' of method 'StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/StackExchange.Redis/ScriptParameterMapper.cs(260): Trim analysis warning IL2026: StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript): Using member 'System.Linq.Expressions.Expression.Property(Expression,String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]
/_/src/StackExchange.Redis/ScriptParameterMapper.cs(261): Trim analysis warning IL2026: StackExchange.Redis.ScriptParameterMapper.GetParameterExtractor(Type,LuaScript): Using member 'System.Linq.Expressions.Expression.Call(Expression,String,Type[],Expression[])' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Creating Expressions requires unreferenced code because the members being referenced by the Expression may be trimmed. [C:\git\azure-activedirectory-identitymodel-extensions-for-dotnet\test\Microsoft.IdentityModel.AotCompatibility.TestApp\Microsoft.IdentityModel.AotCompatibility.TestApp.csproj]

However, that code isn't used by Microsoft.Extensions.Caching.StackExchangeRedis, so I wouldn't need those warnings fixed.

[agocke]

I could be totally wrong, but this actually looks like it might be a safe pattern with some minor changes. If I understand the code correctly, it looks like the code is searching for a well-known type in a well-known assembly and then calling properties on it.

If the code instead used Type.GetType with an assembly-qualified name, and the assembly with that name is passed to the compiler, my expectation would be that that type would be preserved, and so should the properties (also assuming the rest of the reflection against that type is annotated).

I'm not sure what the failure case would be if the assembly isn't present -- but presumably that's still "AOT-safe" since that code path is a valid one. So maybe it would be safe to suppress a warning if one appeared in that situation.

@vitek-karas or @MichalStrehovsky can correct me if I'm missing something here.

[MichalStrehovsky]

Yes, this could be trim safe if it's written a little bit differently. Even just replacing the

var type = asm.GetType("Microsoft.WindowsAzure.ServiceRuntime.RoleEnvironment"); 

with

var type = Type.GetType("Microsoft.WindowsAzure.ServiceRuntime.RoleEnvironment, Microsoft.WindowsAzure.ServiceRuntime"); 

Would be enough for static analysis to kick in and stop warning (we can keep the foreach (var asmb in AppDomain.CurrentDomain.GetAssemblies()) part if triggering an assembly load is really undesired, otherwise I would replace the whole foreach with this simple line).

[vitek-karas]

I would actually question the use of the foreach. Note that it will only return already loaded assemblies. So if the app does include the assembly in question, but hasn't called anything in it yet the assembly will not show up in that list. Is that actually what the behavior should be in this case?

[eerhardt]

I've opened #2451 to address the easy 2 warnings. The 3rd set will be a lot more work (and isn't needed for Microsoft.Extensions.Caching.StackExchangeRedis.