ZipkinExporter Stuck Batch Fix

[CodeBlanch]

Description

If ZipkinExporter can't connect to Zipkin service the batch will get suck and the underlying CircularBuffer will continue to fill up.

How To Reproduce

• Configure Examples.AspNetCore with "UseExporter": "zipkin" and launch without running Zipkin, or an invalid URL for the service.
• Open the /weatherforecast route to generate a span.
• Look in the VS Output window. These two messages will write in an endless loop:

OpenTelemetry-Instrumentation - EventId: [1], EventName: [NullActivity], Message: [Current Activity is NULL in the 'System.Net.Http.Response' callback. Activity will not be recorded.]
Exception thrown: 'System.Net.Http.HttpRequestException' in System.Private.CoreLib.dll
OpenTelemetry-Exporter-Zipkin - EventId: [1], EventName: [FailedExport], Message: [Failed to export activities: 'System.Net.Http.HttpRequestException: No connection could be made because the target machine actively refused it.
 ---> System.Net.Sockets.SocketException (10061): No connection could be made because the target machine actively refused it.
   at System.Net.Http.ConnectHelper.ConnectAsync(String host, Int32 port, CancellationToken cancellationToken)
   --- End of inner exception stack trace ---
   at System.Net.Http.ConnectHelper.ConnectAsync(String host, Int32 port, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.ConnectAsync(HttpRequestMessage request, Boolean allowHttp2, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.CreateHttp11ConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.GetHttpConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.DiagnosticsHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.FinishSendAsyncBuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts)
   at OpenTelemetry.Exporter.Zipkin.ZipkinExporter.Export(Batch`1& batch) in C:\Source\CodeBlanch\open-telemetry\opentelemetry-dotnet\src\OpenTelemetry.Exporter.Zipkin\ZipkinExporter.cs:line 84']

Changes

• Added a Dispose to Batch<T> which BatchExportProcessor<T> will now call.

• We no longer write the "NullActivity" event when Sdk.SuppressInstrumentation is triggered.

• CHANGELOG.md updated for non-trivial changes

[eddynaka]

should we add this check inside the instrumentationEventSource? because we can forget if someone adds a new Log, no?

[CodeBlanch]

Good question. If you look at this method there are 4 or 5 SuppressInstrumentation checks inside it. Smells like it could use a bit of cleanup. /cc @alanwest

[codecov]

Codecov Report

Merging #1726 (1584a6a) into main (cc93a78) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1726      +/-   ##
==========================================
+ Coverage   82.44%   82.50%   +0.05%     
==========================================
  Files         250      250              
  Lines        6797     6803       +6     
==========================================
+ Hits         5604     5613       +9     
+ Misses       1193     1190       -3     

Impacted Files	Coverage Δ
src/OpenTelemetry/Batch.cs	97.43% <100.00%> (+0.21%)	⬆️
src/OpenTelemetry/BatchExportProcessor.cs	93.24% <100.00%> (+0.18%)	⬆️
...cSourceInstrumentation/DiagnosticSourceListener.cs	80.00% <100.00%> (+0.83%)	⬆️
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs	82.35% <0.00%> (+5.88%)	⬆️
...penTelemetry.Exporter.InMemory/InMemoryExporter.cs	100.00% <0.00%> (+12.50%)	⬆️

[reyang]

[cijothomas]

Lets add to changelog about this bugfix.

[cijothomas]

Added a Dispose to Batch which BaseExportProcessor will now call.

Please update this to say BatchExportProcessor.

[CodeBlanch]

@cijothomas Updated