Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added GetTagValue Activity Extension #1221

Merged
merged 5 commits into from
Sep 3, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
## Unreleased

* Updated System.Diagnostics.DiagnosticSource to version 5.0.0-rc.1.20428.3
* Added `GetTagValue` extension method on `Activity` for retrieving tag values
efficiently
([#1221](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1221))

## 0.5.0-beta.2

Expand Down
5 changes: 5 additions & 0 deletions src/OpenTelemetry.Api/OpenTelemetry.Api.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@
<DefineConstants>$(DefineConstants);API</DefineConstants>
</PropertyGroup>

<ItemGroup>
<Compile Include="..\OpenTelemetry\Internal\EnumerationHelper.cs" Link="Internal\EnumerationHelper.cs" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="System.Diagnostics.DiagnosticSource" Version="$(SystemDiagnosticSourcePkgVer)" />
<PackageReference Include="System.Reflection.Emit.Lightweight" Version="$(SystemReflectionEmitLightweightPkgVer)" />
</ItemGroup>

</Project>
47 changes: 46 additions & 1 deletion src/OpenTelemetry.Api/Trace/ActivityExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
// </copyright>

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.CompilerServices;
using OpenTelemetry.Internal;

Expand All @@ -28,6 +29,14 @@ namespace OpenTelemetry.Trace
/// </summary>
public static class ActivityExtensions
{
private static readonly object EmptyActivityTagObjects = typeof(Activity).GetField("s_emptyTagObjects", BindingFlags.Static | BindingFlags.NonPublic).GetValue(null);

private static readonly Enumerator<IEnumerable<KeyValuePair<string, object>>, KeyValuePair<string, object>, KeyValuePair<string, object>>.AllocationFreeForEachDelegate
ActivityTagObjectsEnumerator = DictionaryEnumerator<string, object, KeyValuePair<string, object>>.BuildAllocationFreeForEachDelegate(
typeof(Activity).GetField("_tags", BindingFlags.Instance | BindingFlags.NonPublic).FieldType);

private static readonly DictionaryEnumerator<string, object, KeyValuePair<string, object>>.ForEachDelegate GetTagValueCallbackRef = GetTagValueCallback;

/// <summary>
/// Sets the status of activity execution.
/// Activity class in .NET does not support 'Status'.
Expand Down Expand Up @@ -75,6 +84,31 @@ public static Status GetStatus(this Activity activity)
return status;
}

/// <summary>
/// Gets the value of a specific tag on an <see cref="Activity"/>.
/// </summary>
/// <param name="activity">Activity instance.</param>
/// <param name="tagName">Case-sensitive tag name to retrieve.</param>
/// <returns>Tag value or null if a match was not found.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static object GetTagValue(this Activity activity, string tagName)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious - I wonder if there are many scenarios where people would try to get multiple tag values. If yes, a further improvement might be giving a GetTagValues overload (which takes an array/list of names and a ref to Span or something allocated on the stack).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's actually a method a few lines up doing two lookups! 1 for status code, 1 for status description. I'll take a stab at this later. If the PR gets merged I'll do as a follow-up.

{
var tagObjects = activity.TagObjects;
if (ReferenceEquals(tagObjects, EmptyActivityTagObjects))
{
return null;
}

KeyValuePair<string, object> state = new KeyValuePair<string, object>(tagName, null);

ActivityTagObjectsEnumerator(
tagObjects,
ref state,
GetTagValueCallbackRef);

return state.Value;
}

/// <summary>
/// Record Exception.
/// </summary>
Expand All @@ -101,5 +135,16 @@ public static void RecordException(this Activity activity, Exception ex)

activity?.AddEvent(new ActivityEvent(SemanticConventions.AttributeExceptionEventName, default, tagsCollection));
}

private static bool GetTagValueCallback(ref KeyValuePair<string, object> state, KeyValuePair<string, object> item)
{
if (item.Key == state.Key)
{
state = item;
return false;
}

return true;
}
}
}
4 changes: 2 additions & 2 deletions src/OpenTelemetry/Internal/EnumerationHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ internal class Enumerator<TEnumerable, TItem, TState>
private static readonly ConcurrentDictionary<Type, AllocationFreeForEachDelegate> AllocationFreeForEachDelegates = new ConcurrentDictionary<Type, AllocationFreeForEachDelegate>();
private static readonly Func<Type, AllocationFreeForEachDelegate> BuildAllocationFreeForEachDelegateRef = BuildAllocationFreeForEachDelegate;

