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

fixing InMemoryExporter & Metrics bug. new class: MetricSnapshot. new ctor: InMemoryExporter(Func) #3266

Merged
merged 23 commits into from
May 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
6060936
initial commit. Metric Copy.
TimothyMothra May 10, 2022
8b011a3
test fix
TimothyMothra May 10, 2022
375f910
public api
TimothyMothra May 10, 2022
32f6d47
Merge branch 'main' into 2361_MetricCopy
TimothyMothra May 10, 2022
868d8de
rename MetricCopy to ReadOnlyMetric
TimothyMothra May 11, 2022
dd4da21
Merge branch '2361_MetricCopy' of https://github.com/TimothyMothra/op…
TimothyMothra May 11, 2022
77992f2
Merge branch 'main' into 2361_MetricCopy
TimothyMothra May 11, 2022
f764922
rename ReadOnlyMetric to MetricSnapshot
TimothyMothra May 13, 2022
20b0c6e
Merge branch 'main' into 2361_MetricCopy
TimothyMothra May 13, 2022
b30668e
Merge branch 'main' into 2361_MetricCopy
TimothyMothra May 17, 2022
20592a5
Merge branch 'main' into 2361_MetricCopy
TimothyMothra May 18, 2022
e79f567
addressing code review comments
TimothyMothra May 18, 2022
fe97058
Merge branch '2361_MetricCopy' of https://github.com/TimothyMothra/op…
TimothyMothra May 18, 2022
04b894e
update unit test
TimothyMothra May 20, 2022
c8261de
fix test
TimothyMothra May 20, 2022
4d75419
ExportMetricSnapshot as method
TimothyMothra May 20, 2022
92df4ea
Merge branch 'main' into 2361_MetricCopy
TimothyMothra May 20, 2022
436fd4b
addressing comments
TimothyMothra May 23, 2022
f2673c7
changelog
TimothyMothra May 23, 2022
2ce413b
markdownlint
TimothyMothra May 23, 2022
da22b5c
code review: restore if
TimothyMothra May 23, 2022
12aa17b
Merge branch 'main' into 2361_MetricCopy
cijothomas May 24, 2022
3427907
Merge branch 'main' into 2361_MetricCopy
cijothomas May 25, 2022
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
@@ -1,3 +1,14 @@
OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions
OpenTelemetry.Metrics.MetricSnapshot
OpenTelemetry.Metrics.MetricSnapshot.Description.get -> string
OpenTelemetry.Metrics.MetricSnapshot.MeterName.get -> string
OpenTelemetry.Metrics.MetricSnapshot.MeterVersion.get -> string
OpenTelemetry.Metrics.MetricSnapshot.MetricPoints.get -> System.Collections.Generic.IReadOnlyList<OpenTelemetry.Metrics.MetricPoint>
OpenTelemetry.Metrics.MetricSnapshot.MetricSnapshot(OpenTelemetry.Metrics.Metric metric) -> void
OpenTelemetry.Metrics.MetricSnapshot.MetricType.get -> OpenTelemetry.Metrics.MetricType
OpenTelemetry.Metrics.MetricSnapshot.Name.get -> string
OpenTelemetry.Metrics.MetricSnapshot.Unit.get -> string
static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection<OpenTelemetry.Metrics.Metric> exportedItems) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection<OpenTelemetry.Metrics.Metric> exportedItems, System.Action<OpenTelemetry.Metrics.MetricReaderOptions> configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection<OpenTelemetry.Metrics.Metric> exportedItems, System.Action<OpenTelemetry.Metrics.MetricReaderOptions> configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection<OpenTelemetry.Metrics.MetricSnapshot> exportedItems) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection<OpenTelemetry.Metrics.MetricSnapshot> exportedItems, System.Action<OpenTelemetry.Metrics.MetricReaderOptions> configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions
OpenTelemetry.Metrics.MetricSnapshot
OpenTelemetry.Metrics.MetricSnapshot.Description.get -> string
OpenTelemetry.Metrics.MetricSnapshot.MeterName.get -> string
OpenTelemetry.Metrics.MetricSnapshot.MeterVersion.get -> string
OpenTelemetry.Metrics.MetricSnapshot.MetricPoints.get -> System.Collections.Generic.IReadOnlyList<OpenTelemetry.Metrics.MetricPoint>
OpenTelemetry.Metrics.MetricSnapshot.MetricSnapshot(OpenTelemetry.Metrics.Metric metric) -> void
OpenTelemetry.Metrics.MetricSnapshot.MetricType.get -> OpenTelemetry.Metrics.MetricType
OpenTelemetry.Metrics.MetricSnapshot.Name.get -> string
OpenTelemetry.Metrics.MetricSnapshot.Unit.get -> string
static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection<OpenTelemetry.Metrics.Metric> exportedItems) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection<OpenTelemetry.Metrics.Metric> exportedItems, System.Action<OpenTelemetry.Metrics.MetricReaderOptions> configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection<OpenTelemetry.Metrics.Metric> exportedItems, System.Action<OpenTelemetry.Metrics.MetricReaderOptions> configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection<OpenTelemetry.Metrics.MetricSnapshot> exportedItems) -> OpenTelemetry.Metrics.MeterProviderBuilder
static OpenTelemetry.Metrics.InMemoryExporterMetricsExtensions.AddInMemoryExporter(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Collections.Generic.ICollection<OpenTelemetry.Metrics.MetricSnapshot> exportedItems, System.Action<OpenTelemetry.Metrics.MetricReaderOptions> configureMetricReader) -> OpenTelemetry.Metrics.MeterProviderBuilder
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Exporter.InMemory/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Adds new `AddInMemoryExporter` extension method to export `Metric` as new
type `MetricSnapshot`.
([#2361](https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361))

## 1.3.0-beta.2

Released 2022-May-16
Expand Down
12 changes: 11 additions & 1 deletion src/OpenTelemetry.Exporter.InMemory/InMemoryExporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// limitations under the License.
// </copyright>

using System;
using System.Collections.Generic;

namespace OpenTelemetry.Exporter
Expand All @@ -22,13 +23,22 @@ public class InMemoryExporter<T> : BaseExporter<T>
where T : class
{
private readonly ICollection<T> exportedItems;
private readonly Func<Batch<T>, ExportResult> onExport;

public InMemoryExporter(ICollection<T> exportedItems)
{
this.exportedItems = exportedItems;
this.onExport = (Batch<T> batch) => this.DefaultExport(batch);
}

public override ExportResult Export(in Batch<T> batch)
internal InMemoryExporter(Func<Batch<T>, ExportResult> exportFunc)
{
this.onExport = exportFunc;
}

public override ExportResult Export(in Batch<T> batch) => this.onExport(batch);

private ExportResult DefaultExport(in Batch<T> batch)
{
if (this.exportedItems == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,28 @@ public static class InMemoryExporterMetricsExtensions
/// <summary>
/// Adds InMemory metric exporter to the <see cref="MeterProviderBuilder"/> using default options.
/// </summary>
/// <remarks>
/// Be aware that <see cref="Metric"/> may continue to be updated after export.
/// </remarks>
/// <param name="builder"><see cref="MeterProviderBuilder"/> builder to use.</param>
/// <param name="exportedItems">Collection which will be populated with the exported MetricItem.</param>
/// <param name="exportedItems">Collection which will be populated with the exported <see cref="Metric"/>.</param>
/// <returns>The instance of <see cref="MeterProviderBuilder"/> to chain the calls.</returns>
public static MeterProviderBuilder AddInMemoryExporter(this MeterProviderBuilder builder, ICollection<Metric> exportedItems)
{
return builder.AddInMemoryExporter(exportedItems: exportedItems, configureMetricReader: null);
}

/// <summary>
/// Adds InMemory metric exporter to the <see cref="MeterProviderBuilder"/>.
/// </summary>
/// <remarks>
/// Be aware that <see cref="Metric"/> may continue to be updated after export.
/// </remarks>
/// <param name="builder"><see cref="MeterProviderBuilder"/> builder to use.</param>
/// <param name="exportedItems">Collection which will be populated with the exported <see cref="Metric"/>.</param>
/// <param name="configureMetricReader"><see cref="MetricReader"/> configuration options.</param>
/// <returns>The instance of <see cref="MeterProviderBuilder"/> to chain the calls.</returns>
public static MeterProviderBuilder AddInMemoryExporter(this MeterProviderBuilder builder, ICollection<Metric> exportedItems, Action<MetricReaderOptions> configureMetricReader)
{
Guard.ThrowIfNull(builder);
Guard.ThrowIfNull(exportedItems);
Expand All @@ -45,21 +63,45 @@ public static MeterProviderBuilder AddInMemoryExporter(this MeterProviderBuilder
{
return deferredMeterProviderBuilder.Configure((sp, builder) =>
{
AddInMemoryExporter(builder, exportedItems, sp.GetOptions<MetricReaderOptions>(), null);
AddInMemoryExporter(builder, exportedItems, sp.GetOptions<MetricReaderOptions>(), configureMetricReader);
});
}

return AddInMemoryExporter(builder, exportedItems, new MetricReaderOptions(), null);
return AddInMemoryExporter(builder, exportedItems, new MetricReaderOptions(), configureMetricReader);
}

/// <summary>
/// Adds InMemory metric exporter to the <see cref="MeterProviderBuilder"/> using default options.
/// The exporter will be setup to export <see cref="MetricSnapshot"/>.
/// </summary>
/// <remarks>
/// Use this if you need a copy of <see cref="Metric"/> that will not be updated after export.
/// </remarks>
/// <param name="builder"><see cref="MeterProviderBuilder"/> builder to use.</param>
/// <param name="exportedItems">Collection which will be populated with the exported MetricItem.</param>
/// <param name="exportedItems">Collection which will be populated with the exported <see cref="Metric"/> represented as <see cref="MetricSnapshot"/>.</param>
/// <returns>The instance of <see cref="MeterProviderBuilder"/> to chain the calls.</returns>
public static MeterProviderBuilder AddInMemoryExporter(
this MeterProviderBuilder builder,
ICollection<MetricSnapshot> exportedItems)
{
return builder.AddInMemoryExporter(exportedItems: exportedItems, configureMetricReader: null);
}

/// <summary>
/// Adds InMemory metric exporter to the <see cref="MeterProviderBuilder"/>.
/// The exporter will be setup to export <see cref="MetricSnapshot"/>.
/// </summary>
/// <remarks>
/// Use this if you need a copy of <see cref="Metric"/> that will not be updated after export.
/// </remarks>
/// <param name="builder"><see cref="MeterProviderBuilder"/> builder to use.</param>
/// <param name="exportedItems">Collection which will be populated with the exported <see cref="Metric"/> represented as <see cref="MetricSnapshot"/>.</param>
/// <param name="configureMetricReader"><see cref="MetricReader"/> configuration options.</param>
/// <returns>The instance of <see cref="MeterProviderBuilder"/> to chain the calls.</returns>
public static MeterProviderBuilder AddInMemoryExporter(this MeterProviderBuilder builder, ICollection<Metric> exportedItems, Action<MetricReaderOptions> configureMetricReader)
public static MeterProviderBuilder AddInMemoryExporter(
this MeterProviderBuilder builder,
ICollection<MetricSnapshot> exportedItems,
Action<MetricReaderOptions> configureMetricReader)
{
Guard.ThrowIfNull(builder);
Guard.ThrowIfNull(exportedItems);
Expand Down Expand Up @@ -93,5 +135,40 @@ private static MeterProviderBuilder AddInMemoryExporter(

return builder.AddReader(metricReader);
}

private static MeterProviderBuilder AddInMemoryExporter(
MeterProviderBuilder builder,
ICollection<MetricSnapshot> exportedItems,
MetricReaderOptions metricReaderOptions,
Action<MetricReaderOptions> configureMetricReader)
{
configureMetricReader?.Invoke(metricReaderOptions);

var metricExporter = new InMemoryExporter<Metric>(
exportFunc: metricBatch => ExportMetricSnapshot(metricBatch, exportedItems));

var metricReader = PeriodicExportingMetricReaderHelper.CreatePeriodicExportingMetricReader(
metricExporter,
metricReaderOptions,
DefaultExportIntervalMilliseconds,
DefaultExportTimeoutMilliseconds);

return builder.AddReader(metricReader);
}

private static ExportResult ExportMetricSnapshot(in Batch<Metric> batch, ICollection<MetricSnapshot> exportedItems)
{
if (exportedItems == null)
TimothyMothra marked this conversation as resolved.
Show resolved Hide resolved
{
return ExportResult.Failure;
}

foreach (var metric in batch)
{
exportedItems.Add(new MetricSnapshot(metric));
}

return ExportResult.Success;
}
}
}
58 changes: 58 additions & 0 deletions src/OpenTelemetry.Exporter.InMemory/MetricSnapshot.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// <copyright file="MetricSnapshot.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

using System.Collections.Generic;

namespace OpenTelemetry.Metrics
{
/// <summary>
/// This class represents a selective copy of <see cref="Metric"/>.
/// This contains the minimum fields and properties needed for most
/// unit testing scenarios.
/// </summary>
public class MetricSnapshot
{
private readonly MetricStreamIdentity instrumentIdentity;

public MetricSnapshot(Metric metric)
{
this.instrumentIdentity = metric.InstrumentIdentity;
this.MetricType = metric.MetricType;

List<MetricPoint> metricPoints = new();
foreach (ref readonly var metricPoint in metric.GetMetricPoints())
{
metricPoints.Add(metricPoint.Copy());
}

this.MetricPoints = metricPoints;
}

public string Name => this.instrumentIdentity.InstrumentName;

public string Description => this.instrumentIdentity.Description;

public string Unit => this.instrumentIdentity.Unit;

public string MeterName => this.instrumentIdentity.MeterName;

public MetricType MetricType { get; }

public string MeterVersion => this.instrumentIdentity.MeterVersion;

public IReadOnlyList<MetricPoint> MetricPoints { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
</ItemGroup>

<ItemGroup>
<Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\PeriodicExportingMetricReaderHelper.cs" Link="Includes\PeriodicExportingMetricReaderHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\ServiceProviderExtensions.cs" Link="Includes\ServiceProviderExtensions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\Guard.cs" Link="Includes\Guard.cs" />
</ItemGroup>

Expand Down
1 change: 1 addition & 0 deletions src/OpenTelemetry/AssemblyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using System.Runtime.CompilerServices;

[assembly: InternalsVisibleTo("OpenTelemetry.Tests" + AssemblyInfo.PublicKey)]
[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.InMemory" + AssemblyInfo.PublicKey)]
[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.Prometheus" + AssemblyInfo.PublicKey)]
[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.Prometheus.Tests" + AssemblyInfo.PublicKey)]
[assembly: InternalsVisibleTo("OpenTelemetry.Extensions.Hosting.Tests" + AssemblyInfo.PublicKey)]
Expand Down
33 changes: 17 additions & 16 deletions test/OpenTelemetry.Tests/Metrics/InMemoryExporterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,19 @@

using System.Collections.Generic;
using System.Diagnostics.Metrics;

using OpenTelemetry.Tests;

using Xunit;

namespace OpenTelemetry.Metrics.Tests
{
public class InMemoryExporterTests
{
[Fact(Skip = "To be run after https://github.com/open-telemetry/opentelemetry-dotnet/issues/2361 is fixed")]
[Fact]
public void InMemoryExporterShouldDeepCopyMetricPoints()
{
var exportedItems = new List<Metric>();
var exportedItems = new List<MetricSnapshot>();

using var meter = new Meter(Utils.GetCurrentMethodName());
using var meterProvider = Sdk.CreateMeterProviderBuilder()
Expand All @@ -39,30 +41,29 @@ public void InMemoryExporterShouldDeepCopyMetricPoints()

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

// Emit 10 for the MetricPoint with a single key-vaue pair: ("tag1", "value1")
// TEST 1: Emit 10 for the MetricPoint with a single key-vaue pair: ("tag1", "value1")
counter.Add(10, new KeyValuePair<string, object>("tag1", "value1"));

meterProvider.ForceFlush();

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 readonly var metricPointForFirstExport = ref metricPointsEnumerator.Current;
utpilla marked this conversation as resolved.
Show resolved Hide resolved
Assert.Equal(10, metricPointForFirstExport.GetSumLong());
Assert.Single(exportedItems);
var metric1 = exportedItems[0]; // Only one Metric object is added to the collection at this point
Assert.Single(metric1.MetricPoints);
Assert.Equal(10, metric1.MetricPoints[0].GetSumLong());

// Emit 25 for the MetricPoint with a single key-vaue pair: ("tag1", "value1")
// TEST 2: Emit 25 for the MetricPoint with a single key-vaue pair: ("tag1", "value1")
counter.Add(25, new KeyValuePair<string, object>("tag1", "value1"));

meterProvider.ForceFlush();

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;
Assert.Equal(25, metricPointForSecondExport.GetSumLong());
Assert.Equal(2, exportedItems.Count);
var metric2 = exportedItems[1]; // Second Metric object is added to the collection at this point
Assert.Single(metric2.MetricPoints);
Assert.Equal(25, metric2.MetricPoints[0].GetSumLong());

// MetricPoint.LongValue for the first exporter metric should still be 10
Assert.Equal(10, metricPointForFirstExport.GetSumLong());
// TEST 3: Verify first exported metric is unchanged
// MetricPoint.LongValue for the first exported metric should still be 10
Assert.Equal(10, metric1.MetricPoints[0].GetSumLong());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,8 @@ public void CheckExportForRecordingButNotSampledActivity()
[Fact]
public void CheckExportDrainsBatchOnFailure()
{
using var exporter = new InMemoryExporter<Activity>(null);
using var processor = new BatchActivityExportProcessor(
exporter,
exporter: new FailureExporter<Activity>(),
maxQueueSize: 3,
maxExportBatchSize: 3);

Expand All @@ -208,5 +207,11 @@ public void CheckExportDrainsBatchOnFailure()

Assert.Equal(3, processor.ProcessedCount); // Verify batch was drained even though nothing was exported.
}

private class FailureExporter<T> : BaseExporter<T>
where T : class
{
public override ExportResult Export(in Batch<T> batch) => ExportResult.Failure;
}
}
}