getsentry / getsentry/sentry-dotnet
feat: add TagList overload to Metrics-Emit APIs as Attributes parameter
- 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
Assessment
This issue has not been assessed yet.