From a2e64cde01e1a99616e4da9298f30cb32eb2c123 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Fri, 15 Sep 2023 11:51:46 -0500 Subject: [PATCH 1/3] Address AOT warnings in Exporter.OpenTelemetryProtocol This addresses the AOT warnings in the last library in this opentelemetry-dotnet. 1. Google.Protobuf v3.19.4 isn't trimming and AOT compatible. We need to upgrade to a newer version. I chose the latest patch in v3.22.x. v3.22.0 was the first version that is AOT compatible. 2. There were 2 places in Exporter.OpenTelemetryProtocol that was using System.Reflection.Emit to set a private field on Protobuf's RepeatedField class in order to return it to a pool. Setting this private field is no longer necessary since v3.22.0 because the Clear method was updated to support this scenario. See https://github.com/protocolbuffers/protobuf/commit/a4fd21618635b891fefe34d5e440d6d9464e659f Fix #3429 --- Directory.Packages.props | 2 +- .../Implementation/ActivityExtensions.cs | 27 +------------------ .../Implementation/MetricItemExtensions.cs | 26 +----------------- ...nTelemetry.AotCompatibility.TestApp.csproj | 1 + 4 files changed, 4 insertions(+), 52 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 4b2193facb7..f64afc245ae 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -13,7 +13,7 @@ vulnerability in the NuGet packages that are published from this repository. --> - + diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs index b4b2d2f4204..e967cef97e6 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs @@ -16,11 +16,8 @@ using System.Collections.Concurrent; using System.Diagnostics; -using System.Reflection; -using System.Reflection.Emit; using System.Runtime.CompilerServices; using Google.Protobuf; -using Google.Protobuf.Collections; using OpenTelemetry.Internal; using OpenTelemetry.Proto.Collector.Trace.V1; using OpenTelemetry.Proto.Common.V1; @@ -34,7 +31,6 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation; internal static class ActivityExtensions { private static readonly ConcurrentBag SpanListPool = new(); - private static readonly Action, int> RepeatedFieldOfSpanSetCountAction = CreateRepeatedFieldOfSpanSetCountAction(); internal static void AddBatch( this ExportTraceServiceRequest request, @@ -84,7 +80,7 @@ internal static void Return(this ExportTraceServiceRequest request) foreach (var scope in resourceSpans.ScopeSpans) { - RepeatedFieldOfSpanSetCountAction(scope.Spans, 0); + scope.Spans.Clear(); SpanListPool.Add(scope); } } @@ -298,27 +294,6 @@ private static Span.Types.Event ToOtlpEvent(in ActivityEvent activityEvent, SdkL return otlpEvent; } - private static Action, int> CreateRepeatedFieldOfSpanSetCountAction() - { - FieldInfo repeatedFieldOfSpanCountField = typeof(RepeatedField).GetField("count", BindingFlags.NonPublic | BindingFlags.Instance); - - DynamicMethod dynamicMethod = new DynamicMethod( - "CreateSetCountAction", - null, - new[] { typeof(RepeatedField), typeof(int) }, - typeof(ActivityExtensions).Module, - skipVisibility: true); - - var generator = dynamicMethod.GetILGenerator(); - - generator.Emit(OpCodes.Ldarg_0); - generator.Emit(OpCodes.Ldarg_1); - generator.Emit(OpCodes.Stfld, repeatedFieldOfSpanCountField); - generator.Emit(OpCodes.Ret); - - return (Action, int>)dynamicMethod.CreateDelegate(typeof(Action, int>)); - } - private struct TagEnumerationState : PeerServiceResolver.IPeerServiceState { public SdkLimitOptions SdkLimitOptions; diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs index 3f7cd218d42..9c20d38cb78 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs @@ -15,8 +15,6 @@ // using System.Collections.Concurrent; -using System.Reflection; -using System.Reflection.Emit; using System.Runtime.CompilerServices; using Google.Protobuf; using Google.Protobuf.Collections; @@ -31,7 +29,6 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation; internal static class MetricItemExtensions { private static readonly ConcurrentBag MetricListPool = new(); - private static readonly Action, int> RepeatedFieldOfMetricSetCountAction = CreateRepeatedFieldOfMetricSetCountAction(); internal static void AddMetrics( this OtlpCollector.ExportMetricsServiceRequest request, @@ -82,7 +79,7 @@ internal static void Return(this OtlpCollector.ExportMetricsServiceRequest reque foreach (var scope in resourceMetrics.ScopeMetrics) { - RepeatedFieldOfMetricSetCountAction(scope.Metrics, 0); + scope.Metrics.Clear(); MetricListPool.Add(scope); } } @@ -418,25 +415,4 @@ private static OtlpMetrics.Exemplar ToOtlpExemplar(this IExemplar exemplar) return otlpExemplar; } */ - - private static Action, int> CreateRepeatedFieldOfMetricSetCountAction() - { - FieldInfo repeatedFieldOfMetricCountField = typeof(RepeatedField).GetField("count", BindingFlags.NonPublic | BindingFlags.Instance); - - DynamicMethod dynamicMethod = new DynamicMethod( - "CreateSetCountAction", - null, - new[] { typeof(RepeatedField), typeof(int) }, - typeof(MetricItemExtensions).Module, - skipVisibility: true); - - var generator = dynamicMethod.GetILGenerator(); - - generator.Emit(OpCodes.Ldarg_0); - generator.Emit(OpCodes.Ldarg_1); - generator.Emit(OpCodes.Stfld, repeatedFieldOfMetricCountField); - generator.Emit(OpCodes.Ret); - - return (Action, int>)dynamicMethod.CreateDelegate(typeof(Action, int>)); - } } diff --git a/test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj b/test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj index 3a2e891f913..4d82290d9a1 100644 --- a/test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj +++ b/test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj @@ -17,6 +17,7 @@ + From 0fd1bb6a145ec00f2bac574ee6fe87f73cbd2a9a Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Tue, 19 Sep 2023 10:38:09 -0500 Subject: [PATCH 2/3] Respond to PR feedback. - Add Changelog - Remove unneeded reference to System.Reflection.Emit.Lightweight --- Directory.Packages.props | 3 --- .../CHANGELOG.md | 6 ++++++ .../OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj | 1 - 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index a07e5b70e83..9e283fcd84a 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -41,9 +41,6 @@ --> - - - diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md index c6f246d2d7d..d6bc3b6ed7e 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +* Bumped the version of `Google.Protobuf` used by the project to `3.22.5` so + that consuming applications can be NativeAOT published successfully. Also, + a new performance feature can be used instead of reflection emit, which is + not AOT-compatible. Removed the dependency on `System.Reflection.Emit.Lightweight`. + ([#4859](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4859)) + ## 1.6.0 Released 2023-Sep-05 diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj index ba0bde3424b..ecd759f4ea6 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj @@ -19,7 +19,6 @@ - From e194f5e1618ab4e144b5b94b6b73f0c4db38c690 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Tue, 19 Sep 2023 10:40:31 -0500 Subject: [PATCH 3/3] fix changelog --- src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md index d6bc3b6ed7e..f0f9461eff6 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md @@ -3,7 +3,7 @@ ## Unreleased * Bumped the version of `Google.Protobuf` used by the project to `3.22.5` so - that consuming applications can be NativeAOT published successfully. Also, + that consuming applications can be published as NativeAOT successfully. Also, a new performance feature can be used instead of reflection emit, which is not AOT-compatible. Removed the dependency on `System.Reflection.Emit.Lightweight`. ([#4859](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4859))