[API Proposal]: Add `TagList` constructor to `Measurement`

[stevejgordon]

Background and motivation

Currently, if a consumer creating a Measurement wants to provide tags, several ctor overloads are available. One possibility when working with metrics is that the consumer creates a TagList that holds tags for use with measurements. The TagList struct is designed for performance cases and avoids allocating an array if the number of tags is less than nine. When passing a TagList to the constructor for a new Measurement, I noticed that it resolves to the IEnumerable<KeyValuePair<string, object?>> overload. This issue is because it causes the TagList to be boxed, allocating 160B on the heap.

Since this type is intended to improve performance and reduce allocations, I propose creating a specific ctor overload on Measurement to avoid the boxing. I also propose we use the in parameter modifier to avoid copying where possible. I believe ref readonly would be more correct here, but that would introduce a break for any existing call sites as they would be required to add the ref keyword to pass their argument.

Based on a local POC, I ran some benchmarks with the proposed API and an initial implementation.

| Method          | Size | Mean       | Error     | StdDev    | Median     | Ratio    | RatioSD | Gen0   | Gen1   | Allocated | Alloc Ratio |
|---------------- |----- |-----------:|----------:|----------:|-----------:|---------:|--------:|-------:|-------:|----------:|------------:|
| TagListOriginal | 1    |  36.610 ns | 0.7556 ns | 1.2415 ns |  36.396 ns | baseline |         | 0.0216 |      - |     272 B |             |
| TagListNewIn    | 1    |   6.684 ns | 0.1508 ns | 0.1613 ns |   6.685 ns |     -82% |    3.5% | 0.0032 |      - |      40 B |        -85% |
| TagListNewNoIn  | 1    |  11.576 ns | 0.2476 ns | 0.2316 ns |  11.520 ns |     -69% |    3.7% | 0.0032 |      - |      40 B |        -85% |
|                 |      |            |           |           |            |          |         |        |        |           |             |
| TagListOriginal | 8    |  71.095 ns | 1.4430 ns | 1.8249 ns |  70.799 ns | baseline |         | 0.0395 |      - |     496 B |             |
| TagListNewIn    | 8    |  34.036 ns | 0.6731 ns | 0.9214 ns |  33.787 ns |     -52% |    3.2% | 0.0121 |      - |     152 B |        -69% |
| TagListNewNoIn  | 8    |  39.731 ns | 0.7731 ns | 0.6853 ns |  39.720 ns |     -44% |    3.3% | 0.0121 |      - |     152 B |        -69% |
|                 |      |            |           |           |            |          |         |        |        |           |             |
| TagListOriginal | 100  | 165.453 ns | 3.1845 ns | 3.1276 ns | 165.638 ns | baseline |         | 0.2742 | 0.0017 |    3440 B |             |
| TagListNewIn    | 100  |  71.436 ns | 1.1935 ns | 1.2256 ns |  71.489 ns |     -57% |    2.7% | 0.1293 |      - |    1624 B |        -53% |
| TagListNewNoIn  | 100  |  78.025 ns | 1.8611 ns | 5.3698 ns |  76.523 ns |     -49% |    6.6% | 0.1293 |      - |    1624 B |        -53% |

/cc @tarekgh

API Proposal

namespace System.Diagnostics.Metrics;

public readonly struct Measurement<T> where T : struct
{
    public Measurement(T value) { }
    public Measurement(T value, IEnumerable<KeyValuePair<string, object?>>? tags) { }
    public Measurement(T value, params KeyValuePair<string, object?>[]? tags) { }
    public Measurement(T value, params ReadOnlySpan<KeyValuePair<string, object?>> tags) { }
+   public Measurement(T value, in TagList tags) { }

    ...
}

API Usage

var tagList = new TagList(
    new KeyValuePair<string, object?>("key1", "value1"),
    new KeyValuePair<string, object?>("key2", "value2"));

var measurement = new Measurement<long>(10, in tagList);

NOTE: The in keyword is optional, so non-breaking.

Alternative Designs

Add a factory method to Measurement to cover this scenario. A disadvantage is that the existing customer code would continue to cause boxing via the IEnumerable ctor, so there's no "free" performance gain from upgrading to a new version of .NET.

public static Measurement<T> Create(T value, in TagList tags);

Risks

No response

[stevejgordon]

