Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sdk] Added support for setting CardinalityLimit allowed for the metric managed by the view. #5312

Merged
merged 14 commits into from
Feb 6, 2024
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
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
#else
internal
#endif
int? CardinalityLimit
{
get => this.cardinalityLimit;
set
{
if (value != null)
{
Guard.ThrowIfOutOfRange(value.Value, min: 1, max: int.MaxValue);
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
}

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
Loading