getsentry / getsentry/sentry-dotnet

feat: add TagList overload to Metrics-Emit APIs as Attributes parameter

Open
#5,259 1 comment 0 reactions 0 assignees View on GitHub
.NET Improvement Metrics Performance public API
Dominant language
C#
Stars
770
Forks
248
Avg merge
3d 4h
Merged PRs (30d)
49

Description

### Summary

Add overloads to `Sentry.SentrySdk.Metrics.Emit*` methods with a by-ref `System.Diagnostics.TagList` parameter for **Sentry Attributes**.

### Remarks

Added in .NET 6.0, and via the out-of-band NuGet package [System.Diagnostics.DiagnosticSource](https://www.nuget.org/packages/System.Diagnostics.DiagnosticSource/), the [System.Diagnostics.TagList](https://learn.microsoft.com/dotnet/api/system.diagnostics.taglist) is a collection of `KeyValuePair` that stores up to 8 (eight) tags within this `struct`, and only allocates contiguous regions of memory on the heap when representing more than 8 (eight) tags (starting with an array of 16 elements, growing again by eight when exceeded).
The `TagList` is used widely across the `System.Diagnostics.Metrics` APIs.

We can provide an overload to the `EmitCounter`, `EmitGauge` and `EmitDistribution` method groups so that callers can make use of this type, too, by representing [Sentry Attributes](https://develop.sentry.dev/sdk/foundations/state-management/scopes/attributes/).

A common scenario is when collecting/exporting `Meter`-based Metrics via the [System.Diagnostics.Metrics.MeterListener](https://learn.microsoft.com/dotnet/api/system.diagnostics.metrics.meterlistener), that we are currently prototyping.

We should pass this `struct` `ByRef` rather than `ByVal`, as it's a larger struct. Preferred in a `readonly` fashion, where `in` might be the best fit (because an _rvalue_ also makes sense semantically since we don't necessarily require a location). But make sure to only invoke `readonly` members of that `struct`, as it's not a `readonly struct`.

Also, consume via a `for` loop rather than a `foreach` loop, because the implicit `GetEnumerator` implementation does not return the `struct-based` `TagList.Enumerator`, but just the `IEnumerator>` interface, which is allocating a box for the `struct` on the heap on runtimes that cannot de-virtualize the call via dynamic PGO.
See blog [Performance Improvements in .NET 10](https://devblogs.microsoft.com/dotnet/performance-improvements-in-net-10/).

### Potential follow-up
Propose to `dotnet/runtime` to expose the `internal` `Tags` property, returning `ReadOnlySpan>`, via an unsafe Marshal type, similar to `System.Runtime.InteropServices.AsSpan<>(System.Collections.Generic.List?)`, highlighting our usage example where we could forward to our already existing internal `KeyValuePair` overloads, rather than providing a whole new code path for `in TagList`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.