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

[Exporter.Geneva] Skip dispose for ETW singleton of GenevaMetricExporter #1537

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
2e256ca
Skip dispose for singleton instance
xiang17 Jan 18, 2024
06da054
Merge branch 'main' into xiang17/GenevaExporterFixSingletonDisposeBug
xiang17 Jan 18, 2024
889b514
Update CHANGELOG
xiang17 Jan 18, 2024
5b4bea3
Fix comment style check failure
xiang17 Jan 18, 2024
f9afaa9
Remove question mark for non-nullable references
xiang17 Jan 18, 2024
c9c14e0
Check if it's the singleton variable and remove comment
xiang17 Jan 19, 2024
2edf332
Add unit test to test whether the MetricEtwDataTransport singleton is…
xiang17 Jan 24, 2024
fefb538
Reduce unnessary code
xiang17 Jan 24, 2024
d38d950
Merge branch 'main' into xiang17/GenevaExporterFixSingletonDisposeBug
xiang17 Jan 24, 2024
cd4325c
Fix compiler warning and reword comment
xiang17 Jan 24, 2024
c60a7ab
Update src/OpenTelemetry.Exporter.Geneva/Metrics/Transport/MetricEtwD…
xiang17 Jan 24, 2024
c0ba69a
reword comment
xiang17 Jan 24, 2024
83387e5
Merge branch 'main' into xiang17/GenevaExporterFixSingletonDisposeBug
xiang17 Jan 24, 2024
fbe60e5
Thrown an exception when the Shared MetricEtwDataTransport singleton …
xiang17 Jan 30, 2024
53dec5f
Merge branch 'main' into xiang17/GenevaExporterFixSingletonDisposeBug
xiang17 Jan 30, 2024
f2112b2
Remove the throw exception statement: this exception only happens if …
xiang17 Jan 30, 2024
b637cb0
Rename to singleton instance name to Instance.
xiang17 Jan 30, 2024
e22b316
use `get; private set;` so that the private variable can be removed.
xiang17 Jan 30, 2024
b23bab8
Merge branch 'main' into xiang17/GenevaExporterFixSingletonDisposeBug
vishweshbankwar Feb 1, 2024
4a513cd
Merge branch 'main' into xiang17/GenevaExporterFixSingletonDisposeBug
cijothomas Feb 2, 2024
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
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Exporter.Geneva/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Fix a bug in `GenevaMetricExporter` where the `MetricEtwDataTransport` singleton
is disposed.
([#1537](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1537))

## 1.7.0

Released 2023-Dec-11
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public GenevaMetricExporter(GenevaMetricExporterOptions options)
case TransportProtocol.Unspecified:
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
this.metricDataTransport = MetricEtwDataTransport.Shared;
this.metricDataTransport = MetricEtwDataTransport.Instance;
break;
}
else
Expand Down Expand Up @@ -268,7 +268,12 @@ protected override void Dispose(bool disposing)
{
try
{
this.metricDataTransport?.Dispose();
// The ETW data transport singleton on Windows should NOT be disposed.
vishweshbankwar marked this conversation as resolved.
Show resolved Hide resolved
// On Linux, Unix Domain Socket is used and should be disposed.
if (this.metricDataTransport != MetricEtwDataTransport.Instance)
{
this.metricDataTransport.Dispose();
}
}
catch (Exception ex)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,9 @@ namespace OpenTelemetry.Exporter.Geneva;
internal sealed class MetricEtwDataTransport : EventSource, IMetricDataTransport
{
private readonly int fixedPayloadEndIndex;
private bool isDisposed;

static MetricEtwDataTransport()
{
Shared = new();
}

public static readonly MetricEtwDataTransport Shared;
public static MetricEtwDataTransport Instance { get; private set; } = new();

private MetricEtwDataTransport()
{
Expand Down Expand Up @@ -57,4 +53,23 @@ private void ExternallyAggregatedDoubleDistributionMetric()
private void TLVMetricEvent()
{
}

protected override void Dispose(bool disposing)
{
if (this.isDisposed)
{
return;
}

if (disposing)
{
// No managed resources to release.
// The singleton instance is kept alive for the lifetime of the application.
// Set the instance to null so that future calls to the singleton property can fail explicitly.
Instance = null;
}

this.isDisposed = true;
base.Dispose(disposing);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void Dispose()
return;
}

this.udsDataTransport?.Dispose();
this.udsDataTransport.Dispose();
this.isDisposed = true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public void Dispose()
try
{
// DO NOT Dispose eventBuilder, keyValuePairs, and partCFields as they are static
this.eventProvider?.Dispose();
this.eventProvider.Dispose();
this.serializationData?.Dispose();
}
catch (Exception ex)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public void Dispose()
try
{
// DO NOT Dispose eventBuilder, keyValuePairs, and partCFields as they are static
this.eventProvider?.Dispose();
this.eventProvider.Dispose();
}
catch (Exception ex)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,35 @@ public void SuccessfulExportOnLinux()
}
}

[Fact]
public void MultipleCallsOnWindowsReusesSingletonEtwDataTransport()
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
var singleton = MetricEtwDataTransport.Instance;
this.EmitMetrics("one");
Assert.Equal(singleton, MetricEtwDataTransport.Instance);
this.EmitMetrics("two");
Assert.Equal(singleton, MetricEtwDataTransport.Instance);
}
}

private void EmitMetrics(string attempt)
{
using var meterProviderBuilder = Sdk.CreateMeterProviderBuilder()
.AddMeter("*")
.AddGenevaMetricExporter(x =>
{
x.MetricExportIntervalMilliseconds = 1000;
x.ConnectionString = "Account=OTelGeneva;Namespace=MeteringSample";
})
.Build();

using var meter = new Meter("MeterName", "0.0.1");
var counter = meter.CreateCounter<long>("counter_" + attempt);
counter.Add(1);
}

[Theory]
[InlineData(true)]
[InlineData(false)]
Expand Down
Loading