Migrate ZipkinExporter to BatchExportActivityProcessor (#1103)

* migrate zipkin exporter to BatchExportActivityProcessor * update changelog * fix test failure Co-authored-by: Cijo Thomas <[email protected]>
open-telemetry · Aug 20, 2020 · b4ce36f · b4ce36f
1 parent d13ef78
commit b4ce36f
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 28 deletions.
diff --git a/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md b/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md
@@ -2,7 +2,10 @@
 
 ## Unreleased
 
-Renamed extension method from `UseZipkinExporter` to `AddZipkinExporter`.
+* Renamed extension method from `UseZipkinExporter` to `AddZipkinExporter`
+  ([#1066](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1066))
+* Changed `ZipkinExporter` to use `BatchExportActivityProcessor` by default
+  ([#1103](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1103))
 
 ## 0.4.0-beta.2
 

diff --git a/src/OpenTelemetry.Exporter.Zipkin/ZipkinExporter.cs b/src/OpenTelemetry.Exporter.Zipkin/ZipkinExporter.cs
@@ -37,7 +37,7 @@ namespace OpenTelemetry.Exporter.Zipkin
     /// <summary>
     /// Zipkin exporter.
     /// </summary>
-    public class ZipkinExporter : ActivityExporter
+    public class ZipkinExporter : ActivityExporterSync
     {
         private readonly ZipkinExporterOptions options;
         private readonly HttpClient httpClient;
@@ -57,33 +57,30 @@ public ZipkinExporter(ZipkinExporterOptions options, HttpClient client = null)
         internal ZipkinEndpoint LocalEndpoint { get; }
 
         /// <inheritdoc/>
-        public override async Task<ExportResult> ExportAsync(IEnumerable<Activity> batchActivity, CancellationToken cancellationToken)
+        public override ExportResultSync Export(in Batch<Activity> batch)
         {
             try
             {
-                await this.SendBatchActivityAsync(batchActivity, cancellationToken).ConfigureAwait(false);
+                // take a snapshot of the batch
+                var activities = new List<Activity>();
+                foreach (var activity in batch)
+                {
+                    activities.Add(activity);
+                }
+
+                this.SendBatchActivityAsync(activities, CancellationToken.None).GetAwaiter().GetResult();
 
-                return ExportResult.Success;
+                return ExportResultSync.Success;
             }
             catch (Exception ex)
             {
                 ZipkinExporterEventSource.Log.FailedExport(ex);
 
                 // TODO distinguish retryable exceptions
-                return ExportResult.FailedNotRetryable;
+                return ExportResultSync.Failure;
             }
         }
 
-        /// <inheritdoc/>
-        public override Task ShutdownAsync(CancellationToken cancellationToken)
-        {
-#if NET452
-            return Task.FromResult(0);
-#else
-            return Task.CompletedTask;
-#endif
-        }
-
         private static string ResolveHostAddress(string hostName, AddressFamily family)
         {
             string result = null;

diff --git a/src/OpenTelemetry.Exporter.Zipkin/ZipkinExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.Zipkin/ZipkinExporterHelperExtensions.cs
@@ -42,7 +42,7 @@ public static TracerProviderBuilder AddZipkinExporter(this TracerProviderBuilder
             var zipkinExporter = new ZipkinExporter(exporterOptions);
 
             // TODO: Pick Simple vs Batching based on ZipkinExporterOptions
-            return builder.AddProcessor(new SimpleActivityProcessor(zipkinExporter));
+            return builder.AddProcessor(new BatchExportActivityProcessor(zipkinExporter));
         }
     }
 }
diff --git a/test/Benchmarks/Exporter/ZipkinExporterBenchmarks.cs b/test/Benchmarks/Exporter/ZipkinExporterBenchmarks.cs
@@ -22,6 +22,7 @@
 using BenchmarkDotNet.Attributes;
 using OpenTelemetry.Exporter.Zipkin;
 using OpenTelemetry.Tests;
+using OpenTelemetry.Trace;
 
 namespace Benchmarks.Exporter
 {
@@ -60,21 +61,19 @@ public void GlobalCleanup()
         }
 
         [Benchmark]
-        public async Task ZipkinExporter_ExportAsync()
+        public void ZipkinExporter_Export()
         {
             var zipkinExporter = new ZipkinExporter(
                 new ZipkinExporterOptions
                 {
                     Endpoint = new Uri($"http://{this.serverHost}:{this.serverPort}"),
                 });
+            var processor = new SimpleExportActivityProcessor(zipkinExporter);
 
-            var activities = new List<Activity>(this.NumberOfActivities);
             for (int i = 0; i < this.NumberOfActivities; i++)
             {
-                activities.Add(this.testActivity);
+                processor.OnEnd(this.testActivity);
             }
-
-            await zipkinExporter.ExportAsync(activities, CancellationToken.None).ConfigureAwait(false);
         }
 
         private Activity CreateTestActivity()

diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs
@@ -94,10 +94,8 @@ public void ZipkinExporter_BadArgs()
         [Theory]
         [InlineData(true)]
         [InlineData(false)]
-        public async Task ZipkinExporterIntegrationTest(bool useShortTraceIds)
+        public void ZipkinExporterIntegrationTest(bool useShortTraceIds)
         {
-            var batchActivity = new List<Activity> { CreateTestActivity() };
-
             Guid requestId = Guid.NewGuid();
 
             ZipkinExporter exporter = new ZipkinExporter(
@@ -107,11 +105,11 @@ public async Task ZipkinExporterIntegrationTest(bool useShortTraceIds)
                     UseShortTraceIds = useShortTraceIds,
                 });
 
-            await exporter.ExportAsync(batchActivity, CancellationToken.None).ConfigureAwait(false);
+            var activity = CreateTestActivity();
+            var processor = new SimpleExportActivityProcessor(exporter);
 
-            await exporter.ShutdownAsync(CancellationToken.None).ConfigureAwait(false);
+            processor.OnEnd(activity);
 
-            var activity = batchActivity[0];
             var context = activity.Context;
 
             var timestamp = activity.StartTimeUtc.ToEpochMicroseconds();