[sdk-metrics] ExemplarReservoir dedicated diagnostic and custom Exemp…

…larReservoir support (#5558)

OpenTelemetry.sln

@@ -312,6 +312,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "experimental-apis", "experi
 		docs\diagnostics\experimental-apis\OTEL1001.md = docs\diagnostics\experimental-apis\OTEL1001.md
 		docs\diagnostics\experimental-apis\OTEL1002.md = docs\diagnostics\experimental-apis\OTEL1002.md
 		docs\diagnostics\experimental-apis\OTEL1003.md = docs\diagnostics\experimental-apis\OTEL1003.md
+		docs\diagnostics\experimental-apis\OTEL1004.md = docs\diagnostics\experimental-apis\OTEL1004.md
 		docs\diagnostics\experimental-apis\README.md = docs\diagnostics\experimental-apis\README.md
 	EndProjectSection
 EndProject

build/Common.props

@@ -10,7 +10,7 @@
     <Nullable>enable</Nullable>
     <ImplicitUsings>enable</ImplicitUsings>
     <!-- Suppress warnings for repo code using experimental features -->
-    <NoWarn>$(NoWarn);OTEL1000;OTEL1001;OTEL1002;OTEL1003</NoWarn>
+    <NoWarn>$(NoWarn);OTEL1000;OTEL1001;OTEL1002;OTEL1003;OTEL1004</NoWarn>
     <!--temporarily disable. See 3958-->
     <!--<AnalysisLevel>latest-All</AnalysisLevel>-->
   </PropertyGroup>

docs/diagnostics/experimental-apis/OTEL1002.md

@@ -4,12 +4,12 @@
 
 This is an Experimental API diagnostic covering the following APIs:
 
-* `AlwaysOnExemplarFilter`
-* `AlwaysOffExemplarFilter`
 * `Exemplar`
-* `ExemplarFilter`
+* `ExemplarFilterType`
 * `MeterProviderBuilder.SetExemplarFilter` extension method
-* `TraceBasedExemplarFilter`
+* `ReadOnlyExemplarCollection`
+* `ReadOnlyFilteredTagCollection`
+* `MetricPoint.TryGetExemplars`
 
 Experimental APIs may be changed or removed in the future.
 

docs/diagnostics/experimental-apis/OTEL1004.md

@@ -0,0 +1,62 @@
+# OpenTelemetry .NET Diagnostic: OTEL1004
+
+## Overview
+
+This is an Experimental API diagnostic covering the following APIs:
+
+* `ExemplarReservoir`
+* `FixedSizeExemplarReservoir`
+* `ExemplarMeasurement<T>`
+* `MetricStreamConfiguration.ExemplarReservoirFactory.get`
+* `MetricStreamConfiguration.ExemplarReservoirFactory.set`
+
+Experimental APIs may be changed or removed in the future.
+
+## Details
+
+The OpenTelemetry Specification defines an [ExemplarReservoir
+API](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#exemplarreservoir)
+and a mechanism for configuring `ExemplarReservoir` via the [View
+API](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#stream-configuration)
+in the Metrics SDK.
+
+From the specification:
+
+> The SDK MUST provide a mechanism for SDK users to provide their own
+> ExemplarReservoir implementation. This extension MUST be configurable on a
+> metric View, although individual reservoirs MUST still be instantiated per
+> metric-timeseries...
+
+We are exposing these APIs experimentally for the following reasons:
+
+* `FixedSizeExemplarReservoir` is not part of the spec. It is meant to help
+  custom reservoir authors and takes care of correctly creating & updating
+  `struct Exemplar`s (managing tag filtering when views are used), handles
+  `Exemplar` collection, and ensures all operations are safe to be called
+  concurrency (spec requirement). We want to see if this is helpful and meets
+  the needs of users.
+
+* There is currently no way to use
+  `MetricStreamConfiguration.ExemplarReservoirFactory` to switch a metric to a
+  different built-in reservoir (`AlignedHistogramBucketExemplarReservoir` or
+  `SimpleFixedSizeExemplarReservoir`). This is something supported by the spec
+  but we want to understand the use cases and needs before exposing these types.
+  Also it seems the default reservoirs may change.
+
+* There is currently no way to get access to the bucket index inside a reservoir
+  when a measurement is recorded against a histogram with explicit bounds. The
+  spec says the reservoir should calculate this given the
+  definition/configuration of the bounds but the SDK has already done this
+  computation. It seems unncessarily complicated to expose the configuration and
+  wasteful to do the work twice. We want to understand the types of algorithms
+  which users will want to implement before exposing something.
+
+**TL;DR** We want to gather feedback on the usability of the API and for the
+need(s) in general for custom reservoirs before exposing a stable API.
+
+<!--
+## Provide feedback
+
+Please provide feedback on [this issue](TODO) if you need stable support for
+custom `ExemplarReservoir`s.
+-->

docs/diagnostics/experimental-apis/README.md

@@ -39,6 +39,12 @@ Description: MetricStreamConfiguration CardinalityLimit Support
 
 Details: [OTEL1003](./OTEL1003.md)
 
+### OTEL1004
+
+Description: ExemplarReservoir Support
+
+Details: [OTEL1004](./OTEL1004.md)
+
 ## Inactive
 
 Experimental APIs which have been released stable or removed:

src/OpenTelemetry/.publicApi/Experimental/PublicAPI.Unshipped.txt

@@ -25,8 +25,12 @@ OpenTelemetry.Metrics.ExemplarMeasurement<T>.ExemplarMeasurement() -> void
 OpenTelemetry.Metrics.ExemplarMeasurement<T>.Tags.get -> System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string!, object?>>
 OpenTelemetry.Metrics.ExemplarMeasurement<T>.Value.get -> T
 OpenTelemetry.Metrics.ExemplarReservoir
-OpenTelemetry.Metrics.ExemplarReservoir.ExemplarReservoir() -> void
 OpenTelemetry.Metrics.ExemplarReservoir.ResetOnCollect.get -> bool
+OpenTelemetry.Metrics.FixedSizeExemplarReservoir
+OpenTelemetry.Metrics.FixedSizeExemplarReservoir.Capacity.get -> int
+OpenTelemetry.Metrics.FixedSizeExemplarReservoir.FixedSizeExemplarReservoir(int capacity) -> void
+OpenTelemetry.Metrics.FixedSizeExemplarReservoir.UpdateExemplar(int exemplarIndex, in OpenTelemetry.Metrics.ExemplarMeasurement<double> measurement) -> void
+OpenTelemetry.Metrics.FixedSizeExemplarReservoir.UpdateExemplar(int exemplarIndex, in OpenTelemetry.Metrics.ExemplarMeasurement<long> measurement) -> void
 OpenTelemetry.Metrics.MetricPoint.TryGetExemplars(out OpenTelemetry.Metrics.ReadOnlyExemplarCollection exemplars) -> bool
 OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.get -> int?
 OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.set -> void
@@ -46,6 +50,7 @@ OpenTelemetry.ReadOnlyFilteredTagCollection.Enumerator.Enumerator() -> void
 OpenTelemetry.ReadOnlyFilteredTagCollection.Enumerator.MoveNext() -> bool
 OpenTelemetry.ReadOnlyFilteredTagCollection.GetEnumerator() -> OpenTelemetry.ReadOnlyFilteredTagCollection.Enumerator
 OpenTelemetry.ReadOnlyFilteredTagCollection.ReadOnlyFilteredTagCollection() -> void
+override sealed OpenTelemetry.Metrics.FixedSizeExemplarReservoir.Collect() -> OpenTelemetry.Metrics.ReadOnlyExemplarCollection
 static OpenTelemetry.Logs.LoggerProviderBuilderExtensions.AddProcessor(this OpenTelemetry.Logs.LoggerProviderBuilder! loggerProviderBuilder, OpenTelemetry.BaseProcessor<OpenTelemetry.Logs.LogRecord!>! processor) -> OpenTelemetry.Logs.LoggerProviderBuilder!
 static OpenTelemetry.Logs.LoggerProviderBuilderExtensions.AddProcessor(this OpenTelemetry.Logs.LoggerProviderBuilder! loggerProviderBuilder, System.Func<System.IServiceProvider!, OpenTelemetry.BaseProcessor<OpenTelemetry.Logs.LogRecord!>!>! implementationFactory) -> OpenTelemetry.Logs.LoggerProviderBuilder!
 static OpenTelemetry.Logs.LoggerProviderBuilderExtensions.AddProcessor<T>(this OpenTelemetry.Logs.LoggerProviderBuilder! loggerProviderBuilder) -> OpenTelemetry.Logs.LoggerProviderBuilder!
@@ -63,3 +68,4 @@ static OpenTelemetry.Sdk.CreateLoggerProviderBuilder() -> OpenTelemetry.Logs.Log
 static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder) -> Microsoft.Extensions.Logging.ILoggingBuilder!
 static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action<OpenTelemetry.Logs.LoggerProviderBuilder!>! configure) -> Microsoft.Extensions.Logging.ILoggingBuilder!
 static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action<OpenTelemetry.Logs.LoggerProviderBuilder!>? configureBuilder, System.Action<OpenTelemetry.Logs.OpenTelemetryLoggerOptions!>? configureOptions) -> Microsoft.Extensions.Logging.ILoggingBuilder!
