[API Proposal]: System.Diagnostics.Activity: Enumeration API [Iteration 2] #67207
Description
First proposal: #67072
Background and motivation
One of the main things the OTel .NET SDK needs to do is export Activity (trace) data. There is a lot of enumeration needed to do that. Activity tags, links(+tags), and events(+tags). Today the .NET API is all IEnumerable<T>/IEnumerator<T> based and all of the items being enumerated are readonly structs. Our performance testing was showing a lot of allocations incurred using the enumeration interfaces. To avoid that we generate IL to bind to the internal struct contracts. That fixes the allocations but a lot time is currently being spent copying value types.
What would be great is if the .NET API exposed a public method for enumerating these elements without allocation and as readonly ref to avoid copying.
/cc @reyang @cijothomas
API Proposal
The idea is to expose a base class which users may use (with a cast) to enumerate Activity data more efficiently:
namespace System.Diagnostics
{
// New
public abstract class ActivityItemEnumerable<T> : System.Collections.Generic.IEnumerable<T>
{
public abstract Enumerator GetEnumerator();
System.Collections.Generic.IEnumerator<T> System.Collections.Generic.IEnumerable<T>.GetEnumerator() { throw null; }
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; }
public struct Enumerator : System.Collections.Generic.IEnumerator<T>
{
public readonly ref T Current { get { throw null; } }
T System.Collections.Generic.IEnumerator<T>.Current { get { throw null; } }
object? System.Collections.IEnumerator.Current { get { throw null; } }
public bool MoveNext() { throw null; }
void System.Collections.IEnumerator.Reset() { }
public void Dispose() { }
}
}
}API Usage
public int EnumerateTagsUsingNewApi()
{
ActivityItemEnumerable<KeyValuePair<string, object>> enumerable = (ActivityItemEnumerable<KeyValuePair<string, object>>)this.activity.TagObjects;
int count = 0;
foreach (ref readonly KeyValuePair<string, object> tag in enumerable)
{
if (tag.Value != null)
{
count++;
}
}
return count;
}Example implementation
Diff: https://github.com/dotnet/runtime/compare/main...CodeBlanch:activity-enumerator?expand=1
Alternatives / considerations
Originally I wanted to change the return types on the Activity class but chatting with @tarekgh that would be a breaking change. Decided the cast would work well enough.
Not part of the proposal, but desired:
namespace System.Diagnostics
{
public abstract class Activity
{
- public System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object?>> TagObjects { get { throw null; } }
- public System.Collections.Generic.IEnumerable<System.Diagnostics.ActivityEvent> Events { get { throw null; } }
- public System.Collections.Generic.IEnumerable<System.Diagnostics.ActivityLink> Links { get { throw null; } }
+ public System.Diagnostics.DiagnosticEnumerable<System.Collections.Generic.KeyValuePair<string, object?>> TagObjects { get { throw null; } }
+ public System.Diagnostics.DiagnosticEnumerable<System.Diagnostics.ActivityEvent> Events { get { throw null; } }
+ public System.Diagnostics.DiagnosticEnumerable<System.Diagnostics.ActivityLink> Links { get { throw null; } }
}
}Instead of forcing a cast we could modify the Activity API to expose a strongly typed API at the cost of some duplication/confusion:
namespace System.Diagnostics
{
public abstract class Activity
{
public System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object?>> TagObjects { get { throw null; } }
public System.Collections.Generic.IEnumerable<System.Diagnostics.ActivityEvent> Events { get { throw null; } }
public System.Collections.Generic.IEnumerable<System.Diagnostics.ActivityLink> Links { get { throw null; } }
+ public System.Diagnostics.DiagnosticEnumerable<System.Collections.Generic.KeyValuePair<string, object?>> TagObjectsEnumerable { get { throw null; } }
+ public System.Diagnostics.DiagnosticEnumerable<System.Diagnostics.ActivityEvent> EventsEnumerable { get { throw null; } }
+ public System.Diagnostics.DiagnosticEnumerable<System.Diagnostics.ActivityLink> LinksEnumerable { get { throw null; } }
}
}Testing + benchmarks
.NET 6 API w/ OTel SDK
| Method | Mean | Error | StdDev | Gen 0 | Allocated |
|---|---|---|---|---|---|
| EnumerateTags | 129.53 ns | 2.354 ns | 2.087 ns | 0.0095 | 40 B |
| EnumerateTagsUsingReflection | 37.28 ns | 0.280 ns | 0.248 ns | - | - |
.NET 7 API w/ OTel SDK
Note: .NET 7 build contains already merged #67012 so the base cases are seeing some improvement from that optimization.
| Method | Mean | Error | StdDev | Gen 0 | Allocated |
|---|---|---|---|---|---|
| EnumerateTags | 94.96 ns | 1.927 ns | 3.892 ns | 0.0076 | 32 B |
| EnumerateTagsUsingReflection* | 20.21 ns | 0.423 ns | 0.503 ns | - | - |
| EnumerateTagsUsingNewApi | 15.82 ns | 0.250 ns | 0.234 ns | - | - |
* The proposed API implementation does not break the existing OTel SDK reflection/IL generation.
Benchmark code
using System;
using System.Collections.Generic;
using System.Diagnostics;
using BenchmarkDotNet.Attributes;
using OpenTelemetry.Trace;
namespace Benchmarks.Trace
{
[MemoryDiagnoser]
public class ActivityEnumerationBenchmarks : IDisposable
{
private readonly ActivitySource source = new("Source");
private readonly Activity activity;
public ActivityEnumerationBenchmarks()
{
Activity.DefaultIdFormat = ActivityIdFormat.W3C;
ActivitySource.AddActivityListener(new ActivityListener
{
ActivityStarted = null,
ActivityStopped = null,
ShouldListenTo = (activitySource) => activitySource.Name == "Source",
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllDataAndRecorded,
});
this.activity = this.source.StartActivity("test");
this.activity.SetTag("tag1", "value1");
this.activity.SetTag("tag2", "value2");
this.activity.SetTag("tag3", "value3");
this.activity.SetTag("tag4", "value4");
this.activity.SetTag("tag5", "value5");
this.activity.Stop();
}
public void Dispose()
{
this.source.Dispose();
}
[Benchmark]
public int EnumerateTags()
{
int count = 0;
foreach (KeyValuePair<string, object> tag in this.activity.TagObjects)
{
if (tag.Value != null)
{
count++;
}
}
return count;
}
[Benchmark]
public int EnumerateTagsUsingReflection()
{
TagEnumerationState state = default;
this.activity.EnumerateTags(ref state);
return state.Count;
}
[Benchmark]
public int EnumerateTagsUsingNewApi()
{
ActivityItemEnumerable<KeyValuePair<string, object>> enumerable = (ActivityItemEnumerable<KeyValuePair<string, object>>)this.activity.TagObjects;
int count = 0;
foreach (ref readonly KeyValuePair<string, object> tag in enumerable)
{
if (tag.Value != null)
{
count++;
}
}
return count;
}
private struct TagEnumerationState : IActivityEnumerator<KeyValuePair<string, object>>
{
public int Count;
public bool ForEach(KeyValuePair<string, object> activityTag)
{
if (activityTag.Value != null)
{
this.Count++;
}
return true;
}
}
}
}