diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index ab72592c345..dc340f83d42 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -9,6 +9,10 @@ * Added `EnumerateEvents` extension method on `Activity` for retrieving events efficiently ([#1319](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1319)) +* Added `EnumerateTags` extension methods on `ActivityLink` & `ActivityEvent` + for retrieving tags efficiently. Renamed `Activity.EnumerateTagValues` -> + `Activity.EnumerateTags`. + ([#1320](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1320)) ## 0.6.0-beta.1 diff --git a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs index fa832eee989..8ad2875161e 100644 --- a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs +++ b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs @@ -64,7 +64,7 @@ public static Status GetStatus(this Activity activity) ActivityStatusTagEnumerator state = default; - ActivityTagObjectsEnumeratorFactory.Enumerate(activity, ref state); + ActivityTagsEnumeratorFactory.Enumerate(activity, ref state); var status = SpanHelper.ResolveCanonicalCodeToStatus(state.StatusCode); @@ -90,7 +90,7 @@ public static object GetTagValue(this Activity activity, string tagName) ActivitySingleTagEnumerator state = new ActivitySingleTagEnumerator(tagName); - ActivityTagObjectsEnumeratorFactory.Enumerate(activity, ref state); + ActivityTagsEnumeratorFactory.Enumerate(activity, ref state); return state.Value; } @@ -103,12 +103,12 @@ public static object GetTagValue(this Activity activity, string tagName) /// Tag enumerator. [MethodImpl(MethodImplOptions.AggressiveInlining)] [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "ActivityProcessor is hot path")] - public static void EnumerateTagValues(this Activity activity, ref T tagEnumerator) + public static void EnumerateTags(this Activity activity, ref T tagEnumerator) where T : struct, IActivityEnumerator> { Debug.Assert(activity != null, "Activity should not be null"); - ActivityTagObjectsEnumeratorFactory.Enumerate(activity, ref tagEnumerator); + ActivityTagsEnumeratorFactory.Enumerate(activity, ref tagEnumerator); } /// @@ -127,6 +127,20 @@ public static void EnumerateLinks(this Activity activity, ref T linkEnumerato ActivityLinksEnumeratorFactory.Enumerate(activity, ref linkEnumerator); } + /// + /// Enumerates all the key/value pairs on an without performing an allocation. + /// + /// The struct implementation to use for the enumeration. + /// ActivityLink instance. + /// Tag enumerator. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "ActivityProcessor is hot path")] + public static void EnumerateTags(this ActivityLink activityLink, ref T tagEnumerator) + where T : struct, IActivityEnumerator> + { + ActivityTagsEnumeratorFactory.Enumerate(activityLink, ref tagEnumerator); + } + /// /// Enumerates all the s on an without performing an allocation. /// @@ -143,6 +157,20 @@ public static void EnumerateEvents(this Activity activity, ref T eventEnumera ActivityEventsEnumeratorFactory.Enumerate(activity, ref eventEnumerator); } + /// + /// Enumerates all the key/value pairs on an without performing an allocation. + /// + /// The struct implementation to use for the enumeration. + /// ActivityEvent instance. + /// Tag enumerator. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "ActivityProcessor is hot path")] + public static void EnumerateTags(this ActivityEvent activityEvent, ref T tagEnumerator) + where T : struct, IActivityEnumerator> + { + ActivityTagsEnumeratorFactory.Enumerate(activityEvent, ref tagEnumerator); + } + /// /// Record Exception. /// @@ -216,15 +244,20 @@ public bool ForEach(KeyValuePair item) } } - private static class ActivityTagObjectsEnumeratorFactory + 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 object EmptyActivityEventTags = typeof(ActivityEvent).GetField("s_emptyTags", 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.AllocationFreeForEachDelegate + ActivityTagsCollectionEnumerator = DictionaryEnumerator.BuildAllocationFreeForEachDelegate(typeof(ActivityTagsCollection)); + private static readonly DictionaryEnumerator.ForEachDelegate ForEachTagValueCallbackRef = ForEachTagValueCallback; [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -243,6 +276,38 @@ public static void Enumerate(Activity activity, ref TState state) ForEachTagValueCallbackRef); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void Enumerate(ActivityLink activityLink, ref TState state) + { + var tags = activityLink.Tags; + + if (tags is null) + { + return; + } + + ActivityTagsCollectionEnumerator( + tags, + ref state, + ForEachTagValueCallbackRef); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void Enumerate(ActivityEvent activityEvent, ref TState state) + { + var tags = activityEvent.Tags; + + if (ReferenceEquals(tags, EmptyActivityEventTags)) + { + return; + } + + ActivityTagsCollectionEnumerator( + tags, + ref state, + ForEachTagValueCallbackRef); + } + private static bool ForEachTagValueCallback(ref TState state, KeyValuePair item) => state.ForEach(item); } diff --git a/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs b/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs index 982e3f7c6fb..5d18473674f 100644 --- a/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs +++ b/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs @@ -50,8 +50,6 @@ internal static class JaegerActivityExtensions [SemanticConventions.AttributeDbInstance] = 2, // peer.service for Redis. }; - private static readonly DictionaryEnumerator>.ForEachDelegate ProcessTagRef = ProcessTag; - public static JaegerSpan ToJaegerSpan(this Activity activity) { var jaegerTags = new TagEnumerationState @@ -59,7 +57,7 @@ public static JaegerSpan ToJaegerSpan(this Activity activity) Tags = PooledList.Create(), }; - activity.EnumerateTagValues(ref jaegerTags); + activity.EnumerateTags(ref jaegerTags); string peerServiceName = null; if (activity.Kind == ActivityKind.Client || activity.Kind == ActivityKind.Producer) @@ -178,24 +176,20 @@ public static PooledList ToJaegerLogs(this Activity activity) public static JaegerLog ToJaegerLog(this ActivityEvent timedEvent) { - var tags = new PooledListState + var jaegerTags = new EventTagsEnumerationState { - Created = true, - List = PooledList.Create(), + Tags = PooledList.Create(), }; - DictionaryEnumerator>.AllocationFreeForEach( - timedEvent.Tags, - ref tags, - ProcessTagRef); + timedEvent.EnumerateTags(ref jaegerTags); // Matches what OpenTracing and OpenTelemetry defines as the event name. // https://github.com/opentracing/specification/blob/master/semantic_conventions.md#log-fields-table // https://github.com/open-telemetry/opentelemetry-specification/pull/397/files - PooledList.Add(ref tags.List, new JaegerTag("message", JaegerTagType.STRING, vStr: timedEvent.Name)); + PooledList.Add(ref jaegerTags.Tags, new JaegerTag("message", JaegerTagType.STRING, vStr: timedEvent.Name)); // TODO: Use the same function as JaegerConversionExtensions or check that the perf here is acceptable. - return new JaegerLog(timedEvent.Timestamp.ToEpochMicroseconds(), tags.List); + return new JaegerLog(timedEvent.Timestamp.ToEpochMicroseconds(), jaegerTags.Tags); } public static JaegerSpanRef ToJaegerSpanRef(this in ActivityLink link) @@ -306,20 +300,6 @@ private static void ProcessJaegerTag(ref TagEnumerationState state, string key, PooledList.Add(ref state.Tags, jaegerTag); } - private static bool ProcessTag(ref PooledListState state, KeyValuePair attribute) - { - if (attribute.Value is Array) - { - ProcessJaegerTagArray(ref state.List, attribute); - } - else if (attribute.Value != null) - { - PooledList.Add(ref state.List, attribute.ToJaegerTag()); - } - - return true; - } - private struct TagEnumerationState : IActivityEnumerator> { public PooledList Tags; @@ -389,11 +369,23 @@ public bool ForEach(ActivityEvent activityEvent) } } - private struct PooledListState + private struct EventTagsEnumerationState : IActivityEnumerator> { - public bool Created; + public PooledList Tags; + + public bool ForEach(KeyValuePair tag) + { + if (tag.Value is Array) + { + ProcessJaegerTagArray(ref this.Tags, tag); + } + else if (tag.Value != null) + { + PooledList.Add(ref this.Tags, tag.ToJaegerTag()); + } - public PooledList List; + return true; + } } } } diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs index 39349bc1cc8..a36c5dc04bc 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs @@ -64,7 +64,7 @@ internal static ZipkinSpan ToZipkinSpan(this Activity activity, ZipkinEndpoint d Tags = PooledList>.Create(), }; - activity.EnumerateTagValues(ref tagState); + activity.EnumerateTags(ref tagState); var activitySource = activity.Source; if (!string.IsNullOrEmpty(activitySource.Name)) diff --git a/src/OpenTelemetry/Internal/EnumerationHelper.cs b/src/OpenTelemetry/Internal/EnumerationHelper.cs index 7b6d346c690..0c9abc43c95 100644 --- a/src/OpenTelemetry/Internal/EnumerationHelper.cs +++ b/src/OpenTelemetry/Internal/EnumerationHelper.cs @@ -1,4 +1,4 @@ -// +// // // Copyright The OpenTelemetry Authors // @@ -16,9 +16,7 @@ // using System; using System.Collections; -using System.Collections.Concurrent; using System.Collections.Generic; -using System.Diagnostics; using System.Reflection; using System.Reflection.Emit; @@ -55,8 +53,6 @@ internal class Enumerator 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"); - private static readonly ConcurrentDictionary AllocationFreeForEachDelegates = new ConcurrentDictionary(); - private static readonly Func BuildAllocationFreeForEachDelegateRef = BuildAllocationFreeForEachDelegate; public delegate void AllocationFreeForEachDelegate(TEnumerable instance, ref TState state, ForEachDelegate itemCallback); @@ -66,19 +62,6 @@ protected Enumerator() { } - public static void AllocationFreeForEach(TEnumerable instance, ref TState state, ForEachDelegate itemCallback) - { - Debug.Assert(instance != null && itemCallback != null); - - var type = instance.GetType(); - - var allocationFreeForEachDelegate = AllocationFreeForEachDelegates.GetOrAdd( - type, - BuildAllocationFreeForEachDelegateRef); - - allocationFreeForEachDelegate(instance, ref state, itemCallback); - } - /* We want to do this type of logic... public static void AllocationFreeForEach(Dictionary dictionary, ref TState state, ForEachDelegate itemCallback) { @@ -108,7 +91,7 @@ public static AllocationFreeForEachDelegate BuildAllocationFreeForEachDelegate(T var enumeratorType = getEnumeratorMethod.ReturnType; var dynamicMethod = new DynamicMethod( - nameof(AllocationFreeForEach), + nameof(AllocationFreeForEachDelegate), null, new[] { typeof(TEnumerable), typeof(TState).MakeByRefType(), itemCallbackType }, typeof(AllocationFreeForEachDelegate).Module, diff --git a/test/Benchmarks/ActivityBenchmarks.cs b/test/Benchmarks/ActivityBenchmarks.cs index e6d66b0f548..a119b71e38f 100644 --- a/test/Benchmarks/ActivityBenchmarks.cs +++ b/test/Benchmarks/ActivityBenchmarks.cs @@ -27,6 +27,8 @@ public class ActivityBenchmarks { private static readonly Activity EmptyActivity = new Activity("EmptyActivity"); private static readonly Activity Activity; + private static readonly ActivityLink ActivityLink; + private static readonly ActivityEvent ActivityEvent; static ActivityBenchmarks() { @@ -39,11 +41,20 @@ static ActivityBenchmarks() Sample = (ref ActivityCreationOptions options) => ActivitySamplingResult.AllData, }); + var activityTagCollection = new ActivityTagsCollection(new Dictionary + { + ["tag1"] = "value1", + ["tag2"] = "value2", + ["tag3"] = "value3", + }); + + ActivityLink = new ActivityLink(default, activityTagCollection); + var activityLinks = new[] { - new ActivityLink(default), - new ActivityLink(default), - new ActivityLink(default), + ActivityLink, + new ActivityLink(default, activityTagCollection), + new ActivityLink(default, activityTagCollection), }; Activity = activitySource.StartActivity( @@ -61,9 +72,11 @@ static ActivityBenchmarks() Activity.AddTag($"AutoTag{i}", i); } - Activity.AddEvent(new ActivityEvent("event1")); - Activity.AddEvent(new ActivityEvent("event2")); - Activity.AddEvent(new ActivityEvent("event3")); + ActivityEvent = new ActivityEvent("event1", tags: activityTagCollection); + + Activity.AddEvent(ActivityEvent); + Activity.AddEvent(new ActivityEvent("event2", tags: activityTagCollection)); + Activity.AddEvent(new ActivityEvent("event3", tags: activityTagCollection)); Activity.Stop(); } @@ -135,7 +148,7 @@ public void EnumerateTagValuesNonemptyTagObjects() { TagEnumerator state = default; - Activity.EnumerateTagValues(ref state); + Activity.EnumerateTags(ref state); } [Benchmark] @@ -156,6 +169,24 @@ public void EnumerateLinksNonemptyActivityLinks() Activity.EnumerateLinks(ref state); } + [Benchmark] + public void EnumerateNonemptyActivityLinkTags() + { + int count = 0; + foreach (var tag in ActivityLink.Tags) + { + count++; + } + } + + [Benchmark] + public void EnumerateLinksNonemptyActivityLinkTags() + { + TagEnumerator state = default; + + ActivityLink.EnumerateTags(ref state); + } + [Benchmark] public void EnumerateNonemptyActivityEvents() { @@ -174,6 +205,24 @@ public void EnumerateEventsNonemptyActivityEvents() Activity.EnumerateEvents(ref state); } + [Benchmark] + public void EnumerateNonemptyActivityEventTags() + { + int count = 0; + foreach (var tag in ActivityEvent.Tags) + { + count++; + } + } + + [Benchmark] + public void EnumerateLinksNonemptyActivityEventTags() + { + TagEnumerator state = default; + + ActivityEvent.EnumerateTags(ref state); + } + private struct TagEnumerator : IActivityEnumerator> { public int Count { get; private set; } diff --git a/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs b/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs index 0e35d6b08e9..a7cbfd43ca1 100644 --- a/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs +++ b/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs @@ -143,7 +143,7 @@ public void EnumerateTagValuesEmpty() TagEnumerator state = default; - activity.EnumerateTagValues(ref state); + activity.EnumerateTags(ref state); Assert.Equal(0, state.Count); Assert.False(state.LastTag.HasValue); @@ -159,7 +159,7 @@ public void EnumerateTagValuesAll() TagEnumerator state = default; - activity.EnumerateTagValues(ref state); + activity.EnumerateTags(ref state); Assert.Equal(3, state.Count); Assert.True(state.LastTag.HasValue); @@ -178,7 +178,7 @@ public void EnumerateTagValuesWithBreak() TagEnumerator state = default; state.BreakOnCount = 1; - activity.EnumerateTagValues(ref state); + activity.EnumerateTags(ref state); Assert.Equal(1, state.Count); Assert.True(state.LastTag.HasValue); @@ -228,6 +228,40 @@ public void EnumerateLinksNonempty() Assert.Equal(links.Last(), state.LastLink); } + [Fact] + public void EnumerateLinkTagsEmpty() + { + ActivityLink activityLink = new ActivityLink(default); + + TagEnumerator state = default; + + activityLink.EnumerateTags(ref state); + + Assert.Equal(0, state.Count); + Assert.False(state.LastTag.HasValue); + } + + [Fact] + public void EnumerateLinkTagsNonempty() + { + ActivityLink activityLink = new ActivityLink( + default, + new ActivityTagsCollection(new Dictionary + { + ["tag1"] = "value1", + ["tag2"] = "value2", + ["tag3"] = "value3", + })); + + TagEnumerator state = default; + + activityLink.EnumerateTags(ref state); + + Assert.True(state.LastTag.HasValue); + Assert.Equal("tag3", state.LastTag?.Key); + Assert.Equal("value3", state.LastTag?.Value); + } + [Fact] public void EnumerateEventsEmpty() { @@ -258,6 +292,40 @@ public void EnumerateEventsNonempty() Assert.Equal("event3", state.LastEvent.Value.Name); } + [Fact] + public void EnumerateEventTagsEmpty() + { + ActivityEvent activityEvent = new ActivityEvent(default); + + TagEnumerator state = default; + + activityEvent.EnumerateTags(ref state); + + Assert.Equal(0, state.Count); + Assert.False(state.LastTag.HasValue); + } + + [Fact] + public void EnumerateEventTagsNonempty() + { + ActivityEvent activityEvent = new ActivityEvent( + "event", + tags: new ActivityTagsCollection(new Dictionary + { + ["tag1"] = "value1", + ["tag2"] = "value2", + ["tag3"] = "value3", + })); + + TagEnumerator state = default; + + activityEvent.EnumerateTags(ref state); + + Assert.True(state.LastTag.HasValue); + Assert.Equal("tag3", state.LastTag?.Key); + Assert.Equal("value3", state.LastTag?.Value); + } + private struct TagEnumerator : IActivityEnumerator> { public int BreakOnCount { get; set; }