+virtual OpenTelemetry.Metrics.FixedSizeExemplarReservoir.OnCollected() -> void

src/OpenTelemetry/CHANGELOG.md

@@ -12,6 +12,12 @@
   which could have led to a measurement being dropped.
   ([#5546](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5546))
 
+* **Experimental (pre-release builds only):** Exposed
+  `FixedSizeExemplarReservoir` as a public API to support custom implementations
+  of `ExemplarReservoir` which may be configured using the
+  `ExemplarReservoirFactory` property on the View API.
+  ([#5558](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5558))
+
 ## 1.8.1
 
 Released 2024-Apr-17

src/OpenTelemetry/Metrics/Exemplar/ExemplarMeasurement.cs

@@ -12,10 +12,10 @@ namespace OpenTelemetry.Metrics;
 /// <summary>
 /// Represents an Exemplar measurement.
 /// </summary>
-/// <remarks><inheritdoc cref="Exemplar" path="/remarks/para[@experimental-warning='true']"/></remarks>
+/// <remarks><inheritdoc cref="ExemplarReservoir" path="/remarks/para[@experimental-warning='true']"/></remarks>
 /// <typeparam name="T">Measurement type.</typeparam>
 #if NET8_0_OR_GREATER
-[Experimental(DiagnosticDefinitions.ExemplarExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)]
+[Experimental(DiagnosticDefinitions.ExemplarReservoirExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)]
 #endif
 public
 #else

src/OpenTelemetry/Metrics/Exemplar/ExemplarReservoir.cs

@@ -13,19 +13,26 @@ namespace OpenTelemetry.Metrics;
 /// ExemplarReservoir base implementation and contract.
 /// </summary>
 /// <remarks>
-/// <inheritdoc cref="Exemplar" path="/remarks/para[@experimental-warning='true']"/>
+/// <para experimental-warning="true"><b>WARNING</b>: This is an experimental API which might change or be removed in the future. Use at your own risk.</para>
 /// Specification: <see
 /// href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#exemplarreservoir"/>.
 /// </remarks>
 #if NET8_0_OR_GREATER
-[Experimental(DiagnosticDefinitions.ExemplarExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)]
+[Experimental(DiagnosticDefinitions.ExemplarReservoirExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)]
 #endif
 public
 #else
 internal
 #endif
     abstract class ExemplarReservoir
 {
+    // Note: This constructor is internal because we don't allow custom
+    // ExemplarReservoir implementations to be based directly on the base class
+    // only FixedSizeExemplarReservoir.
+    internal ExemplarReservoir()
+    {
+    }
+
     /// <summary>
     /// Gets a value indicating whether or not the <see
     /// cref="ExemplarReservoir"/> should reset its state when performing

src/OpenTelemetry/Metrics/Exemplar/FixedSizeExemplarReservoir.cs

@@ -1,25 +1,56 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+#if EXPOSE_EXPERIMENTAL_FEATURES && NET8_0_OR_GREATER
+using System.Diagnostics.CodeAnalysis;
+#endif
 using OpenTelemetry.Internal;
 
 namespace OpenTelemetry.Metrics;
 
-internal abstract class FixedSizeExemplarReservoir : ExemplarReservoir
+#if EXPOSE_EXPERIMENTAL_FEATURES
+/// <summary>
+/// An <see cref="ExemplarReservoir"/> implementation which contains a fixed
+/// number of <see cref="Exemplar"/>s.
+/// </summary>
+/// <remarks><inheritdoc cref="ExemplarReservoir" path="/remarks/para[@experimental-warning='true']"/></remarks>
+#if NET8_0_OR_GREATER
+[Experimental(DiagnosticDefinitions.ExemplarReservoirExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)]
+#endif
+public
+#else
+internal
+#endif
+    abstract class FixedSizeExemplarReservoir : ExemplarReservoir
 {
     private readonly Exemplar[] runningExemplars;
     private readonly Exemplar[] snapshotExemplars;
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="FixedSizeExemplarReservoir"/> class.
+    /// </summary>
+    /// <param name="capacity">The capacity (number of <see cref="Exemplar"/>s)
+    /// to be contained in the reservoir.</param>
+#pragma warning disable RS0022 // Constructor make noninheritable base class inheritable
     protected FixedSizeExemplarReservoir(int capacity)
+#pragma warning restore RS0022 // Constructor make noninheritable base class inheritable
     {
+        // Note: RS0022 is suppressed because we do want to allow custom
+        // ExemplarReservoir implementations to be created by deriving from
+        // FixedSizeExemplarReservoir.
+
         Guard.ThrowIfOutOfRange(capacity, min: 1);
 
         this.runningExemplars = new Exemplar[capacity];
         this.snapshotExemplars = new Exemplar[capacity];
         this.Capacity = capacity;
     }
 
-    internal int Capacity { get; }
+    /// <summary>
+    /// Gets the capacity (number of <see cref="Exemplar"/>s) contained in the
+    /// reservoir.
+    /// </summary>
+    public int Capacity { get; }
 
     /// <summary>
     /// Collects all the exemplars accumulated by the Reservoir.
@@ -56,12 +87,48 @@ internal sealed override void Initialize(AggregatorStore aggregatorStore)
         base.Initialize(aggregatorStore);
     }
 
+    internal void UpdateExemplar<T>(
+        int exemplarIndex,
+        in ExemplarMeasurement<T> measurement)
+        where T : struct
+    {
+        this.runningExemplars[exemplarIndex].Update(in measurement);
+    }
+
+    /// <summary>
+    /// Fired when <see cref="Collect"/> has finished before returning a <see cref="ReadOnlyExemplarCollection"/>.
+    /// </summary>
+    /// <remarks>
+    /// Note: This method is typically used to reset the state of reservoirs and
+    /// is called regardless of the value of <see
+    /// cref="ExemplarReservoir.ResetOnCollect"/>.
+    /// </remarks>
     protected virtual void OnCollected()
     {
     }
 
-    protected void UpdateExemplar<T>(int exemplarIndex, in ExemplarMeasurement<T> measurement)
-        where T : struct
+    /// <summary>
+    /// Updates the <see cref="Exemplar"/> stored in the reservoir at the
+    /// specified index with an <see cref="ExemplarMeasurement{T}"/>.
+    /// </summary>
+    /// <param name="exemplarIndex">Index of the <see cref="Exemplar"/> to update.</param>
+    /// <param name="measurement"><see cref="ExemplarMeasurement{T}"/>.</param>
+    protected void UpdateExemplar(
+        int exemplarIndex,
+        in ExemplarMeasurement<long> measurement)
+    {
+        this.runningExemplars[exemplarIndex].Update(in measurement);
+    }
+
+    /// <summary>
+    /// Updates the <see cref="Exemplar"/> stored in the reservoir at the
+    /// specified index with an <see cref="ExemplarMeasurement{T}"/>.
+    /// </summary>
+    /// <param name="exemplarIndex">Index of the <see cref="Exemplar"/> to update.</param>
+    /// <param name="measurement"><see cref="ExemplarMeasurement{T}"/>.</param>
+    protected void UpdateExemplar(
+        int exemplarIndex,
+        in ExemplarMeasurement<double> measurement)
     {
         this.runningExemplars[exemplarIndex].Update(in measurement);
     }

src/OpenTelemetry/Metrics/MetricPoint.cs

@@ -2,6 +2,9 @@
 // SPDX-License-Identifier: Apache-2.0
 
 using System.Diagnostics;
+#if EXPOSE_EXPERIMENTAL_FEATURES && NET8_0_OR_GREATER
+using System.Diagnostics.CodeAnalysis;
+#endif
 using System.Runtime.CompilerServices;
 using OpenTelemetry.Internal;
 
@@ -370,6 +373,9 @@ public readonly bool TryGetHistogramMinMaxValues(out double min, out double max)
     /// <param name="exemplars"><see cref="ReadOnlyExemplarCollection"/>.</param>
     /// <returns><see langword="true" /> if exemplars exist; <see langword="false" /> otherwise.</returns>
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
+#if NET8_0_OR_GREATER
+    [Experimental(DiagnosticDefinitions.ExemplarExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)]
+#endif
     public
 #else
     [MethodImpl(MethodImplOptions.AggressiveInlining)]

src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs

@@ -144,15 +144,15 @@ public string[]? TagKeys
     /// when storing <see cref="Exemplar"/>s.
     /// </summary>
     /// <remarks>
-    /// <inheritdoc cref="Exemplar" path="/remarks/para[@experimental-warning='true']"/>
+    /// <inheritdoc cref="ExemplarReservoir" path="/remarks/para[@experimental-warning='true']"/>
     /// <para>Note: Returning <see langword="null"/> from the factory function will
     /// result in the default <see cref="ExemplarReservoir"/> being chosen by
     /// the SDK based on the type of metric.</para>
     /// Specification: <see
     /// href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#stream-configuration"/>.
     /// </remarks>
 #if NET8_0_OR_GREATER
-    [Experimental(DiagnosticDefinitions.ExemplarExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)]
+    [Experimental(DiagnosticDefinitions.ExemplarReservoirExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)]
 #endif
     public Func<ExemplarReservoir?>? ExemplarReservoirFactory { get; set; }
 #else

src/Shared/DiagnosticDefinitions.cs

@@ -13,4 +13,5 @@ internal static class DiagnosticDefinitions
     public const string LogsBridgeExperimentalApi = "OTEL1001";
     public const string ExemplarExperimentalApi = "OTEL1002";
     public const string CardinalityLimitExperimentalApi = "OTEL1003";
+    public const string ExemplarReservoirExperimentalApi = "OTEL1004";
 }