private delegate void AllocationFreeForEachDelegate(TEnumerable instance, ref TState state, ForEachDelegate itemCallback);
public delegate void AllocationFreeForEachDelegate(TEnumerable instance, ref TState state, ForEachDelegate itemCallback);

public delegate bool ForEachDelegate(ref TState state, TItem item);

Expand Down Expand Up @@ -93,7 +93,7 @@ public static void AllocationFreeForEach(Dictionary<string, int> dictionary, ref
}
...because it takes advantage of the struct Enumerator on the built-in types which give an allocation-free way to enumerate.
*/
private static AllocationFreeForEachDelegate BuildAllocationFreeForEachDelegate(Type enumerableType)
public static AllocationFreeForEachDelegate BuildAllocationFreeForEachDelegate(Type enumerableType)
{
var itemCallbackType = typeof(ForEachDelegate);

Expand Down
92 changes: 92 additions & 0 deletions test/Benchmarks/ActivityBenchmarks.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// <copyright file="ActivityBenchmarks.cs" company="OpenTelemetry Authors">
// 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.
// </copyright>
using System.Collections.Generic;
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
using System.Diagnostics;
using System.Linq;
using BenchmarkDotNet.Attributes;
using OpenTelemetry.Trace;

namespace OpenTelemetry.Benchmarks
{
[MemoryDiagnoser]
public class ActivityBenchmarks
{
private static readonly Activity EmptyActivity;
private static readonly Activity Activity;

static ActivityBenchmarks()
{
EmptyActivity = new Activity("EmptyActivity");

Activity = new Activity("Activity");
Activity.AddTag("Tag1", "Value1");
Activity.AddTag("Tag2", 2);
Activity.AddTag("Tag3", false);
}

[Benchmark]
public void EnumerateEmptyTagObjects()
{
object value;
foreach (KeyValuePair<string, object> tag in EmptyActivity.TagObjects)
{
if (tag.Key == "Tag3")
{
value = tag.Value;
break;
}
}
}

[Benchmark]
public void LinqEmptyTagObjects()
{
EmptyActivity.TagObjects.FirstOrDefault(i => i.Key == "Tag3");
}

[Benchmark]
public void GetTagValueEmptyTagObjects()
{
EmptyActivity.GetTagValue("Tag3");
}

[Benchmark]
public void EnumerateNonemptyTagObjects()
{
object value;
foreach (KeyValuePair<string, object> tag in Activity.TagObjects)
{
if (tag.Key == "Tag3")
{
value = tag.Value;
break;
}
}
}

[Benchmark]
public void LinqNonemptyTagObjects()
{
Activity.TagObjects.FirstOrDefault(i => i.Key == "Tag3");
}

[Benchmark]
public void GetTagValueNonemptyTagObjects()
{
Activity.GetTagValue("Tag3");
}
}
}
19 changes: 19 additions & 0 deletions test/OpenTelemetry.Tests/Trace/ActivityExtensionsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,5 +115,24 @@ public void CheckRecordException()
Assert.Equal(message, @event.Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionMessage).Value);
Assert.Equal(exception.GetType().Name, @event.Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionType).Value);
}

[Fact]
public void GetTagValueEmpty()
{
Activity activity = new Activity("Test");

Assert.Null(activity.GetTagValue("Tag1"));
}

[Fact]
public void GetTagValue()
{
Activity activity = new Activity("Test");
activity.AddTag("Tag1", "Value1");

Assert.Equal("Value1", activity.GetTagValue("Tag1"));
Assert.Null(activity.GetTagValue("tag1"));
Assert.Null(activity.GetTagValue("Tag2"));
}
}
}