@tarekgh Do you think there is any chance of slipping this into the .NET 9 milestone? I can contribute the implementation if the API is approved. I have it ready to go in a branch locally.

[stevejgordon]

I've updated the issue with an alternative design to consider.

[tarekgh]

Do you think there is any chance of slipping this into the .NET 9 milestone? I can contribute the implementation if the API is approved. I have it ready to go in a branch locally.

We only have a couple of weeks left to accept new APIs for .NET 9.0, considering the remaining planned work. Given this timeline, the new proposals are likely to be considered for a future release. However, since this is a small feature, I'll see if we can fit it into 9.0. No promise though 😄

The constructor proposal looks good to me. I am not in favor of the factory method alternative though.

[cijothomas]

Thanks! LGTM as is.

Also, any reason to not include dedicated overload for up to 3 Tags (which is most common usage pattern)? (For sync instruments, there is dedicated overload for up to 3 Tags)

@stevejgordon curious about the actual usage? I believe Measurement is directly used only with Observables right? Given they (Observable callbacks) are run once in few secs, is that allocation concern relevant?

[tarekgh]

Also, any reason to not include dedicated overload for up to 3 Tags (which is most common usage pattern)? (For sync instruments, there is dedicated overload for up to 3 Tags)

If we added the constructor taking TagList, why do we need to have more overloads taking more tags? It should be simple to use the TagList constructor. right?

            var m1 = new Measurement<int>(10, new TagList { { "tag1", "value1" }, { "tag2", "value2" }, { "tag3", "value3" } } );
vs.
            var m2 = new Measurement<int>(10, KeyValuePair.Create("tag1", "value1"), KeyValuePair.Create("tag2", "value2"), KeyValuePair.Create("tag3", "value3")); 

[stevejgordon]

@cijothomas For sending low numbers of tags, the params ReadOnlySpan<KeyValuePair<string, object?>> that's been added should be matched.

Yes, AFAIK, it's the observable case. The saving would add up for when lots of Meters, each with several instruments are being observed. Its use is probably reasonably rare as I think most send an array/list of the individual tags via params, but spotted the potential to avoid some boxing for those using TagList.

[cijothomas]

If we added the constructor taking TagList, why do we need to have more overloads taking more tags?

Same reason why the sync instruments (Counter, etc.) has dedicated overloads accept 1/2/3 Tag, and then one accepting TagList. The dedicated ones are faster than TagList for 1,2,3!

[tarekgh]

Same reason why the sync instruments (Counter, etc.) has dedicated overloads accept 1/2/3 Tag, and then one accepting TagList.

Internally we wrap the tags into TagList https://source.dot.net/#System.Diagnostics.DiagnosticSource/System/Diagnostics/Metrics/Instrument.netcore.cs,42.

The dedicated ones are https://github.com/open-telemetry/opentelemetry-dotnet/pull/2418#issuecomment-929507653 than TagList for 1,2,3!

I don't think this is true.

[cijothomas]

The dedicated ones are https://github.com/open-telemetry/opentelemetry-dotnet/pull/2418#issuecomment-929507653 than TagList for 1,2,3!

I don't think this is true.

The benchmarks tell otherwise! The linked PR has the benchmark code along with results!

[tarekgh]

I stand corrected @cijothomas.

Looking more at instrument publishing, the three tags overload convert the tags into inline array and then use it as Span. In the TagList case, the overhead is calling Add three times and then creates the Span (using the stack).

[stevejgordon]

@cijothomas I'm curious if the returns of the params keyword on public void Add(T delta, params ReadOnlySpan<KeyValuePair<string, object?>> tags) => RecordMeasurement(delta, tags); will make a difference to the results for those benchmarks too. I'll try to make some time to update them locally to use the newest System.Diagnostics code and compare.

[terrajobst]

Video

• Looks good as proposed

namespace System.Diagnostics.Metrics;

public partial struct Measurement<T>
{
    // Existing
    // public Measurement(T value);
    // public Measurement(T value, IEnumerable<KeyValuePair<string, object?>>? tags);
    // public Measurement(T value, params KeyValuePair<string, object?>[]? tags);
    // public Measurement(T value, params ReadOnlySpan<KeyValuePair<string, object?>> tags);
    public Measurement(T value, in TagList tags);
}

[stevejgordon]

@tarekgh I'll get the PR in with this change tomorrow AM (UK Time!)