Trimming and AOT compatibility of this library

[eerhardt]

Is your feature request related to a problem? Please describe.

I would like to use RabbitMQ.Client in an app that has been published for native AOT. See https://learn.microsoft.com/dotnet/core/deploying/native-aot/?tabs=net8. However, when I do I'm getting a single warning coming from RabbitMQ's Error Logging EventSource code:

ILC : Trim analysis warning IL2026: RabbitMQ.Client.Logging.RabbitMqClientEventSource.Error(String,RabbitMqExceptionDetail): Using member 'System.Diagnostics.Tracing.EventSource.WriteEvent(Int32,Object[])' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. EventSource will serialize the whole object graph. Trimmer will not safely handle this case because properties may be trimmed.

This warning is coming from

rabbitmq-dotnet-client/projects/RabbitMQ.Client/client/logging/RabbitMqClientEventSource.cs

Lines 73 to 77 in a0321c6

	[Event(3, Message = "ERROR", Keywords = Keywords.Log, Level = EventLevel.Error)]
	public void Error(string message, RabbitMqExceptionDetail ex)
	{
	if (IsEnabled())
	WriteEvent(3, message, ex);

and

rabbitmq-dotnet-client/projects/RabbitMQ.Client/client/logging/RabbitMqExceptionDetail.cs

Lines 38 to 66 in a0321c6

	[EventData]
	public class RabbitMqExceptionDetail
	{
	public RabbitMqExceptionDetail(Exception ex)
	{
	Type = ex.GetType().FullName;
	Message = ex.Message;
	StackTrace = ex.StackTrace;
	if (ex.InnerException != null)
	{
	InnerException = ex.InnerException.ToString();
	}
	}
	public RabbitMqExceptionDetail(IDictionary<string, object> ex)
	{
	Type = ex["Type"].ToString();
	Message = ex["Message"].ToString();
	StackTrace = ex["StackTrace"].ToString();
	if (ex.TryGetValue("InnerException", out object inner))
	{
	InnerException = inner.ToString();
	}
	}
	public string Type { get; }
	public string Message { get; }
	public string StackTrace { get; }
	public string InnerException { get; }

This EventSource code is writing a complex object RabbitMqExceptionDetail to an Event. When EventSource sees a complex object, it uses Reflection to get all the properties recursively and gets the values to write to the Event. This is not trimming compatible because the Properties might be trimmed. If they are, the Event data will be different between a trimmed and non-trimmed app.

The AOT/trimming tools produce warnings like the above to let developers know which parts of their code will break when the app is published. You can read more about preparing a library for trimming at https://learn.microsoft.com/dotnet/core/deploying/trimming/prepare-libraries-for-trimming.

Describe the solution you'd like

We should fix the above warning so developers can use RabbitMQ.Client in AOT and trimming apps without warnings, and their apps continue to work

Describe alternatives you've considered

I can think of 2 different approaches to fixing this trimming warning:

1. Use a System.Diagnostics.CodeAnalysis annotation to ensure the properties of RabbitMqExceptionDetail are preserved in a trimmed app and suppress the warning. Something like the following:

        [Event(3, Message = "ERROR", Keywords = Keywords.Log, Level = EventLevel.Error)]
        public void Error(string message, RabbitMqExceptionDetail ex)
        {
            if (IsEnabled())
                WriteExceptionEvent(ex);
                
            [UnconditionalSuppressMessage(...)]
            void WriteExceptionEvent<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] T>(string message, T ex)
            {
                WriteEvent(3, message, ex);
            }
        }

This would ensure the properties are preserved and the warning no longer occurs.

2. When a complex object is written to an EventSource, EventSource under the covers will serialize all the properties to a object[] of primitive values. This same behavior can be simulated by using the WriteEventCore method, which is trim compatible by design. However it takes more code, and we would need to be careful to ensure we don't change the format of the data being emitted from this EventSource event, so we didn't break existing listeners.

My recommendation would be to take approach (1) above.

Additional context

No response

[michaelklishin]

@eerhardt what are the risks of option 1? If we annotation RabbitMqExceptionDetail like suggested, is it guarantee to be trimming-safe?

[eerhardt]

The risks are if RabbitMqExceptionDetail gets updated in the future with a complex property type itself. For example:

 [EventData] 
 public class RabbitMqExceptionDetail 
 { 
    ...
+    public RabbitMqNestedExceptionDetail NestedDetails { get; set; }
}

+public class RabbitMqNestedExceptionDetail
+{
+    public string String1 { get; set; }
+    public string String2 { get; set; }
+}

If that ever happened, we wouldn't get a new trim warning (because we suppressed the warning), but String1 and String2 properties could get trimmed because nothing was telling the trimming tools to preserve them. The [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] attribute only applies to the top-level properties on RabbitMqExceptionDetail - Type, Message, StackTrace, InnerException, NestedDetails. But it doesn't transitively apply to the properties String1 and String2 of RabbitMqNestedExceptionDetail.

So in this scenario, when someone trimmed or AOT'd their app, it would be likely that they wouldn't get event source values written for NestedDetails.String1 and NestedDetails.String2.

[michaelklishin]

Right. I think that risk is fairly low.

[lukebakken]

@eerhardt would you mind checking these lines that I had to add to fix a warning?

https://github.com/rabbitmq/rabbitmq-dotnet-client/pull/1457/files#diff-578fb561d8fb661e0bf3e00dd4a9efeb4c7ad9d6654ab26b8a7aeca19732f6aaR8-R9

It seems as though your trimming changes will only be compatible in newer .NET environments.

Here is the warning:

"C:\Users\lbakken\development\rabbitmq\rabbitmq-dotnet-client_6.x\RabbitMQDotNetClient.sln" (default target) (1:2) ->
"C:\Users\lbakken\development\rabbitmq\rabbitmq-dotnet-client_6.x\projects\RabbitMQ.Client\RabbitMQ.Client.csproj" (default target) (6:17) ->
"C:\Users\lbakken\development\rabbitmq\rabbitmq-dotnet-client_6.x\projects\RabbitMQ.Client\RabbitMQ.Client.csproj" (Build target) (6:18) ->
    C:\Program Files\dotnet\sdk\8.0.100\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(90,5): warning NETSDK1212: IsTrimmable and EnableTrimAnalyzer are not supported for the target framework. Consider multi-targeting to a supported framework to enable trimming, and set Is
Trimmable only for the supported frameworks. For example: [C:\Users\lbakken\development\rabbitmq\rabbitmq-dotnet-client_6.x\projects\RabbitMQ.Client\RabbitMQ.Client.csproj::TargetFramework=net462]
C:\Program Files\dotnet\sdk\8.0.100\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(90,5): warning NETSDK1212: <IsTrimmable Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))">true</IsTrimmable> [C:\Users\lbakken\development\rabbitmq\rabb
itmq-dotnet-client_6.x\projects\RabbitMQ.Client\RabbitMQ.Client.csproj::TargetFramework=net462]


