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.