Skip to content

Commit

Permalink
[sdk] Added support for setting CardinalityLimit allowed for the me…
Browse files Browse the repository at this point in the history
…tric managed by the view. (#5312)

Co-authored-by: Mikel Blanchard <mblanchard@macrosssoftware.com>
  • Loading branch information
Yun-Ting and CodeBlanch authored Feb 6, 2024
1 parent c057e6a commit 8ff986b
Show file tree
Hide file tree
Showing 11 changed files with 147 additions and 11 deletions.
1 change: 1 addition & 0 deletions OpenTelemetry.sln
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "experimental-apis", "experi
docs\diagnostics\experimental-apis\OTEL1000.md = docs\diagnostics\experimental-apis\OTEL1000.md
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\README.md = docs\diagnostics\experimental-apis\README.md
EndProjectSection
EndProject
Expand Down
2 changes: 1 addition & 1 deletion build/Common.props
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
<!-- Suppress warnings for repo code using experimental features -->
<NoWarn>$(NoWarn);OTEL1000;OTEL1001;OTEL1002</NoWarn>
<NoWarn>$(NoWarn);OTEL1000;OTEL1001;OTEL1002;OTEL1003</NoWarn>
<!--temporarily disable. See 3958-->
<!--<AnalysisLevel>latest-All</AnalysisLevel>-->
</PropertyGroup>
Expand Down
29 changes: 29 additions & 0 deletions docs/diagnostics/experimental-apis/OTEL1003.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# OpenTelemetry .NET Diagnostic: OTEL1003

## Overview

This is an Experimental API diagnostic covering the following API:

* `MetricStreamConfiguration.CardinalityLimit.get`
* `MetricStreamConfiguration.CardinalityLimit.set`

Experimental APIs may be changed or removed in the future.

## Details

The OpenTelemetry Specification defines the
[cardinality limit](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#cardinality-limits)
of a metric can be set by the matching view.

From the specification:

> The cardinality limit for an aggregation is defined in one of three ways:
> A view with criteria matching the instrument an aggregation is created for has
> an aggregation_cardinality_limit value defined for the stream, that value
> SHOULD be used. If there is no matching view, but the MetricReader defines a
> default cardinality limit value based on the instrument an aggregation is
> created for, that value SHOULD be used. If none of the previous values are
> defined, the default value of 2000 SHOULD be used.
We are exposing these APIs experimentally until the specification declares them
stable.
6 changes: 6 additions & 0 deletions docs/diagnostics/experimental-apis/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ Description: Metrics Exemplar Support

Details: [OTEL1002](./OTEL1002.md)

### OTEL1003

Description: MetricStreamConfiguration CardinalityLimit Support

Details: [OTEL1003](./OTEL1003.md)

## Inactive

Experimental APIs which have been released stable or removed:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ OpenTelemetry.Metrics.Exemplar.TraceId.get -> System.Diagnostics.ActivityTraceId
OpenTelemetry.Metrics.ExemplarFilter
OpenTelemetry.Metrics.ExemplarFilter.ExemplarFilter() -> void
OpenTelemetry.Metrics.MetricPoint.GetExemplars() -> OpenTelemetry.Metrics.Exemplar[]!
OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.get -> int?
OpenTelemetry.Metrics.MetricStreamConfiguration.CardinalityLimit.set -> void
OpenTelemetry.Metrics.TraceBasedExemplarFilter
OpenTelemetry.Metrics.TraceBasedExemplarFilter.TraceBasedExemplarFilter() -> void
static OpenTelemetry.Logs.LoggerProviderBuilderExtensions.AddProcessor(this OpenTelemetry.Logs.LoggerProviderBuilder! loggerProviderBuilder, OpenTelemetry.BaseProcessor<OpenTelemetry.Logs.LogRecord!>! processor) -> OpenTelemetry.Logs.LoggerProviderBuilder!
Expand Down
9 changes: 7 additions & 2 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,18 @@

* Fixed an issue where `SimpleExemplarReservoir` was not resetting internal
state for cumulative temporality.
[#5230](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5230)
([#5230](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5230))

* Fixed an issue causing `LogRecord`s to be incorrectly reused when wrapping an
instance of `BatchLogRecordExportProcessor` inside another
`BaseProcessor<LogRecord>` which leads to missing or incorrect data during
export.
[#5255](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5255)
([#5255](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5255))

* **Experimental (pre-release builds only):** Added support for setting
`CardinalityLimit` (the maximum number of data points allowed for a metric)
when configuring a view.
([#5312](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5312))

## 1.7.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ public static MeterProviderBuilder SetMaxMetricStreams(this MeterProviderBuilder
/// This may change in the future. See: https://github.com/open-telemetry/opentelemetry-dotnet/issues/2360.
/// </remarks>
/// <param name="meterProviderBuilder"><see cref="MeterProviderBuilder"/>.</param>
/// <param name="maxMetricPointsPerMetricStream">Maximum maximum number of metric points allowed per metric stream.</param>
/// <param name="maxMetricPointsPerMetricStream">Maximum number of metric points allowed per metric stream.</param>
/// <returns>The supplied <see cref="MeterProviderBuilder"/> for chaining.</returns>
public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterProviderBuilder meterProviderBuilder, int maxMetricPointsPerMetricStream)
{
Expand Down
6 changes: 6 additions & 0 deletions src/OpenTelemetry/Metrics/MetricReaderExt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ internal List<Metric> AddMetricsListWithViews(Instrument instrument, List<Metric
else
{
bool shouldReclaimUnusedMetricPoints = this.parentProvider is MeterProviderSdk meterProviderSdk && meterProviderSdk.ShouldReclaimUnusedMetricPoints;

if (metricStreamConfig != null && metricStreamConfig.CardinalityLimit != null)
{
this.maxMetricPointsPerMetricStream = metricStreamConfig.CardinalityLimit.Value;
}

Metric metric = new(metricStreamIdentity, this.GetAggregationTemporality(metricStreamIdentity.InstrumentType), this.maxMetricPointsPerMetricStream, this.emitOverflowAttribute, shouldReclaimUnusedMetricPoints, this.exemplarFilter);

this.instrumentIdentityToMetric[metricStreamIdentity] = metric;
Expand Down
48 changes: 44 additions & 4 deletions src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
// 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;

/// <summary>
Expand All @@ -10,6 +15,8 @@ public class MetricStreamConfiguration
{
private string? name;

private int? cardinalityLimit = null;

/// <summary>
/// Gets the drop configuration.
/// </summary>
Expand Down Expand Up @@ -91,11 +98,44 @@ public string[]? TagKeys
}
}

#if EXPOSE_EXPERIMENTAL_FEATURES
/// <summary>
/// Gets or sets a positive integer value defining the maximum number of
/// data points allowed for the metric managed by the view.
/// </summary>
/// <remarks>
/// <para><b>WARNING</b>: This is an experimental API which might change or
/// be removed in the future. Use at your own risk.</para>
/// <para>Spec reference: <see
/// href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#cardinality-limits">Cardinality
/// limits</see>.</para>
/// Note: If not set, the MeterProvider cardinality limit value will be
/// used, which defaults to 2000. Call <see
/// cref="MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream"/>
/// to configure the MeterProvider default.
/// </remarks>
#if NET8_0_OR_GREATER
[Experimental(DiagnosticDefinitions.CardinalityLimitExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)]
#endif
public
#else
internal
#endif
int? CardinalityLimit
{
get => this.cardinalityLimit;
set
{
if (value != null)
{
Guard.ThrowIfOutOfRange(value.Value, min: 1, max: int.MaxValue);
}

this.cardinalityLimit = value;
}
}

internal string[]? CopiedTagKeys { get; private set; }

internal int? ViewId { get; set; }

// TODO: MetricPoints caps can be configured here on
// a per stream basis, when we add such a capability
// in the future.
}
1 change: 1 addition & 0 deletions src/Shared/DiagnosticDefinitions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ internal static class DiagnosticDefinitions
public const string LoggerProviderExperimentalApi = "OTEL1000";
public const string LogsBridgeExperimentalApi = "OTEL1001";
public const string ExemplarExperimentalApi = "OTEL1002";
public const string CardinalityLimitExperimentalApi = "OTEL1003";
}
52 changes: 49 additions & 3 deletions test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

using System.Diagnostics.Metrics;
using System.Reflection;
using OpenTelemetry.Internal;
using OpenTelemetry.Tests;
using Xunit;
Expand Down Expand Up @@ -919,6 +920,34 @@ public void ViewConflict_OneInstrument_DifferentDescription()
Assert.Equal(10, metricPoint2.GetSumLong());
}

[Fact]
public void CardinalityLimitofMatchingViewTakesPrecedenceOverMetricProviderWhenBothWereSet()
{
using var meter = new Meter(Utils.GetCurrentMethodName());
var exportedItems = new List<Metric>();

using var container = this.BuildMeterProvider(out var meterProvider, builder => builder
.AddMeter(meter.Name)
.SetMaxMetricPointsPerMetricStream(3)
.AddView((instrument) =>
{
return new MetricStreamConfiguration() { Name = "MetricStreamA", CardinalityLimit = 10000 };
})
.AddInMemoryExporter(exportedItems));

var counter = meter.CreateCounter<long>("counter");
counter.Add(100);

meterProvider.ForceFlush(MaxTimeToAllowForFlush);

var metric = exportedItems[0];

var aggregatorStore = typeof(Metric).GetField("aggStore", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(metric) as AggregatorStore;
var maxMetricPointsAttribute = (int)typeof(AggregatorStore).GetField("maxMetricPoints", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(aggregatorStore);

Assert.Equal(10000, maxMetricPointsAttribute);
}

[Fact]
public void ViewConflict_TwoDistinctInstruments_ThreeStreams()
{
Expand All @@ -930,13 +959,18 @@ public void ViewConflict_TwoDistinctInstruments_ThreeStreams()
.AddMeter(meter.Name)
.AddView((instrument) =>
{
return new MetricStreamConfiguration() { Name = "MetricStreamA", Description = "description" };
return new MetricStreamConfiguration() { Name = "MetricStreamA", Description = "description", CardinalityLimit = 256 };
})
.AddView((instrument) =>
{
return instrument.Description == "description1"
? new MetricStreamConfiguration() { Name = "MetricStreamB" }
: new MetricStreamConfiguration() { Name = "MetricStreamC" };
? new MetricStreamConfiguration() { Name = "MetricStreamB", CardinalityLimit = 3 }
: new MetricStreamConfiguration() { Name = "MetricStreamC", CardinalityLimit = 200000 };
})
.AddView((instrument) =>
{
// This view is ignored as the passed in CardinalityLimit is out of range.
return new MetricStreamConfiguration() { Name = "MetricStreamD", CardinalityLimit = -1 };
})
.AddInMemoryExporter(exportedItems));

Expand All @@ -953,12 +987,24 @@ public void ViewConflict_TwoDistinctInstruments_ThreeStreams()
var metricB = exportedItems[1];
var metricC = exportedItems[2];

var aggregatorStoreA = typeof(Metric).GetField("aggStore", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(metricA) as AggregatorStore;
var maxMetricPointsAttributeA = (int)typeof(AggregatorStore).GetField("maxMetricPoints", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(aggregatorStoreA);

Assert.Equal(256, maxMetricPointsAttributeA);
Assert.Equal("MetricStreamA", metricA.Name);
Assert.Equal(20, GetAggregatedValue(metricA));

var aggregatorStoreB = typeof(Metric).GetField("aggStore", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(metricB) as AggregatorStore;
var maxMetricPointsAttributeB = (int)typeof(AggregatorStore).GetField("maxMetricPoints", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(aggregatorStoreB);

Assert.Equal(3, maxMetricPointsAttributeB);
Assert.Equal("MetricStreamB", metricB.Name);
Assert.Equal(10, GetAggregatedValue(metricB));

var aggregatorStoreC = typeof(Metric).GetField("aggStore", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(metricC) as AggregatorStore;
var maxMetricPointsAttributeC = (int)typeof(AggregatorStore).GetField("maxMetricPoints", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(aggregatorStoreC);

Assert.Equal(200000, maxMetricPointsAttributeC);
Assert.Equal("MetricStreamC", metricC.Name);
Assert.Equal(10, GetAggregatedValue(metricC));

Expand Down

0 comments on commit 8ff986b

Please sign in to comment.