"C:\Users\lbakken\development\rabbitmq\rabbitmq-dotnet-client_6.x\RabbitMQDotNetClient.sln" (default target) (1:2) ->
"C:\Users\lbakken\development\rabbitmq\rabbitmq-dotnet-client_6.x\projects\RabbitMQ.Client\RabbitMQ.Client.csproj" (default target) (6:17) ->
"C:\Users\lbakken\development\rabbitmq\rabbitmq-dotnet-client_6.x\projects\RabbitMQ.Client\RabbitMQ.Client.csproj" (Build target) (6:19) ->
    C:\Program Files\dotnet\sdk\8.0.100\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(90,5): warning NETSDK1212: IsTrimmable and EnableTrimAnalyzer are not supported for the target framework. Consider multi-targeting to a supported framework to enable trimming, and set Is
Trimmable only for the supported frameworks. For example: [C:\Users\lbakken\development\rabbitmq\rabbitmq-dotnet-client_6.x\projects\RabbitMQ.Client\RabbitMQ.Client.csproj::TargetFramework=netstandard2.0]
C:\Program Files\dotnet\sdk\8.0.100\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(90,5): warning NETSDK1212: <IsTrimmable Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))">true</IsTrimmable> [C:\Users\lbakken\development\rabbitmq\rabb
itmq-dotnet-client_6.x\projects\RabbitMQ.Client\RabbitMQ.Client.csproj::TargetFramework=netstandard2.0]

[lukebakken]

@eerhardt it seems like the warning is to alert users that the analyzers are only supported in later .NET versions, not the trimming itself. Going go go with that!

Source - https://blog.martincostello.com/upgrading-to-dotnet-8-part-5-preview-7-and-rc-1-2/

[lukebakken]

Fixed by:

• Trimming and AOT compatibility #1411
• Back-port #1411 to 6.x #1457

[eerhardt]

@lukebakken - Thank you for merging my change and backporting it to 6.x. I apologize for my delay in response - I've been swamped since being back from a break at the end of the year.

The backport to 6.x has some issues. I've opened #1470 to address them. This change will make the 6.x version of the library work in Native AOT'd apps without warnings. I hope it helps.

[lukebakken]

@eerhardt I'm really glad you understand this stuff 😅