From a97ce3f77f60a102694625311769dd41deb3b786 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 4 Sep 2020 09:57:58 -0700 Subject: [PATCH 1/3] Added EnumerateTagValues extension on Activity for enumerating tags. --- .../Trace/ActivityExtensions.cs | 119 ++++++++++++++---- .../Trace/IActivityTagEnumerator.cs | 34 +++++ test/Benchmarks/ActivityBenchmarks.cs | 46 ++++++- 3 files changed, 167 insertions(+), 32 deletions(-) create mode 100644 src/OpenTelemetry.Api/Trace/IActivityTagEnumerator.cs diff --git a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs index 0d6ccf7d7ce..90c4060231f 100644 --- a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs +++ b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs @@ -30,12 +30,6 @@ public static class ActivityExtensions { private static readonly object EmptyActivityTagObjects = typeof(Activity).GetField("s_emptyTagObjects", BindingFlags.Static | BindingFlags.NonPublic).GetValue(null); - private static readonly Enumerator>, KeyValuePair, KeyValuePair>.AllocationFreeForEachDelegate - ActivityTagObjectsEnumerator = DictionaryEnumerator>.BuildAllocationFreeForEachDelegate( - typeof(Activity).GetField("_tags", BindingFlags.Instance | BindingFlags.NonPublic).FieldType); - - private static readonly DictionaryEnumerator>.ForEachDelegate GetTagValueCallbackRef = GetTagValueCallback; - /// /// Sets the status of activity execution. /// Activity class in .NET does not support 'Status'. @@ -70,14 +64,17 @@ public static Status GetStatus(this Activity activity) { Debug.Assert(activity != null, "Activity should not be null"); - var statusCanonicalCode = activity.GetTagValue(SpanAttributeConstants.StatusCodeKey) as string; - var statusDescription = activity.GetTagValue(SpanAttributeConstants.StatusDescriptionKey) as string; + ActivityStatusTagEnumerator state = default; + + ActivityTagObjectsEnumeratorFactory.Enumerate( + activity.TagObjects, + ref state); - var status = SpanHelper.ResolveCanonicalCodeToStatus(statusCanonicalCode); + var status = SpanHelper.ResolveCanonicalCodeToStatus(state.StatusCode); - if (status.IsValid && !string.IsNullOrEmpty(statusDescription)) + if (status.IsValid && !string.IsNullOrEmpty(state.StatusDescription)) { - return status.WithDescription(statusDescription); + return status.WithDescription(state.StatusDescription); } return status; @@ -92,22 +89,30 @@ public static Status GetStatus(this Activity activity) [MethodImpl(MethodImplOptions.AggressiveInlining)] public static object GetTagValue(this Activity activity, string tagName) { - var tagObjects = activity.TagObjects; - if (ReferenceEquals(tagObjects, EmptyActivityTagObjects)) - { - return null; - } + Debug.Assert(activity != null, "Activity should not be null"); - KeyValuePair state = new KeyValuePair(tagName, null); + ActivitySingleTagEnumerator state = new ActivitySingleTagEnumerator(tagName); - ActivityTagObjectsEnumerator( - tagObjects, - ref state, - GetTagValueCallbackRef); + ActivityTagObjectsEnumeratorFactory.Enumerate(activity.TagObjects, ref state); return state.Value; } + /// + /// Enumerates all the key/value pairs on an without performing an allocation. + /// + /// The struct implementation to use for the enumeration. + /// Activity instance. + /// Tag enumerator. + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static void EnumerateTagValues(this Activity activity, ref T tagEnumerator) + where T : struct, IActivityTagEnumerator + { + Debug.Assert(activity != null, "Activity should not be null"); + + ActivityTagObjectsEnumeratorFactory.Enumerate(activity.TagObjects, ref tagEnumerator); + } + /// /// Record Exception. /// @@ -135,15 +140,77 @@ public static void RecordException(this Activity activity, Exception ex) activity?.AddEvent(new ActivityEvent(SemanticConventions.AttributeExceptionEventName, default, tagsCollection)); } - private static bool GetTagValueCallback(ref KeyValuePair state, KeyValuePair item) + private struct ActivitySingleTagEnumerator : IActivityTagEnumerator { - if (item.Key == state.Key) + private readonly string tagName; + + public ActivitySingleTagEnumerator(string tagName) + { + this.tagName = tagName; + this.Value = null; + } + + public object Value { get; private set; } + + public bool ForEach(KeyValuePair item) + { + if (item.Key == this.tagName) + { + this.Value = item.Value; + return false; + } + + return true; + } + } + + private struct ActivityStatusTagEnumerator : IActivityTagEnumerator + { + public string StatusCode { get; private set; } + + public string StatusDescription { get; private set; } + + public bool ForEach(KeyValuePair item) + { + switch (item.Key) + { + case SpanAttributeConstants.StatusCodeKey: + this.StatusCode = item.Value as string; + break; + case SpanAttributeConstants.StatusDescriptionKey: + this.StatusDescription = item.Value as string; + break; + } + + return this.StatusCode == null || this.StatusDescription == null; + } + } + + private static class ActivityTagObjectsEnumeratorFactory + where TState : struct, IActivityTagEnumerator + { + 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(IEnumerable> tagObjects, ref TState state) { - state = item; - return false; + if (ReferenceEquals(tagObjects, EmptyActivityTagObjects)) + { + return; + } + + ActivityTagObjectsEnumerator( + tagObjects, + ref state, + ForEachTagValueCallbackRef); } - return true; + private static bool ForEachTagValueCallback(ref TState state, KeyValuePair item) + => state.ForEach(item); } } } diff --git a/src/OpenTelemetry.Api/Trace/IActivityTagEnumerator.cs b/src/OpenTelemetry.Api/Trace/IActivityTagEnumerator.cs new file mode 100644 index 00000000000..10a2241afa7 --- /dev/null +++ b/src/OpenTelemetry.Api/Trace/IActivityTagEnumerator.cs @@ -0,0 +1,34 @@ +// +// 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; + +namespace OpenTelemetry.Trace +{ + /// + /// An interface used to perform zero-allocation enumeration of tags. Implementation must be a struct. + /// + public interface IActivityTagEnumerator + { + /// + /// Called for each tag while the enumeration is executing. + /// + /// Tag key/value pair. + /// to continue the enumeration of records or to stop (break) the enumeration. + bool ForEach(KeyValuePair item); + } +} diff --git a/test/Benchmarks/ActivityBenchmarks.cs b/test/Benchmarks/ActivityBenchmarks.cs index 1af7782c1d5..2e2dff3c05c 100644 --- a/test/Benchmarks/ActivityBenchmarks.cs +++ b/test/Benchmarks/ActivityBenchmarks.cs @@ -36,10 +36,15 @@ static ActivityBenchmarks() Activity.AddTag("Tag1", "Value1"); Activity.AddTag("Tag2", 2); Activity.AddTag("Tag3", false); + + for (int i = 0; i < 1024; i++) + { + Activity.AddTag($"AutoTag{i}", i); + } } [Benchmark] - public void EnumerateEmptyTagObjects() + public void SearchEnumerateEmptyTagObjects() { object value; foreach (KeyValuePair tag in EmptyActivity.TagObjects) @@ -53,19 +58,19 @@ public void EnumerateEmptyTagObjects() } [Benchmark] - public void LinqEmptyTagObjects() + public void SearchLinqEmptyTagObjects() { EmptyActivity.TagObjects.FirstOrDefault(i => i.Key == "Tag3"); } [Benchmark] - public void GetTagValueEmptyTagObjects() + public void SearchGetTagValueEmptyTagObjects() { EmptyActivity.GetTagValue("Tag3"); } [Benchmark] - public void EnumerateNonemptyTagObjects() + public void SearchEnumerateNonemptyTagObjects() { object value; foreach (KeyValuePair tag in Activity.TagObjects) @@ -79,15 +84,44 @@ public void EnumerateNonemptyTagObjects() } [Benchmark] - public void LinqNonemptyTagObjects() + public void SearchLinqNonemptyTagObjects() { Activity.TagObjects.FirstOrDefault(i => i.Key == "Tag3"); } [Benchmark] - public void GetTagValueNonemptyTagObjects() + public void SearchGetTagValueNonemptyTagObjects() { Activity.GetTagValue("Tag3"); } + + [Benchmark] + public void EnumerateNonemptyTagObjects() + { + int count = 0; + foreach (KeyValuePair tag in Activity.TagObjects) + { + count++; + } + } + + [Benchmark] + public void EnumerateTagValuesNonemptyTagObjects() + { + Enumerator state = default; + + Activity.EnumerateTagValues(ref state); + } + + private struct Enumerator : IActivityTagEnumerator + { + public int Count { get; private set; } + + public bool ForEach(KeyValuePair item) + { + this.Count++; + return true; + } + } } } From 85897ee9b1b829596176b467d932db393eac24f7 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 4 Sep 2020 10:08:20 -0700 Subject: [PATCH 2/3] Refactor. --- src/OpenTelemetry.Api/Trace/ActivityExtensions.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs index 90c4060231f..87b11698d8a 100644 --- a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs +++ b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs @@ -66,9 +66,7 @@ public static Status GetStatus(this Activity activity) ActivityStatusTagEnumerator state = default; - ActivityTagObjectsEnumeratorFactory.Enumerate( - activity.TagObjects, - ref state); + ActivityTagObjectsEnumeratorFactory.Enumerate(activity, ref state); var status = SpanHelper.ResolveCanonicalCodeToStatus(state.StatusCode); @@ -93,7 +91,7 @@ public static object GetTagValue(this Activity activity, string tagName) ActivitySingleTagEnumerator state = new ActivitySingleTagEnumerator(tagName); - ActivityTagObjectsEnumeratorFactory.Enumerate(activity.TagObjects, ref state); + ActivityTagObjectsEnumeratorFactory.Enumerate(activity, ref state); return state.Value; } @@ -110,7 +108,7 @@ public static void EnumerateTagValues(this Activity activity, ref T tagEnumer { Debug.Assert(activity != null, "Activity should not be null"); - ActivityTagObjectsEnumeratorFactory.Enumerate(activity.TagObjects, ref tagEnumerator); + ActivityTagObjectsEnumeratorFactory.Enumerate(activity, ref tagEnumerator); } /// @@ -196,8 +194,10 @@ private static readonly DictionaryEnumerator.AllocationF private static readonly DictionaryEnumerator.ForEachDelegate ForEachTagValueCallbackRef = ForEachTagValueCallback; [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void Enumerate(IEnumerable> tagObjects, ref TState state) + public static void Enumerate(Activity activity, ref TState state) { + var tagObjects = activity.TagObjects; + if (ReferenceEquals(tagObjects, EmptyActivityTagObjects)) { return; From d040c626569d92c6effe4dca501a2970377b67ac Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 4 Sep 2020 10:50:09 -0700 Subject: [PATCH 3/3] Unit tests and changelog. --- src/OpenTelemetry.Api/CHANGELOG.md | 3 + .../Trace/ActivityExtensionsTest.cs | 72 +++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 27b7e856a31..79b9adbcf6b 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -6,6 +6,9 @@ * Added `GetTagValue` extension method on `Activity` for retrieving tag values efficiently ([#1221](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1221)) +* Added `EnumerateTagValues` extension method on `Activity` for enumerating tag + values efficiently + ([#1236](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1236)) ## 0.5.0-beta.2 diff --git a/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs b/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs index 0bdcced1a82..ccc16fc27fd 100644 --- a/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs +++ b/test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs @@ -15,6 +15,7 @@ // using System; +using System.Collections.Generic; using System.Diagnostics; using System.Linq; using Xunit; @@ -134,5 +135,76 @@ public void GetTagValue() Assert.Null(activity.GetTagValue("tag1")); Assert.Null(activity.GetTagValue("Tag2")); } + + [Fact] + public void EnumerateTagValuesEmpty() + { + Activity activity = new Activity("Test"); + + Enumerator state = default; + + activity.EnumerateTagValues(ref state); + + Assert.Equal(0, state.Count); + Assert.False(state.LastTag.HasValue); + } + + [Fact] + public void EnumerateTagValuesAll() + { + Activity activity = new Activity("Test"); + activity.AddTag("Tag1", "Value1"); + activity.AddTag("Tag2", "Value2"); + activity.AddTag("Tag3", "Value3"); + + Enumerator state = default; + + activity.EnumerateTagValues(ref state); + + Assert.Equal(3, state.Count); + Assert.True(state.LastTag.HasValue); + Assert.Equal("Tag3", state.LastTag?.Key); + Assert.Equal("Value3", state.LastTag?.Value); + } + + [Fact] + public void EnumerateTagValuesWithBreak() + { + Activity activity = new Activity("Test"); + activity.AddTag("Tag1", "Value1"); + activity.AddTag("Tag2", "Value2"); + activity.AddTag("Tag3", "Value3"); + + Enumerator state = default; + state.BreakOnCount = 1; + + activity.EnumerateTagValues(ref state); + + Assert.Equal(1, state.Count); + Assert.True(state.LastTag.HasValue); + Assert.Equal("Tag1", state.LastTag?.Key); + Assert.Equal("Value1", state.LastTag?.Value); + } + + private struct Enumerator : IActivityTagEnumerator + { + public int BreakOnCount { get; set; } + + public KeyValuePair? LastTag { get; private set; } + + public int Count { get; private set; } + + public bool ForEach(KeyValuePair item) + { + this.LastTag = item; + this.Count++; + if (this.BreakOnCount == this.Count) + { + return false; + } + + return true; + } + } } }