From 5120526e5d637e4ea52d1c62deac37d099b7802e Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Wed, 26 Jul 2023 05:34:53 -0700 Subject: [PATCH 1/4] [AOT] Remove usage of Linq.Expressions and especially lambda compilation In AOT `Expression.Lambda.Compile` method is implemented via an interpreter. So it does work, but it's relatively slow. Additionally it's not fully NativeAOT compatible yet. There are two places where it's used in the source code: * `ExceptionProcessor` uses it to access `Marshal.GetExceptionPointers` method which is not part of netstandard API. So in this case simply ifdef and on net6.0+ access the method directly (as it's visible in the net6.0 APIs) and fallback to the Expression.Lambda solution on down-level platforms. Since AOT/trimming is net6.0+ this fixes the problem. * `ActivityInstrumentationHelper` uses it to access two setters on the `Activity` class which are internal. Using reflection and then `CreateDelegate` will provide basically the same performance (a delegate which directly calls the setter) without a need for `Expression.Lambda` - so the code is simpler. Since the reflection is statically analyzable (all types are known, the property names are known) this code is fully trim and AOT compatible. There's no impact on warning count because the .NET 7 ASP.NET has Linq.Expression dependency as well. On .NET 8 though this removes all warnings from Linq.Expressions. --- src/OpenTelemetry/Trace/ExceptionProcessor.cs | 10 +++++++++- src/Shared/ActivityInstrumentationHelper.cs | 16 ++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/OpenTelemetry/Trace/ExceptionProcessor.cs b/src/OpenTelemetry/Trace/ExceptionProcessor.cs index ad6616f5f6..2ce5e79951 100644 --- a/src/OpenTelemetry/Trace/ExceptionProcessor.cs +++ b/src/OpenTelemetry/Trace/ExceptionProcessor.cs @@ -17,9 +17,11 @@ #nullable enable using System.Diagnostics; +using System.Runtime.InteropServices; +#if !NET6_0_OR_GREATER using System.Linq.Expressions; using System.Reflection; -using System.Runtime.InteropServices; +#endif namespace OpenTelemetry.Trace; @@ -31,6 +33,11 @@ internal sealed class ExceptionProcessor : BaseProcessor public ExceptionProcessor() { +#if NET6_0_OR_GREATER + this.fnGetExceptionPointers = Marshal.GetExceptionPointers; +#else + // When running on netstandard and similar the Marshal class it no part of the netstandard API + // but it still most likely available in the underlying framework, so use reflection to get to it. try { var flags = BindingFlags.Static | BindingFlags.Public; @@ -43,6 +50,7 @@ public ExceptionProcessor() { throw new NotSupportedException($"'{typeof(Marshal).FullName}.GetExceptionPointers' is not supported", ex); } +#endif } /// diff --git a/src/Shared/ActivityInstrumentationHelper.cs b/src/Shared/ActivityInstrumentationHelper.cs index ea0b7af05d..0e841fe19d 100644 --- a/src/Shared/ActivityInstrumentationHelper.cs +++ b/src/Shared/ActivityInstrumentationHelper.cs @@ -15,8 +15,6 @@ // using System.Diagnostics; -using System.Linq.Expressions; -using System.Reflection; #pragma warning restore IDE0005 namespace OpenTelemetry.Instrumentation; @@ -28,19 +26,13 @@ internal static class ActivityInstrumentationHelper private static Action CreateActivitySourceSetter() { - ParameterExpression instance = Expression.Parameter(typeof(Activity), "instance"); - ParameterExpression propertyValue = Expression.Parameter(typeof(ActivitySource), "propertyValue"); - PropertyInfo sourcePropertyInfo = typeof(Activity).GetProperty("Source"); - var body = Expression.Assign(Expression.Property(instance, sourcePropertyInfo), propertyValue); - return Expression.Lambda>(body, instance, propertyValue).Compile(); + return (Action)typeof(Activity).GetProperty("Source") + .SetMethod.CreateDelegate(typeof(Action)); } private static Action CreateActivityKindSetter() { - ParameterExpression instance = Expression.Parameter(typeof(Activity), "instance"); - ParameterExpression propertyValue = Expression.Parameter(typeof(ActivityKind), "propertyValue"); - PropertyInfo kindPropertyInfo = typeof(Activity).GetProperty("Kind"); - var body = Expression.Assign(Expression.Property(instance, kindPropertyInfo), propertyValue); - return Expression.Lambda>(body, instance, propertyValue).Compile(); + return (Action)typeof(Activity).GetProperty("Kind") + .SetMethod.CreateDelegate(typeof(Action)); } } From 1fdd8ee1483144afead5d9533c98b65594d64120 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Thu, 27 Jul 2023 17:15:31 -0700 Subject: [PATCH 2/4] Update src/OpenTelemetry/Trace/ExceptionProcessor.cs --- src/OpenTelemetry/Trace/ExceptionProcessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Trace/ExceptionProcessor.cs b/src/OpenTelemetry/Trace/ExceptionProcessor.cs index 2ce5e79951..b6f1dadf1b 100644 --- a/src/OpenTelemetry/Trace/ExceptionProcessor.cs +++ b/src/OpenTelemetry/Trace/ExceptionProcessor.cs @@ -36,7 +36,7 @@ public ExceptionProcessor() #if NET6_0_OR_GREATER this.fnGetExceptionPointers = Marshal.GetExceptionPointers; #else - // When running on netstandard and similar the Marshal class it no part of the netstandard API + // When running on netstandard or similar the Marshal class is not a part of the netstandard API // but it still most likely available in the underlying framework, so use reflection to get to it. try { From d041885cae431e6f500e9fa032524b6d2d1da34f Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Thu, 27 Jul 2023 17:15:46 -0700 Subject: [PATCH 3/4] Update src/OpenTelemetry/Trace/ExceptionProcessor.cs --- src/OpenTelemetry/Trace/ExceptionProcessor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Trace/ExceptionProcessor.cs b/src/OpenTelemetry/Trace/ExceptionProcessor.cs index b6f1dadf1b..6de3e96929 100644 --- a/src/OpenTelemetry/Trace/ExceptionProcessor.cs +++ b/src/OpenTelemetry/Trace/ExceptionProcessor.cs @@ -37,7 +37,7 @@ public ExceptionProcessor() this.fnGetExceptionPointers = Marshal.GetExceptionPointers; #else // When running on netstandard or similar the Marshal class is not a part of the netstandard API - // but it still most likely available in the underlying framework, so use reflection to get to it. + // but it would still most likely be available in the underlying framework, so use reflection to retrieve it. try { var flags = BindingFlags.Static | BindingFlags.Public; From 0a2a1052bd19e030c6bdb4d5197760936c51f7c1 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Fri, 28 Jul 2023 02:19:23 -0700 Subject: [PATCH 4/4] Use the API directly even on netfx --- src/OpenTelemetry/Trace/ExceptionProcessor.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry/Trace/ExceptionProcessor.cs b/src/OpenTelemetry/Trace/ExceptionProcessor.cs index 6de3e96929..17023d46c8 100644 --- a/src/OpenTelemetry/Trace/ExceptionProcessor.cs +++ b/src/OpenTelemetry/Trace/ExceptionProcessor.cs @@ -18,7 +18,7 @@ using System.Diagnostics; using System.Runtime.InteropServices; -#if !NET6_0_OR_GREATER +#if !NET6_0_OR_GREATER && !NETFRAMEWORK using System.Linq.Expressions; using System.Reflection; #endif @@ -33,7 +33,7 @@ internal sealed class ExceptionProcessor : BaseProcessor public ExceptionProcessor() { -#if NET6_0_OR_GREATER +#if NET6_0_OR_GREATER || NETFRAMEWORK this.fnGetExceptionPointers = Marshal.GetExceptionPointers; #else // When running on netstandard or similar the Marshal class is not a part of the netstandard API