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

Refactor temporality to align with the spec #2666

Merged
merged 4 commits into from
Nov 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public static MeterProviderBuilder AddConsoleExporter(this MeterProviderBuilder
? new BaseExportingMetricReader(exporter)
: new PeriodicExportingMetricReader(exporter, options.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds);

reader.PreferredAggregationTemporality = options.AggregationTemporality;
reader.Temporality = options.AggregationTemporality;

return builder.AddReader(reader);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

namespace OpenTelemetry.Exporter
{
[AggregationTemporality(AggregationTemporality.Cumulative | AggregationTemporality.Delta, AggregationTemporality.Cumulative)]
public class ConsoleMetricExporter : ConsoleExporter<Metric>
{
private Resource resource;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ namespace OpenTelemetry.Exporter
/// Exporter consuming <see cref="Metric"/> and exporting the data using
/// the OpenTelemetry protocol (OTLP).
/// </summary>
[AggregationTemporality(AggregationTemporality.Cumulative | AggregationTemporality.Delta, AggregationTemporality.Cumulative)]
public class OtlpMetricExporter : BaseExporter<Metric>
{
private readonly IExportClient<OtlpCollector.ExportMetricsServiceRequest> exportClient;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private static MeterProviderBuilder AddOtlpExporter(MeterProviderBuilder builder

var metricExporter = new OtlpMetricExporter(options);
var metricReader = new PeriodicExportingMetricReader(metricExporter, options.MetricExportIntervalMilliseconds);
metricReader.PreferredAggregationTemporality = options.AggregationTemporality;
metricReader.Temporality = options.AggregationTemporality;
return builder.AddReader(metricReader);
}
}
Expand Down
12 changes: 4 additions & 8 deletions src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.AggregationTemporality.Cumulative = 1 -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.AggregationTemporality.Delta = 2 -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.AggregationTemporalityAttribute
OpenTelemetry.Metrics.AggregationTemporalityAttribute.AggregationTemporalityAttribute(OpenTelemetry.Metrics.AggregationTemporality supported) -> void
OpenTelemetry.Metrics.AggregationTemporalityAttribute.AggregationTemporalityAttribute(OpenTelemetry.Metrics.AggregationTemporality supported, OpenTelemetry.Metrics.AggregationTemporality preferred) -> void
OpenTelemetry.Metrics.AggregationTemporalityAttribute.Preferred.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.AggregationTemporalityAttribute.Supported.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.AggregationTemporalityAttribute.AggregationTemporalityAttribute(OpenTelemetry.Metrics.AggregationTemporality temporality) -> void
OpenTelemetry.Metrics.AggregationTemporalityAttribute.Temporality.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.BaseExportingMetricReader
OpenTelemetry.Metrics.BaseExportingMetricReader.BaseExportingMetricReader(OpenTelemetry.BaseExporter<OpenTelemetry.Metrics.Metric> exporter) -> void
OpenTelemetry.Metrics.BaseExportingMetricReader.SupportedExportModes.get -> OpenTelemetry.Metrics.ExportModes
Expand Down Expand Up @@ -74,11 +72,9 @@ OpenTelemetry.Metrics.MetricReader.Collect(int timeoutMilliseconds = -1) -> bool
OpenTelemetry.Metrics.MetricReader.Dispose() -> void
OpenTelemetry.Metrics.MetricReader.MetricReader() -> void
OpenTelemetry.Metrics.MetricReader.ParentProvider.get -> OpenTelemetry.BaseProvider
OpenTelemetry.Metrics.MetricReader.PreferredAggregationTemporality.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.MetricReader.PreferredAggregationTemporality.set -> void
OpenTelemetry.Metrics.MetricReader.Shutdown(int timeoutMilliseconds = -1) -> bool
OpenTelemetry.Metrics.MetricReader.SupportedAggregationTemporality.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.MetricReader.SupportedAggregationTemporality.set -> void
OpenTelemetry.Metrics.MetricReader.Temporality.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.MetricReader.Temporality.set -> void
OpenTelemetry.Metrics.MetricReaderType
OpenTelemetry.Metrics.MetricReaderType.Manual = 0 -> OpenTelemetry.Metrics.MetricReaderType
OpenTelemetry.Metrics.MetricReaderType.Periodic = 1 -> OpenTelemetry.Metrics.MetricReaderType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.AggregationTemporality.Cumulative = 1 -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.AggregationTemporality.Delta = 2 -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.AggregationTemporalityAttribute
OpenTelemetry.Metrics.AggregationTemporalityAttribute.AggregationTemporalityAttribute(OpenTelemetry.Metrics.AggregationTemporality supported) -> void
OpenTelemetry.Metrics.AggregationTemporalityAttribute.AggregationTemporalityAttribute(OpenTelemetry.Metrics.AggregationTemporality supported, OpenTelemetry.Metrics.AggregationTemporality preferred) -> void
OpenTelemetry.Metrics.AggregationTemporalityAttribute.Preferred.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.AggregationTemporalityAttribute.Supported.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.AggregationTemporalityAttribute.AggregationTemporalityAttribute(OpenTelemetry.Metrics.AggregationTemporality temporality) -> void
OpenTelemetry.Metrics.AggregationTemporalityAttribute.Temporality.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.BaseExportingMetricReader
OpenTelemetry.Metrics.BaseExportingMetricReader.BaseExportingMetricReader(OpenTelemetry.BaseExporter<OpenTelemetry.Metrics.Metric> exporter) -> void
OpenTelemetry.Metrics.BaseExportingMetricReader.SupportedExportModes.get -> OpenTelemetry.Metrics.ExportModes
Expand Down Expand Up @@ -74,11 +72,9 @@ OpenTelemetry.Metrics.MetricReader.Collect(int timeoutMilliseconds = -1) -> bool
OpenTelemetry.Metrics.MetricReader.Dispose() -> void
OpenTelemetry.Metrics.MetricReader.MetricReader() -> void
OpenTelemetry.Metrics.MetricReader.ParentProvider.get -> OpenTelemetry.BaseProvider
OpenTelemetry.Metrics.MetricReader.PreferredAggregationTemporality.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.MetricReader.PreferredAggregationTemporality.set -> void
OpenTelemetry.Metrics.MetricReader.Shutdown(int timeoutMilliseconds = -1) -> bool
OpenTelemetry.Metrics.MetricReader.SupportedAggregationTemporality.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.MetricReader.SupportedAggregationTemporality.set -> void
OpenTelemetry.Metrics.MetricReader.Temporality.get -> OpenTelemetry.Metrics.AggregationTemporality
OpenTelemetry.Metrics.MetricReader.Temporality.set -> void
OpenTelemetry.Metrics.MetricReaderType
OpenTelemetry.Metrics.MetricReaderType.Manual = 0 -> OpenTelemetry.Metrics.MetricReaderType
OpenTelemetry.Metrics.MetricReaderType.Periodic = 1 -> OpenTelemetry.Metrics.MetricReaderType
Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
`GetExplicitBounds` methods from `MetricPoint`.
([#2664](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2664))

* Refactored temporality setting to align with the latest spec.
([#2666](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2666))

## 1.2.0-beta2

Released 2021-Nov-19
Expand Down
17 changes: 4 additions & 13 deletions src/OpenTelemetry/Metrics/AggregationTemporalityAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,13 @@ namespace OpenTelemetry.Metrics
[AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
public sealed class AggregationTemporalityAttribute : Attribute
{
private AggregationTemporality preferredAggregationTemporality;
private AggregationTemporality supportedAggregationTemporality;
private AggregationTemporality temporality;

public AggregationTemporalityAttribute(AggregationTemporality supported)
: this(supported, supported)
public AggregationTemporalityAttribute(AggregationTemporality temporality)
{
this.temporality = temporality;
}

public AggregationTemporalityAttribute(AggregationTemporality supported, AggregationTemporality preferred)
{
this.supportedAggregationTemporality = supported;
this.preferredAggregationTemporality = preferred;
}

public AggregationTemporality Preferred => this.preferredAggregationTemporality;

public AggregationTemporality Supported => this.supportedAggregationTemporality;
public AggregationTemporality Temporality => this.temporality;
}
}
3 changes: 1 addition & 2 deletions src/OpenTelemetry/Metrics/BaseExportingMetricReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ public BaseExportingMetricReader(BaseExporter<Metric> exporter)
if (attributes.Length > 0)
{
var attr = (AggregationTemporalityAttribute)attributes[attributes.Length - 1];
this.PreferredAggregationTemporality = attr.Preferred;
this.SupportedAggregationTemporality = attr.Supported;
this.Temporality = attr.Temporality;
}

attributes = exportorType.GetCustomAttributes(typeof(ExportModesAttribute), true);
Expand Down
53 changes: 16 additions & 37 deletions src/OpenTelemetry/Metrics/MetricReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,34 +27,36 @@ namespace OpenTelemetry.Metrics
/// </summary>
public abstract partial class MetricReader : IDisposable
{
private const AggregationTemporality CumulativeAndDelta = AggregationTemporality.Cumulative | AggregationTemporality.Delta;
private const AggregationTemporality AggregationTemporalityUnspecified = (AggregationTemporality)0;
private readonly object newTaskLock = new object();
private readonly object onCollectLock = new object();
private readonly TaskCompletionSource<bool> shutdownTcs = new TaskCompletionSource<bool>();
private AggregationTemporality preferredAggregationTemporality = CumulativeAndDelta;
private AggregationTemporality supportedAggregationTemporality = CumulativeAndDelta;
private AggregationTemporality temporality = AggregationTemporalityUnspecified;
private int shutdownCount;
private TaskCompletionSource<bool> collectionTcs;

public BaseProvider ParentProvider { get; private set; }

public AggregationTemporality PreferredAggregationTemporality
public AggregationTemporality Temporality
{
get => this.preferredAggregationTemporality;
set
get
{
ValidateAggregationTemporality(value, this.supportedAggregationTemporality);
this.preferredAggregationTemporality = value;
if (this.temporality == AggregationTemporalityUnspecified)
{
this.temporality = AggregationTemporality.Cumulative;
}

return this.temporality;
}
}

public AggregationTemporality SupportedAggregationTemporality
{
get => this.supportedAggregationTemporality;
set
{
ValidateAggregationTemporality(this.preferredAggregationTemporality, value);
this.supportedAggregationTemporality = value;
if (this.temporality != AggregationTemporalityUnspecified)
{
throw new NotSupportedException($"The temporality cannot be modified (the current value is {this.temporality}).");
}

this.temporality = value;
}
}

Expand Down Expand Up @@ -268,28 +270,5 @@ protected virtual bool OnShutdown(int timeoutMilliseconds)
protected virtual void Dispose(bool disposing)
{
}

private static void ValidateAggregationTemporality(AggregationTemporality preferred, AggregationTemporality supported)
{
Guard.Zero((int)(preferred & CumulativeAndDelta), $"PreferredAggregationTemporality has an invalid value {preferred}", nameof(preferred));
Guard.Zero((int)(supported & CumulativeAndDelta), $"SupportedAggregationTemporality has an invalid value {supported}", nameof(supported));

/*
| Preferred | Supported | Valid |
| ---------- | ---------- | ----- |
| Both | Both | true |
| Both | Cumulative | false |
| Both | Delta | false |
| Cumulative | Both | true |
| Cumulative | Cumulative | true |
| Cumulative | Delta | false |
| Delta | Both | true |
| Delta | Cumulative | false |
| Delta | Delta | true |
*/
string message = $"PreferredAggregationTemporality {preferred} and SupportedAggregationTemporality {supported} are incompatible";
Guard.Zero((int)(preferred & supported), message, nameof(preferred));
Guard.Range((int)preferred, nameof(preferred), max: (int)supported, maxName: nameof(supported), message: message);
}
}
}
4 changes: 2 additions & 2 deletions src/OpenTelemetry/Metrics/MetricReaderExt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ internal Metric AddMetricWithNoViews(Instrument instrument)
}
else
{
var metric = new Metric(instrument, this.preferredAggregationTemporality, metricName, instrument.Description, this.maxMetricPointsPerMetricStream);
var metric = new Metric(instrument, this.Temporality, metricName, instrument.Description, this.maxMetricPointsPerMetricStream);
this.metrics[index] = metric;
this.metricStreamNames.Add(metricStreamName);
return metric;
Expand Down Expand Up @@ -131,7 +131,7 @@ internal List<Metric> AddMetricsListWithViews(Instrument instrument, List<Metric
string[] tagKeysInteresting = metricStreamConfig?.TagKeys;
double[] histogramBucketBounds = (metricStreamConfig is ExplicitBucketHistogramConfiguration histogramConfig
&& histogramConfig.Boundaries != null) ? histogramConfig.Boundaries : null;
metric = new Metric(instrument, this.preferredAggregationTemporality, metricName, metricDescription, this.maxMetricPointsPerMetricStream, histogramBucketBounds, tagKeysInteresting);
metric = new Metric(instrument, this.Temporality, metricName, metricDescription, this.maxMetricPointsPerMetricStream, histogramBucketBounds, tagKeysInteresting);

this.metrics[index] = metric;
metrics.Add(metric);
Expand Down
2 changes: 1 addition & 1 deletion test/Benchmarks/Metrics/MetricCollectBenchmarks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void ProcessExport(Batch<Metric> batch)

this.reader = new BaseExportingMetricReader(metricExporter)
{
PreferredAggregationTemporality = AggregationTemporality.Cumulative,
Temporality = AggregationTemporality.Cumulative,
};

this.meter = new Meter(Utils.GetCurrentMethodName());
Expand Down
6 changes: 4 additions & 2 deletions test/Benchmarks/Metrics/MetricsBenchmarks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,10 @@ public void Setup()
this.meter = new Meter(Utils.GetCurrentMethodName());

var exportedItems = new List<Metric>();
var reader = new PeriodicExportingMetricReader(new InMemoryExporter<Metric>(exportedItems), 1000);
reader.PreferredAggregationTemporality = this.AggregationTemporality;
var reader = new PeriodicExportingMetricReader(new InMemoryExporter<Metric>(exportedItems), 1000)
{
Temporality = this.AggregationTemporality,
};
this.provider = Sdk.CreateMeterProviderBuilder()
.AddMeter(this.meter.Name) // All instruments from this meter are enabled.
.AddReader(reader)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ public void ToOtlpResourceMetricsTest(bool includeServiceNameInResource)
using var provider = Sdk.CreateMeterProviderBuilder()
.SetResourceBuilder(resourceBuilder)
.AddMeter(meter.Name)
.AddReader(new BaseExportingMetricReader(new InMemoryExporter<Metric>(exportedItems)) { PreferredAggregationTemporality = AggregationTemporality.Delta })
.AddReader(new BaseExportingMetricReader(new InMemoryExporter<Metric>(exportedItems))
{
Temporality = AggregationTemporality.Delta,
})
.Build();

var counter = meter.CreateCounter<int>("counter");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public async Task RequestMetricIsCaptured()

var metricReader = new BaseExportingMetricReader(metricExporter)
{
PreferredAggregationTemporality = AggregationTemporality.Cumulative,
Temporality = AggregationTemporality.Cumulative,
};
this.meterProvider = Sdk.CreateMeterProviderBuilder()
.AddAspNetCoreInstrumentation()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOut

var metricReader = new BaseExportingMetricReader(metricExporter)
{
PreferredAggregationTemporality = AggregationTemporality.Cumulative,
Temporality = AggregationTemporality.Cumulative,
};
var meterProvider = Sdk.CreateMeterProviderBuilder()
.AddHttpClientInstrumentation()
Expand Down
10 changes: 5 additions & 5 deletions test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ public class InMemoryExporterTests
[Fact(Skip = "To be run after https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 is fixed")]
public void InMemoryExporterShouldDeepCopyMetricPoints()
{
var metrics = new List<Metric>();
var exportedItems = new List<Metric>();

using var meter = new Meter(Utils.GetCurrentMethodName());
using var meterProvider = Sdk.CreateMeterProviderBuilder()
.AddMeter(meter.Name)
.AddReader(new BaseExportingMetricReader(new InMemoryExporter<Metric>(metrics))
.AddReader(new BaseExportingMetricReader(new InMemoryExporter<Metric>(exportedItems))
{
PreferredAggregationTemporality = AggregationTemporality.Delta,
Temporality = AggregationTemporality.Delta,
})
.Build();

Expand All @@ -45,7 +45,7 @@ public void InMemoryExporterShouldDeepCopyMetricPoints()

meterProvider.ForceFlush();

var metric = metrics[0]; // Only one Metric object is added to the collection at this point
var metric = exportedItems[0]; // Only one Metric object is added to the collection at this point
var metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator();
Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric
ref var metricPointForFirstExport = ref metricPointsEnumerator.Current;
Expand All @@ -56,7 +56,7 @@ public void InMemoryExporterShouldDeepCopyMetricPoints()

meterProvider.ForceFlush();

metric = metrics[1]; // Second Metric object is added to the collection at this point
metric = exportedItems[1]; // Second Metric object is added to the collection at this point
metricPointsEnumerator = metric.GetMetricPoints().GetEnumerator();
Assert.True(metricPointsEnumerator.MoveNext()); // One MetricPoint is emitted for the Metric
var metricPointForSecondExport = metricPointsEnumerator.Current;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void ExportOnlyWhenPointChanged(AggregationTemporality temporality)
.AddReader(
new BaseExportingMetricReader(new InMemoryExporter<Metric>(exportedItems))
{
PreferredAggregationTemporality = temporality,
Temporality = temporality,
})
.Build();

Expand Down
Loading