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-metrics] Obsolete SetMaxMetricPointsPerMetricStream + standarize on "Cardinality Limit" name #5328

Merged
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 27 additions & 10 deletions docs/diagnostics/experimental-apis/OTEL1003.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,36 @@ 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.
>
> 1. 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.
> 2. 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.
> 3. 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.

### Setting cardinality limit for a specific Metric via the View API

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.

```csharp
using var meterProvider = Sdk.CreateMeterProviderBuilder()
.AddView(
instrumentName: "MyFruitCounter",
new MetricStreamConfiguration { CardinalityLimit = 10 })
.Build();
```

### Setting cardinality limit for a specific MetricReader

This is NOT currently supported by OpenTelemetry .NET.
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 4 additions & 5 deletions docs/metrics/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -356,11 +356,10 @@ predictable and reliable behavior when excessive cardinality happens, whether it
was due to a malicious attack or developer making mistakes while writing code.

OpenTelemetry has a default cardinality limit of `2000` per metric. This limit
can be configured at `MeterProvider` level using the
`SetMaxMetricPointsPerMetricStream` method, or at individual
[view](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#view)
level using `MetricStreamConfiguration.CardinalityLimit`. Refer to this
[doc](../../docs/metrics/customizing-the-sdk/README.md#changing-maximum-metricpoints-per-metricstream)
can be configured for individual metrics by using
[views](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#view)
and the `MetricStreamConfiguration.CardinalityLimit` setting. Refer to this
[doc](../../docs/metrics/customizing-the-sdk/README.md#changing-the-cardinality-limit-for-a-metric)
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
for more information.

Given a metric, once the cardinality limit is reached, any new measurement which
Expand Down
91 changes: 12 additions & 79 deletions docs/metrics/customizing-the-sdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -367,85 +367,18 @@ MyFruitCounter.Add(1, new("name", "apple"), new("color", "red"));
AnotherFruitCounter.Add(1, new("name", "apple"), new("color", "red"));
```

### Changing maximum MetricPoints per MetricStream

A Metric stream can contain as many Metric points as the number of unique
combination of keys and values. To protect the SDK from unbounded memory usage,
SDK limits the maximum number of metric points per metric stream, to a default
of 2000. Once the limit is hit, any new key/value combination for that metric is
ignored. The SDK chooses the key/value combinations in the order in which they
are emitted. `SetMaxMetricPointsPerMetricStream` can be used to override the
default.

> [!NOTE]
> One `MetricPoint` is reserved for every `MetricStream` for the
special case where there is no key/value pair associated with the metric. The
maximum number of `MetricPoint`s has to accommodate for this special case.

Consider the below example. Here we set the maximum number of `MetricPoint`s
allowed to be `3`. This means that for every `MetricStream`, the SDK will export
measurements for up to `3` distinct key/value combinations of the metric. There
are two instruments published here: `MyFruitCounter` and `AnotherFruitCounter`.
There are two total `MetricStream`s created one for each of these instruments.
SDK will limit the maximum number of distinct key/value combinations for each of
these `MetricStream`s to `3`.

```csharp
using System.Collections.Generic;
using System.Diagnostics.Metrics;
using OpenTelemetry;
using OpenTelemetry.Metrics;

Counter<long> MyFruitCounter = MyMeter.CreateCounter<long>("MyFruitCounter");
Counter<long> AnotherFruitCounter = MyMeter.CreateCounter<long>("AnotherFruitCounter");

using var meterProvider = Sdk.CreateMeterProviderBuilder()
.AddMeter("*")
.AddConsoleExporter()
.SetMaxMetricPointsPerMetricStream(3) // The default value is 2000
.Build();

// There are four distinct key/value combinations emitted for `MyFruitCounter`:
// 1. No key/value pair
// 2. (name:apple, color:red)
// 3. (name:lemon, color:yellow)
// 4. (name:apple, color:green)

// Since the maximum number of `MetricPoint`s allowed is `3`, the SDK will only export measurements for the following three combinations:
// 1. No key/value pair
// 2. (name:apple, color:red)
// 3. (name:lemon, color:yellow)

MyFruitCounter.Add(1); // Exported (No key/value pair)
MyFruitCounter.Add(1, new("name", "apple"), new("color", "red")); // Exported
MyFruitCounter.Add(2, new("name", "lemon"), new("color", "yellow")); // Exported
MyFruitCounter.Add(1, new("name", "lemon"), new("color", "yellow")); // Exported
MyFruitCounter.Add(2, new("name", "apple"), new("color", "green")); // Not exported
MyFruitCounter.Add(5, new("name", "apple"), new("color", "red")); // Exported
MyFruitCounter.Add(4, new("name", "lemon"), new("color", "yellow")); // Exported

// There are four distinct key/value combinations emitted for `AnotherFruitCounter`:
// 1. (name:kiwi)
// 2. (name:banana, color:yellow)
// 3. (name:mango, color:yellow)
// 4. (name:banana, color:green)

// Since the maximum number of `MetricPoint`s allowed is `3`, the SDK will only export measurements for the following three combinations:
// 1. No key/value pair (This is a special case. The SDK reserves a `MetricPoint` for it even if it's not explicitly emitted.)
// 2. (name:kiwi)
// 3. (name:banana, color:yellow)

AnotherFruitCounter.Add(4, new KeyValuePair<string, object>("name", "kiwi")); // Exported
AnotherFruitCounter.Add(1, new("name", "banana"), new("color", "yellow")); // Exported
AnotherFruitCounter.Add(2, new("name", "mango"), new("color", "yellow")); // Not exported
AnotherFruitCounter.Add(1, new("name", "mango"), new("color", "yellow")); // Not exported
AnotherFruitCounter.Add(2, new("name", "banana"), new("color", "green")); // Not exported
AnotherFruitCounter.Add(5, new("name", "banana"), new("color", "yellow")); // Exported
AnotherFruitCounter.Add(4, new("name", "mango"), new("color", "yellow")); // Not exported
Comment on lines -408 to -444
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reyang I might have been too aggressive removing all of this. Do you still want it in there just showing the view instead of the global mechanism?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong opinion here. @utpilla might have different ideas.

Copy link
Contributor

@utpilla utpilla Feb 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodeBlanch We should keep the example here. People have found it useful to understand how configuring the cardinality would affect the export.

```

To set the [cardinality limit](../README.md#cardinality-limits) at individual
metric level, use `MetricStreamConfiguration.CardinalityLimit`:
### Changing the cardinality limit for a Metric

A Metric contains many Metric points which are the number of unique combinations
of key/values recorded by an instrument. To protect the SDK from unbounded
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
memory usage, there is a default limit of 2000 metric points per metric which
will be maintained at any given time. Once the limit is hit, any new key/value
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
combination for a metric will be ignored. The SDK chooses the key/value
reyang marked this conversation as resolved.
Show resolved Hide resolved
combinations in the order in which they are emitted.

To set the [cardinality limit](../README.md#cardinality-limits) for an
individual metric, use `MetricStreamConfiguration.CardinalityLimit` setting on
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
the View API:

```csharp
var meterProvider = Sdk.CreateMeterProviderBuilder()
Expand Down
5 changes: 5 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
[IMetricsListener](https://learn.microsoft.com/dotNet/api/microsoft.extensions.diagnostics.metrics.imetricslistener).
([#5265](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5265))

* **Experimental (pre-release builds only):** Obsoleted
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
`MeterProviderBuilderExtensions.SetMaxMetricPointsPerMetricStream` in favor of
the new View-based `CardinalityLimit` API.
([#5328](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5328))
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved

## 1.7.0

Released 2023-Dec-08
Expand Down
46 changes: 23 additions & 23 deletions src/OpenTelemetry/Metrics/AggregatorStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ internal sealed class AggregatorStore
{
internal readonly bool OutputDelta;
internal readonly bool OutputDeltaWithUnusedMetricPointReclaimEnabled;
internal readonly int CardinalityLimit;
internal readonly bool EmitOverflowAttribute;
internal long DroppedMeasurements = 0;

private static readonly string MetricPointCapHitFixMessage = "Consider opting in for the experimental SDK feature to emit all the throttled metrics under the overflow attribute by setting env variable OTEL_DOTNET_EXPERIMENTAL_METRICS_EMIT_OVERFLOW_ATTRIBUTE = true. You could also modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit.";
Expand Down Expand Up @@ -42,8 +44,6 @@ internal sealed class AggregatorStore
private readonly int exponentialHistogramMaxScale;
private readonly UpdateLongDelegate updateLongCallback;
private readonly UpdateDoubleDelegate updateDoubleCallback;
private readonly int maxMetricPoints;
private readonly bool emitOverflowAttribute;
private readonly ExemplarFilter exemplarFilter;
private readonly Func<KeyValuePair<string, object?>[], int, int> lookupAggregatorStore;

Expand All @@ -57,17 +57,17 @@ internal AggregatorStore(
MetricStreamIdentity metricStreamIdentity,
AggregationType aggType,
AggregationTemporality temporality,
int maxMetricPoints,
int cardinalityLimit,
bool emitOverflowAttribute,
bool shouldReclaimUnusedMetricPoints,
ExemplarFilter? exemplarFilter = null)
{
this.name = metricStreamIdentity.InstrumentName;
this.maxMetricPoints = maxMetricPoints;
this.CardinalityLimit = cardinalityLimit;

this.metricPointCapHitMessage = $"Maximum MetricPoints limit reached for this Metric stream. Configured limit: {this.maxMetricPoints}";
this.metricPoints = new MetricPoint[maxMetricPoints];
this.currentMetricPointBatch = new int[maxMetricPoints];
this.metricPointCapHitMessage = $"Maximum MetricPoints limit reached for this Metric stream. Configured limit: {this.CardinalityLimit}";
this.metricPoints = new MetricPoint[cardinalityLimit];
this.currentMetricPointBatch = new int[cardinalityLimit];
this.aggType = aggType;
this.OutputDelta = temporality == AggregationTemporality.Delta;
this.histogramBounds = metricStreamIdentity.HistogramBucketBounds ?? FindDefaultHistogramBounds(in metricStreamIdentity);
Expand All @@ -89,7 +89,7 @@ internal AggregatorStore(
this.tagsKeysInterestingCount = hs.Count;
}

this.emitOverflowAttribute = emitOverflowAttribute;
this.EmitOverflowAttribute = emitOverflowAttribute;

var reservedMetricPointsCount = 1;

Expand All @@ -105,17 +105,17 @@ internal AggregatorStore(

if (this.OutputDeltaWithUnusedMetricPointReclaimEnabled)
{
this.availableMetricPoints = new Queue<int>(maxMetricPoints - reservedMetricPointsCount);
this.availableMetricPoints = new Queue<int>(cardinalityLimit - reservedMetricPointsCount);

// There is no overload which only takes capacity as the parameter
// Using the DefaultConcurrencyLevel defined in the ConcurrentDictionary class: https://github.com/dotnet/runtime/blob/v7.0.5/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs#L2020
// We expect at the most (maxMetricPoints - reservedMetricPointsCount) * 2 entries- one for sorted and one for unsorted input
this.tagsToMetricPointIndexDictionaryDelta =
new ConcurrentDictionary<Tags, LookupData>(concurrencyLevel: Environment.ProcessorCount, capacity: (maxMetricPoints - reservedMetricPointsCount) * 2);
new ConcurrentDictionary<Tags, LookupData>(concurrencyLevel: Environment.ProcessorCount, capacity: (cardinalityLimit - reservedMetricPointsCount) * 2);
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved

// Add all the indices except for the reserved ones to the queue so that threads have
// readily available access to these MetricPoints for their use.
for (int i = reservedMetricPointsCount; i < this.maxMetricPoints; i++)
for (int i = reservedMetricPointsCount; i < this.CardinalityLimit; i++)
{
this.availableMetricPoints.Enqueue(i);
}
Expand Down Expand Up @@ -164,12 +164,12 @@ internal int Snapshot()
}
else if (this.OutputDelta)
{
var indexSnapshot = Math.Min(this.metricPointIndex, this.maxMetricPoints - 1);
var indexSnapshot = Math.Min(this.metricPointIndex, this.CardinalityLimit - 1);
this.SnapshotDelta(indexSnapshot);
}
else
{
var indexSnapshot = Math.Min(this.metricPointIndex, this.maxMetricPoints - 1);
var indexSnapshot = Math.Min(this.metricPointIndex, this.CardinalityLimit - 1);
this.SnapshotCumulative(indexSnapshot);
}

Expand Down Expand Up @@ -227,7 +227,7 @@ internal void SnapshotDeltaWithMetricPointReclaim()

int startIndexForReclaimableMetricPoints = 1;

if (this.emitOverflowAttribute)
if (this.EmitOverflowAttribute)
{
startIndexForReclaimableMetricPoints = 2; // Index 0 and 1 are reserved for no tags and overflow

Expand All @@ -249,7 +249,7 @@ internal void SnapshotDeltaWithMetricPointReclaim()
}
}

for (int i = startIndexForReclaimableMetricPoints; i < this.maxMetricPoints; i++)
for (int i = startIndexForReclaimableMetricPoints; i < this.CardinalityLimit; i++)
{
ref var metricPoint = ref this.metricPoints[i];

Expand Down Expand Up @@ -440,7 +440,7 @@ private int LookupAggregatorStore(KeyValuePair<string, object?>[] tagKeysAndValu
if (!this.tagsToMetricPointIndexDictionary.TryGetValue(sortedTags, out aggregatorIndex))
{
aggregatorIndex = this.metricPointIndex;
if (aggregatorIndex >= this.maxMetricPoints)
if (aggregatorIndex >= this.CardinalityLimit)
{
// sorry! out of data points.
// TODO: Once we support cleanup of
Expand Down Expand Up @@ -469,7 +469,7 @@ private int LookupAggregatorStore(KeyValuePair<string, object?>[] tagKeysAndValu
if (!this.tagsToMetricPointIndexDictionary.TryGetValue(sortedTags, out aggregatorIndex))
{
aggregatorIndex = ++this.metricPointIndex;
if (aggregatorIndex >= this.maxMetricPoints)
if (aggregatorIndex >= this.CardinalityLimit)
{
// sorry! out of data points.
// TODO: Once we support cleanup of
Expand All @@ -496,7 +496,7 @@ private int LookupAggregatorStore(KeyValuePair<string, object?>[] tagKeysAndValu
{
// This else block is for tag length = 1
aggregatorIndex = this.metricPointIndex;
if (aggregatorIndex >= this.maxMetricPoints)
if (aggregatorIndex >= this.CardinalityLimit)
{
// sorry! out of data points.
// TODO: Once we support cleanup of
Expand All @@ -518,7 +518,7 @@ private int LookupAggregatorStore(KeyValuePair<string, object?>[] tagKeysAndValu
if (!this.tagsToMetricPointIndexDictionary.TryGetValue(givenTags, out aggregatorIndex))
{
aggregatorIndex = ++this.metricPointIndex;
if (aggregatorIndex >= this.maxMetricPoints)
if (aggregatorIndex >= this.CardinalityLimit)
{
// sorry! out of data points.
// TODO: Once we support cleanup of
Expand Down Expand Up @@ -929,7 +929,7 @@ private void UpdateLong(long value, ReadOnlySpan<KeyValuePair<string, object?>>
{
Interlocked.Increment(ref this.DroppedMeasurements);

if (this.emitOverflowAttribute)
if (this.EmitOverflowAttribute)
{
this.InitializeOverflowTagPointIfNotInitialized();
this.metricPoints[1].Update(value);
Expand Down Expand Up @@ -973,7 +973,7 @@ private void UpdateLongCustomTags(long value, ReadOnlySpan<KeyValuePair<string,
{
Interlocked.Increment(ref this.DroppedMeasurements);

if (this.emitOverflowAttribute)
if (this.EmitOverflowAttribute)
{
this.InitializeOverflowTagPointIfNotInitialized();
this.metricPoints[1].Update(value);
Expand Down Expand Up @@ -1017,7 +1017,7 @@ private void UpdateDouble(double value, ReadOnlySpan<KeyValuePair<string, object
{
Interlocked.Increment(ref this.DroppedMeasurements);

if (this.emitOverflowAttribute)
if (this.EmitOverflowAttribute)
{
this.InitializeOverflowTagPointIfNotInitialized();
this.metricPoints[1].Update(value);
Expand Down Expand Up @@ -1061,7 +1061,7 @@ private void UpdateDoubleCustomTags(double value, ReadOnlySpan<KeyValuePair<stri
{
Interlocked.Increment(ref this.DroppedMeasurements);

if (this.emitOverflowAttribute)
if (this.EmitOverflowAttribute)
{
this.InitializeOverflowTagPointIfNotInitialized();
this.metricPoints[1].Update(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public static MeterProviderBuilder SetMaxMetricStreams(this MeterProviderBuilder
{
if (builder is MeterProviderBuilderSdk meterProviderBuilderSdk)
{
meterProviderBuilderSdk.SetMaxMetricStreams(maxMetricStreams);
meterProviderBuilderSdk.SetMetricLimit(maxMetricStreams);
}
});

Expand All @@ -238,6 +238,9 @@ public static MeterProviderBuilder SetMaxMetricStreams(this MeterProviderBuilder
/// <param name="meterProviderBuilder"><see cref="MeterProviderBuilder"/>.</param>
/// <param name="maxMetricPointsPerMetricStream">Maximum number of metric points allowed per metric stream.</param>
/// <returns>The supplied <see cref="MeterProviderBuilder"/> for chaining.</returns>
#if EXPOSE_EXPERIMENTAL_FEATURES
[Obsolete("Use MetricStreamConfiguration.CardinalityLimit via the AddView API instead. This method will be removed in a future version.")]
reyang marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[Obsolete("Use MetricStreamConfiguration.CardinalityLimit via the AddView API instead. This method will be removed in a future version.")]
[Obsolete("Use MetricStreamConfiguration.CardinalityLimit via the AddView API instead. This method will be removed in the next major version bump.")]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or combine.. removed in a future version, along with major version bump of the package.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to do this here because we use similar a bunch of places but good thing for a follow-up!

#endif
public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterProviderBuilder meterProviderBuilder, int maxMetricPointsPerMetricStream)
{
Guard.ThrowIfOutOfRange(maxMetricPointsPerMetricStream, min: 1);
Expand All @@ -246,7 +249,7 @@ public static MeterProviderBuilder SetMaxMetricPointsPerMetricStream(this MeterP
{
if (builder is MeterProviderBuilderSdk meterProviderBuilderSdk)
{
meterProviderBuilderSdk.SetMaxMetricPointsPerMetricStream(maxMetricPointsPerMetricStream);
meterProviderBuilderSdk.SetDefaultCardinalityLimit(maxMetricPointsPerMetricStream);
}
});

Expand Down
Loading
Loading