diff --git a/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md b/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md index 8b6931dad03..73990740562 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md @@ -23,6 +23,10 @@ * Span tags will no longer be populated with Resource Attributes. ([#1663](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1663)) +* Spans will no longer be held in memory indefinitely when `ZipkinExporter` + cannot connect to the configured endpoint. + ([#1726](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1726)) + ## 1.0.0-rc1.1 Released 2020-Nov-17 diff --git a/src/OpenTelemetry/.publicApi/net452/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net452/PublicAPI.Unshipped.txt index 8b685d3fbaa..5017dd7d64f 100644 --- a/src/OpenTelemetry/.publicApi/net452/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net452/PublicAPI.Unshipped.txt @@ -13,6 +13,7 @@ OpenTelemetry.BaseProcessor.ParentProvider.get -> OpenTelemetry.BaseProvider OpenTelemetry.BaseProcessor.Shutdown(int timeoutMilliseconds = -1) -> bool OpenTelemetry.Batch OpenTelemetry.Batch.Batch() -> void +OpenTelemetry.Batch.Dispose() -> void OpenTelemetry.Batch.Enumerator OpenTelemetry.Batch.Enumerator.Current.get -> T OpenTelemetry.Batch.Enumerator.Dispose() -> void diff --git a/src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt index 3c19a7dd787..899e11cd643 100644 --- a/src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt @@ -13,6 +13,7 @@ OpenTelemetry.BaseProcessor.ParentProvider.get -> OpenTelemetry.BaseProvider OpenTelemetry.BaseProcessor.Shutdown(int timeoutMilliseconds = -1) -> bool OpenTelemetry.Batch OpenTelemetry.Batch.Batch() -> void +OpenTelemetry.Batch.Dispose() -> void OpenTelemetry.Batch.Enumerator OpenTelemetry.Batch.Enumerator.Current.get -> T OpenTelemetry.Batch.Enumerator.Dispose() -> void diff --git a/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt index 48f96ed9f3f..9cf4125f8ef 100644 --- a/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/net461/PublicAPI.Unshipped.txt @@ -14,6 +14,7 @@ OpenTelemetry.BaseProcessor.ParentProvider.get -> OpenTelemetry.BaseProvider OpenTelemetry.BaseProcessor.Shutdown(int timeoutMilliseconds = -1) -> bool OpenTelemetry.Batch OpenTelemetry.Batch.Batch() -> void +OpenTelemetry.Batch.Dispose() -> void OpenTelemetry.Batch.Enumerator OpenTelemetry.Batch.Enumerator.Current.get -> T OpenTelemetry.Batch.Enumerator.Dispose() -> void diff --git a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt index 48f96ed9f3f..9cf4125f8ef 100644 --- a/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt @@ -14,6 +14,7 @@ OpenTelemetry.BaseProcessor.ParentProvider.get -> OpenTelemetry.BaseProvider OpenTelemetry.BaseProcessor.Shutdown(int timeoutMilliseconds = -1) -> bool OpenTelemetry.Batch OpenTelemetry.Batch.Batch() -> void +OpenTelemetry.Batch.Dispose() -> void OpenTelemetry.Batch.Enumerator OpenTelemetry.Batch.Enumerator.Current.get -> T OpenTelemetry.Batch.Enumerator.Dispose() -> void diff --git a/src/OpenTelemetry/Batch.cs b/src/OpenTelemetry/Batch.cs index 553622e0c8c..6f81c75e62c 100644 --- a/src/OpenTelemetry/Batch.cs +++ b/src/OpenTelemetry/Batch.cs @@ -26,18 +26,18 @@ namespace OpenTelemetry /// Stores a batch of completed objects to be exported. /// /// The type of object in the . - public readonly struct Batch + public readonly struct Batch : IDisposable where T : class { private readonly T item; private readonly CircularBuffer circularBuffer; - private readonly int maxSize; + private readonly long targetCount; internal Batch(T item) { this.item = item ?? throw new ArgumentNullException(nameof(item)); this.circularBuffer = null; - this.maxSize = 1; + this.targetCount = 1; } internal Batch(CircularBuffer circularBuffer, int maxSize) @@ -46,7 +46,20 @@ internal Batch(CircularBuffer circularBuffer, int maxSize) this.item = null; this.circularBuffer = circularBuffer ?? throw new ArgumentNullException(nameof(circularBuffer)); - this.maxSize = maxSize; + this.targetCount = circularBuffer.RemovedCount + Math.Min(maxSize, circularBuffer.Count); + } + + /// + public void Dispose() + { + if (this.circularBuffer != null) + { + // Drain anything left in the batch. + while (this.circularBuffer.RemovedCount < this.targetCount) + { + this.circularBuffer.Read(); + } + } } /// @@ -56,7 +69,7 @@ internal Batch(CircularBuffer circularBuffer, int maxSize) public Enumerator GetEnumerator() { return this.circularBuffer != null - ? new Enumerator(this.circularBuffer, this.maxSize) + ? new Enumerator(this.circularBuffer, this.targetCount) : new Enumerator(this.item); } @@ -66,20 +79,20 @@ public Enumerator GetEnumerator() public struct Enumerator : IEnumerator { private readonly CircularBuffer circularBuffer; - private int count; + private long targetCount; internal Enumerator(T item) { this.Current = item; this.circularBuffer = null; - this.count = -1; + this.targetCount = -1; } - internal Enumerator(CircularBuffer circularBuffer, int maxSize) + internal Enumerator(CircularBuffer circularBuffer, long targetCount) { this.Current = null; this.circularBuffer = circularBuffer; - this.count = Math.Min(maxSize, circularBuffer.Count); + this.targetCount = targetCount; } /// @@ -100,20 +113,19 @@ public bool MoveNext() if (circularBuffer == null) { - if (this.count >= 0) + if (this.targetCount >= 0) { this.Current = null; return false; } - this.count++; + this.targetCount++; return true; } - if (this.count > 0) + if (circularBuffer.RemovedCount < this.targetCount) { this.Current = circularBuffer.Read(); - this.count--; return true; } diff --git a/src/OpenTelemetry/BatchExportProcessor.cs b/src/OpenTelemetry/BatchExportProcessor.cs index a41ee5c647c..94a529352ad 100644 --- a/src/OpenTelemetry/BatchExportProcessor.cs +++ b/src/OpenTelemetry/BatchExportProcessor.cs @@ -217,7 +217,10 @@ private void ExporterProc() if (this.circularBuffer.Count > 0) { - this.exporter.Export(new Batch(this.circularBuffer, this.maxExportBatchSize)); + using (var batch = new Batch(this.circularBuffer, this.maxExportBatchSize)) + { + this.exporter.Export(batch); + } this.dataExportedNotification.Set(); this.dataExportedNotification.Reset(); diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 30d6267a8fa..50a92c4b37f 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -30,6 +30,9 @@ resource's attributes in a conflict. We've rectified to follow a recent change to the spec. We previously prioritized "this" resource's tags. ([#1728](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1728)) +* `BatchExportProcessor` will now flush any remaining spans left in a `Batch` + after the export operation has completed. + ([#1726](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1726)) ## 1.0.0-rc1.1 diff --git a/src/OpenTelemetry/DiagnosticSourceInstrumentation/DiagnosticSourceListener.cs b/src/OpenTelemetry/DiagnosticSourceInstrumentation/DiagnosticSourceListener.cs index 910799bec49..2e0f1236fe6 100644 --- a/src/OpenTelemetry/DiagnosticSourceInstrumentation/DiagnosticSourceListener.cs +++ b/src/OpenTelemetry/DiagnosticSourceInstrumentation/DiagnosticSourceListener.cs @@ -41,7 +41,11 @@ public void OnNext(KeyValuePair value) { if (!this.handler.SupportsNullActivity && Activity.Current == null) { - InstrumentationEventSource.Log.NullActivity(value.Key); + if (!Sdk.SuppressInstrumentation) + { + InstrumentationEventSource.Log.NullActivity(value.Key); + } + return; } diff --git a/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorTest.cs b/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorTest.cs index 120def16b7c..38cacff2a15 100644 --- a/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorTest.cs +++ b/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorTest.cs @@ -176,5 +176,25 @@ public void CheckExportForRecordingButNotSampledActivity() Assert.Empty(exportedItems); Assert.Equal(0, processor.ProcessedCount); } + + [Fact] + public void CheckExportDrainsBatchOnFailure() + { + using var exporter = new InMemoryExporter(null); + using var processor = new BatchActivityExportProcessor( + exporter, + maxQueueSize: 3, + maxExportBatchSize: 3); + + var activity = new Activity("start"); + activity.ActivityTraceFlags = ActivityTraceFlags.Recorded; + + processor.OnEnd(activity); + processor.OnEnd(activity); + processor.OnEnd(activity); + processor.Shutdown(); + + Assert.Equal(3, processor.ProcessedCount); // Verify batch was drained even though nothing was exported. + } } } diff --git a/test/OpenTelemetry.Tests/Trace/BatchTest.cs b/test/OpenTelemetry.Tests/Trace/BatchTest.cs index 230264010a4..010077a7424 100644 --- a/test/OpenTelemetry.Tests/Trace/BatchTest.cs +++ b/test/OpenTelemetry.Tests/Trace/BatchTest.cs @@ -48,6 +48,38 @@ public void CheckValidConstructors() } } + [Fact] + public void CheckDispose() + { + var value = "a"; + var batch = new Batch(value); + batch.Dispose(); // A test to make sure it doesn't bomb on a null CircularBuffer. + + var circularBuffer = new CircularBuffer(10); + circularBuffer.Add(value); + circularBuffer.Add(value); + circularBuffer.Add(value); + batch = new Batch(circularBuffer, 10); // Max size = 10 + batch.GetEnumerator().MoveNext(); + Assert.Equal(3, circularBuffer.AddedCount); + Assert.Equal(1, circularBuffer.RemovedCount); + batch.Dispose(); // Test anything remaining in the batch is drained when disposed. + Assert.Equal(3, circularBuffer.AddedCount); + Assert.Equal(3, circularBuffer.RemovedCount); + batch.Dispose(); // Verify we don't go into an infinite loop or thrown when empty. + + circularBuffer = new CircularBuffer(10); + circularBuffer.Add(value); + circularBuffer.Add(value); + circularBuffer.Add(value); + batch = new Batch(circularBuffer, 2); // Max size = 2 + Assert.Equal(3, circularBuffer.AddedCount); + Assert.Equal(0, circularBuffer.RemovedCount); + batch.Dispose(); // Test the batch is drained up to max size. + Assert.Equal(3, circularBuffer.AddedCount); + Assert.Equal(2, circularBuffer.RemovedCount); + } + [Fact] public void CheckEnumerator() { @@ -63,6 +95,33 @@ public void CheckEnumerator() this.ValidateEnumerator(enumerator, value); } + [Fact] + public void CheckMultipleEnumerator() + { + var value = "a"; + var circularBuffer = new CircularBuffer(10); + circularBuffer.Add(value); + circularBuffer.Add(value); + circularBuffer.Add(value); + var batch = new Batch(circularBuffer, 10); + + int itemsProcessed = 0; + foreach (var item in batch) + { + itemsProcessed++; + } + + Assert.Equal(3, itemsProcessed); + + itemsProcessed = 0; + foreach (var item in batch) + { + itemsProcessed++; + } + + Assert.Equal(0, itemsProcessed); + } + [Fact] public void CheckEnumeratorResetException() {