From 1c7fd9afa4d2f0d68f879b4c69a631725a94c577 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai Date: Sat, 25 Feb 2023 06:52:17 -0800 Subject: [PATCH] Remove shared code using `DynamicMethod` (#1041) --- build/Common.props | 3 - .../Api/ActivityHelperExtensions.cs | 108 ----------- .../Api/EnumerationHelper.cs | 179 ------------------ .../Api/IActivityEnumerator.cs | 33 ---- .../OpenTelemetry.Contrib.Shared.csproj | 3 +- ...Telemetry.Instrumentation.MySqlData.csproj | 3 - .../ActivityHelperExtensions.cs | 37 ++++ .../AWSLambdaWrapperTests.cs | 1 + ...try.Instrumentation.AWSLambda.Tests.csproj | 4 + ...emetry.Instrumentation.AspNet.Tests.csproj | 4 +- ...mentation.ElasticsearchClient.Tests.csproj | 6 +- ...try.Instrumentation.MySqlData.Tests.csproj | 6 +- ...isProfilerEntryToActivityConverterTests.cs | 1 + ...umentation.StackExchangeRedis.Tests.csproj | 1 + 14 files changed, 56 insertions(+), 333 deletions(-) delete mode 100644 src/OpenTelemetry.Contrib.Shared/Api/ActivityHelperExtensions.cs delete mode 100644 src/OpenTelemetry.Contrib.Shared/Api/EnumerationHelper.cs delete mode 100644 src/OpenTelemetry.Contrib.Shared/Api/IActivityEnumerator.cs create mode 100644 test/OpenTelemetry.Contrib.Tests.Shared/ActivityHelperExtensions.cs diff --git a/build/Common.props b/build/Common.props index 469e35cdbe..1e7bfad803 100644 --- a/build/Common.props +++ b/build/Common.props @@ -57,10 +57,7 @@ - - - diff --git a/src/OpenTelemetry.Contrib.Shared/Api/ActivityHelperExtensions.cs b/src/OpenTelemetry.Contrib.Shared/Api/ActivityHelperExtensions.cs deleted file mode 100644 index 7e49b4d713..0000000000 --- a/src/OpenTelemetry.Contrib.Shared/Api/ActivityHelperExtensions.cs +++ /dev/null @@ -1,108 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System.Collections.Generic; -using System.Diagnostics; -using System.Reflection; -using System.Runtime.CompilerServices; -using OpenTelemetry.Internal; - -namespace OpenTelemetry.Trace; - -/// -/// Extension methods on Activity. -/// -internal static class ActivityHelperExtensions -{ - /// - /// Gets the value of a specific tag on an . - /// - /// Activity instance. - /// Case-sensitive tag name to retrieve. - /// Tag value or null if a match was not found. - [MethodImpl(MethodImplOptions.AggressiveInlining)] - [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "ActivityProcessor is hot path")] - public static object GetTagValue(this Activity activity, string tagName) - { - Debug.Assert(activity != null, "Activity should not be null"); - - // TODO: Update to use - // https://docs.microsoft.com/dotnet/api/system.diagnostics.activity.enumeratetagobjects?view=net-7.0 - // instead of reflection. See: - // https://github.com/open-telemetry/opentelemetry-dotnet/blob/928d77056c1d353d8ba72ad3b4b565398117b352/src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs#L85-L98 - - ActivitySingleTagEnumerator state = new ActivitySingleTagEnumerator(tagName); - - ActivityTagsEnumeratorFactory.Enumerate(activity, ref state); - - return state.Value; - } - - private struct ActivitySingleTagEnumerator : IActivityEnumerator> - { - public object Value; - - private readonly string tagName; - - public ActivitySingleTagEnumerator(string tagName) - { - this.tagName = tagName; - this.Value = null; - } - - public bool ForEach(KeyValuePair item) - { - if (item.Key == this.tagName) - { - this.Value = item.Value; - return false; - } - - return true; - } - } - - private static class ActivityTagsEnumeratorFactory - where TState : struct, IActivityEnumerator> - { - private static readonly object EmptyActivityTagObjects = typeof(Activity).GetField("s_emptyTagObjects", BindingFlags.Static | BindingFlags.NonPublic).GetValue(null); - - private static readonly DictionaryEnumerator.AllocationFreeForEachDelegate - ActivityTagObjectsEnumerator = DictionaryEnumerator.BuildAllocationFreeForEachDelegate( - typeof(Activity).GetField("_tags", BindingFlags.Instance | BindingFlags.NonPublic).FieldType); - - private static readonly DictionaryEnumerator.ForEachDelegate ForEachTagValueCallbackRef = ForEachTagValueCallback; - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Enumerate(Activity activity, ref TState state) - { - var tagObjects = activity.TagObjects; - - if (ReferenceEquals(tagObjects, EmptyActivityTagObjects)) - { - return; - } - - ActivityTagObjectsEnumerator( - tagObjects, - ref state, - ForEachTagValueCallbackRef); - } - - private static bool ForEachTagValueCallback(ref TState state, KeyValuePair item) - => state.ForEach(item); - } -} diff --git a/src/OpenTelemetry.Contrib.Shared/Api/EnumerationHelper.cs b/src/OpenTelemetry.Contrib.Shared/Api/EnumerationHelper.cs deleted file mode 100644 index dbae60c27d..0000000000 --- a/src/OpenTelemetry.Contrib.Shared/Api/EnumerationHelper.cs +++ /dev/null @@ -1,179 +0,0 @@ -// -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -using System; -using System.Collections; -using System.Collections.Generic; -using System.Reflection; -using System.Reflection.Emit; - -namespace OpenTelemetry.Internal; - -internal class DictionaryEnumerator : Enumerator ->, - KeyValuePair, - TState> - where TState : struct -{ - protected DictionaryEnumerator() - { - } -} - -internal class ListEnumerator : Enumerator -, - TValue, - TState> - where TState : struct -{ - protected ListEnumerator() - { - } -} - -// A helper class for enumerating over IEnumerable without allocation if a struct enumerator is available. -internal class Enumerator - where TEnumerable : IEnumerable - where TState : struct -{ - private static readonly MethodInfo GenericGetEnumeratorMethod = typeof(IEnumerable).GetMethod("GetEnumerator"); - private static readonly MethodInfo GeneircCurrentGetMethod = typeof(IEnumerator).GetProperty("Current").GetMethod; - private static readonly MethodInfo MoveNextMethod = typeof(IEnumerator).GetMethod("MoveNext"); - private static readonly MethodInfo DisposeMethod = typeof(IDisposable).GetMethod("Dispose"); - - public delegate void AllocationFreeForEachDelegate(TEnumerable instance, ref TState state, ForEachDelegate itemCallback); - - public delegate bool ForEachDelegate(ref TState state, TItem item); - - protected Enumerator() - { - } - - /* We want to do this type of logic... - public static void AllocationFreeForEach(Dictionary dictionary, ref TState state, ForEachDelegate itemCallback) - { - using (Dictionary.Enumerator enumerator = dictionary.GetEnumerator()) - { - while (enumerator.MoveNext()) - { - if (!itemCallback(ref state, enumerator.Current)) - break; - } - } - } - ...because it takes advantage of the struct Enumerator on the built-in types which give an allocation-free way to enumerate. - */ - public static AllocationFreeForEachDelegate BuildAllocationFreeForEachDelegate(Type enumerableType) - { - var itemCallbackType = typeof(ForEachDelegate); - - var getEnumeratorMethod = ResolveGetEnumeratorMethodForType(enumerableType); - if (getEnumeratorMethod == null) - { - // Fallback to allocation mode and use IEnumerable.GetEnumerator. - // Primarily for Array.Empty and Enumerable.Empty case, but also for user types. - getEnumeratorMethod = GenericGetEnumeratorMethod; - } - - var enumeratorType = getEnumeratorMethod.ReturnType; - - var dynamicMethod = new DynamicMethod( - nameof(AllocationFreeForEachDelegate), - null, - new[] { typeof(TEnumerable), typeof(TState).MakeByRefType(), itemCallbackType }, - typeof(AllocationFreeForEachDelegate).Module, - skipVisibility: true); - - var generator = dynamicMethod.GetILGenerator(); - - generator.DeclareLocal(enumeratorType); - - var beginLoopLabel = generator.DefineLabel(); - var processCurrentLabel = generator.DefineLabel(); - var returnLabel = generator.DefineLabel(); - var breakLoopLabel = generator.DefineLabel(); - - generator.Emit(OpCodes.Ldarg_0); - generator.Emit(OpCodes.Callvirt, getEnumeratorMethod); - generator.Emit(OpCodes.Stloc_0); - - // try - generator.BeginExceptionBlock(); - { - generator.Emit(OpCodes.Br_S, beginLoopLabel); - - generator.MarkLabel(processCurrentLabel); - - generator.Emit(OpCodes.Ldarg_2); - generator.Emit(OpCodes.Ldarg_1); - generator.Emit(OpCodes.Ldloca_S, 0); - generator.Emit(OpCodes.Constrained, enumeratorType); - generator.Emit(OpCodes.Callvirt, GeneircCurrentGetMethod); - - generator.Emit(OpCodes.Callvirt, itemCallbackType.GetMethod("Invoke")); - - generator.Emit(OpCodes.Brtrue_S, beginLoopLabel); - - generator.Emit(OpCodes.Leave_S, returnLabel); - - generator.MarkLabel(beginLoopLabel); - - generator.Emit(OpCodes.Ldloca_S, 0); - generator.Emit(OpCodes.Constrained, enumeratorType); - generator.Emit(OpCodes.Callvirt, MoveNextMethod); - - generator.Emit(OpCodes.Brtrue_S, processCurrentLabel); - - generator.MarkLabel(breakLoopLabel); - - generator.Emit(OpCodes.Leave_S, returnLabel); - } - - // finally - generator.BeginFinallyBlock(); - { - if (typeof(IDisposable).IsAssignableFrom(enumeratorType)) - { - generator.Emit(OpCodes.Ldloca_S, 0); - generator.Emit(OpCodes.Constrained, enumeratorType); - generator.Emit(OpCodes.Callvirt, DisposeMethod); - } - } - - generator.EndExceptionBlock(); - - generator.MarkLabel(returnLabel); - - generator.Emit(OpCodes.Ret); - - return (AllocationFreeForEachDelegate)dynamicMethod.CreateDelegate(typeof(AllocationFreeForEachDelegate)); - } - - private static MethodInfo ResolveGetEnumeratorMethodForType(Type type) - { - var methods = type.GetMethods(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); - - foreach (var method in methods) - { - if (method.Name == "GetEnumerator" && !method.ReturnType.IsInterface) - { - return method; - } - } - - return null; - } -} diff --git a/src/OpenTelemetry.Contrib.Shared/Api/IActivityEnumerator.cs b/src/OpenTelemetry.Contrib.Shared/Api/IActivityEnumerator.cs deleted file mode 100644 index 3a55f61399..0000000000 --- a/src/OpenTelemetry.Contrib.Shared/Api/IActivityEnumerator.cs +++ /dev/null @@ -1,33 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System.Diagnostics; - -namespace OpenTelemetry.Trace; - -/// -/// An interface used to perform zero-allocation enumeration of elements. Implementation must be a struct. -/// -/// Enumerated item type. -internal interface IActivityEnumerator -{ - /// - /// Called for each item while the enumeration is executing. - /// - /// Enumeration item. - /// to continue the enumeration of records or to stop (break) the enumeration. - bool ForEach(T item); -} diff --git a/src/OpenTelemetry.Contrib.Shared/OpenTelemetry.Contrib.Shared.csproj b/src/OpenTelemetry.Contrib.Shared/OpenTelemetry.Contrib.Shared.csproj index 5bfdb9cac6..adb7aad559 100644 --- a/src/OpenTelemetry.Contrib.Shared/OpenTelemetry.Contrib.Shared.csproj +++ b/src/OpenTelemetry.Contrib.Shared/OpenTelemetry.Contrib.Shared.csproj @@ -5,8 +5,7 @@ - - + diff --git a/src/OpenTelemetry.Instrumentation.MySqlData/OpenTelemetry.Instrumentation.MySqlData.csproj b/src/OpenTelemetry.Instrumentation.MySqlData/OpenTelemetry.Instrumentation.MySqlData.csproj index 322698ecec..83b39ef2f4 100644 --- a/src/OpenTelemetry.Instrumentation.MySqlData/OpenTelemetry.Instrumentation.MySqlData.csproj +++ b/src/OpenTelemetry.Instrumentation.MySqlData/OpenTelemetry.Instrumentation.MySqlData.csproj @@ -14,9 +14,6 @@ - - - diff --git a/test/OpenTelemetry.Contrib.Tests.Shared/ActivityHelperExtensions.cs b/test/OpenTelemetry.Contrib.Tests.Shared/ActivityHelperExtensions.cs new file mode 100644 index 0000000000..c4fec75bd4 --- /dev/null +++ b/test/OpenTelemetry.Contrib.Tests.Shared/ActivityHelperExtensions.cs @@ -0,0 +1,37 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Diagnostics; + +namespace OpenTelemetry.Tests; + +internal static class ActivityHelperExtensions +{ + public static object GetTagValue(this Activity activity, string tagName) + { + Debug.Assert(activity != null, "Activity should not be null"); + + foreach (var tag in activity.TagObjects) + { + if (tag.Key == tagName) + { + return tag.Value; + } + } + + return null; + } +} diff --git a/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/AWSLambdaWrapperTests.cs b/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/AWSLambdaWrapperTests.cs index 9d4ace50ce..b05b34c12a 100644 --- a/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/AWSLambdaWrapperTests.cs +++ b/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/AWSLambdaWrapperTests.cs @@ -22,6 +22,7 @@ using Moq; using OpenTelemetry.Instrumentation.AWSLambda.Implementation; using OpenTelemetry.Resources; +using OpenTelemetry.Tests; using OpenTelemetry.Trace; using Xunit; diff --git a/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/OpenTelemetry.Instrumentation.AWSLambda.Tests.csproj b/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/OpenTelemetry.Instrumentation.AWSLambda.Tests.csproj index a98b7b9207..ecc88c9a31 100644 --- a/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/OpenTelemetry.Instrumentation.AWSLambda.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.AWSLambda.Tests/OpenTelemetry.Instrumentation.AWSLambda.Tests.csproj @@ -21,4 +21,8 @@ + + + + diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj b/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj index 1809ae8dbf..92750cb40e 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj @@ -28,9 +28,7 @@ - - - + diff --git a/test/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests.csproj b/test/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests.csproj index 744c556d99..853bf851a4 100644 --- a/test/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests/OpenTelemetry.Instrumentation.ElasticsearchClient.Tests.csproj @@ -1,4 +1,4 @@ - + Unit test project for OpenTelemetry Elasticsearch client instrumentation @@ -23,4 +23,8 @@ + + + + diff --git a/test/OpenTelemetry.Instrumentation.MySqlData.Tests/OpenTelemetry.Instrumentation.MySqlData.Tests.csproj b/test/OpenTelemetry.Instrumentation.MySqlData.Tests/OpenTelemetry.Instrumentation.MySqlData.Tests.csproj index 5e59fbcbe0..e24d8f3916 100644 --- a/test/OpenTelemetry.Instrumentation.MySqlData.Tests/OpenTelemetry.Instrumentation.MySqlData.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.MySqlData.Tests/OpenTelemetry.Instrumentation.MySqlData.Tests.csproj @@ -1,4 +1,4 @@ - + netcoreapp3.1 @@ -23,4 +23,8 @@ + + + + diff --git a/test/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests/Implementation/RedisProfilerEntryToActivityConverterTests.cs b/test/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests/Implementation/RedisProfilerEntryToActivityConverterTests.cs index f718265990..784f9a528b 100644 --- a/test/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests/Implementation/RedisProfilerEntryToActivityConverterTests.cs +++ b/test/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests/Implementation/RedisProfilerEntryToActivityConverterTests.cs @@ -19,6 +19,7 @@ using System.Net; using System.Net.Sockets; using Moq; +using OpenTelemetry.Tests; using OpenTelemetry.Trace; using StackExchange.Redis; using StackExchange.Redis.Profiling; diff --git a/test/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests.csproj b/test/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests.csproj index 9d0d48e8a8..95b848e959 100644 --- a/test/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests/OpenTelemetry.Instrumentation.StackExchangeRedis.Tests.csproj @@ -8,6 +8,7 @@ +