Suppress instrumentation in Zipkin exporter (#1150)

* Suppress instrumentation in Zipkin exporter

* Check Sdk.SuppressInstrumentation on ActivityListener.ActivityStarted/ActivityStopped

* Remove raw post in test to Zipkin endpoint URL

* Use ForceFlush on exporter

* Call ForceFlush twice

* Remove IsInternalUrl

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>

src/OpenTelemetry.Exporter.Zipkin/ZipkinExporter.cs

@@ -59,6 +59,9 @@ namespace OpenTelemetry.Exporter.Zipkin
         /// <inheritdoc/>
         public override ExportResult Export(in Batch<Activity> batch)
         {
+            // Prevent Zipkin's HTTP operations from being instrumented.
+            using var scope = SuppressInstrumentationScope.Begin();
+
             try
             {
                 // take a snapshot of the batch

src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs

@@ -44,32 +44,10 @@ namespace OpenTelemetry.Instrumentation.Http
         /// </summary>
         public Func<HttpRequestMessage, bool> FilterFunc { get; set; }
 
-        internal static bool IsInternalUrl(Uri requestUri)
-        {
-            var originalString = requestUri.OriginalString;
-
-            // zipkin
-            if (originalString.Contains(":9411/api/v2/spans"))
-            {
-                return true;
-            }
-
-            return false;
-        }
-
         internal bool EventFilter(string activityName, object arg1)
         {
             if (TryParseHttpRequestMessage(activityName, arg1, out HttpRequestMessage requestMessage))
             {
-                Uri requestUri;
-                if (requestMessage.Method == HttpMethod.Post && (requestUri = requestMessage.RequestUri) != null)
-                {
-                    if (IsInternalUrl(requestUri))
-                    {
-                        return false;
-                    }
-                }
-
                 return this.FilterFunc?.Invoke(requestMessage) ?? true;
             }
 

src/OpenTelemetry.Instrumentation.Http/HttpWebRequestInstrumentationOptions.netfx.cs

@@ -47,14 +47,6 @@ namespace OpenTelemetry.Instrumentation.Http
 
         internal bool EventFilter(HttpWebRequest request)
         {
-            Uri requestUri;
-            if (request.Method == "POST"
-                && (requestUri = request.RequestUri) != null
-                && HttpClientInstrumentationOptions.IsInternalUrl(requestUri))
-            {
-                return false;
-            }
-
             return this.FilterFunc?.Invoke(request) ?? true;
         }
     }

src/OpenTelemetry/Trace/TracerProviderSdk.cs

@@ -68,7 +68,7 @@ namespace OpenTelemetry.Trace
                 // Callback when Activity is started.
                 ActivityStarted = (activity) =>
                 {
-                    if (activity.IsAllDataRequested)
+                    if (!Sdk.SuppressInstrumentation && activity.IsAllDataRequested)
                     {
                         activity.SetResource(this.resource);
                         this.processor?.OnStart(activity);
@@ -78,7 +78,7 @@ namespace OpenTelemetry.Trace
                 // Callback when Activity is stopped.
                 ActivityStopped = (activity) =>
                 {
-                    if (activity.IsAllDataRequested)
+                    if (!Sdk.SuppressInstrumentation && activity.IsAllDataRequested)
                     {
                         this.processor?.OnEnd(activity);
                     }

test/OpenTelemetry.Exporter.Zipkin.Tests/OpenTelemetry.Exporter.Zipkin.Tests.csproj

@@ -28,6 +28,7 @@
   <ItemGroup>
     <ProjectReference Include="$(RepoRoot)\src\OpenTelemetry.Api\OpenTelemetry.Api.csproj" />
     <ProjectReference Include="$(RepoRoot)\src\OpenTelemetry.Exporter.Zipkin\OpenTelemetry.Exporter.Zipkin.csproj" />
+    <ProjectReference Include="$(RepoRoot)\src\OpenTelemetry.Instrumentation.Http\OpenTelemetry.Instrumentation.Http.csproj" />
   </ItemGroup>
 
 </Project>

test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs

@@ -22,6 +22,7 @@ using System.IO;
 using System.Linq;
 using System.Net;
 using System.Text;
+using System.Threading;
 using OpenTelemetry.Exporter.Zipkin.Implementation;
 using OpenTelemetry.Resources;
 using OpenTelemetry.Tests;
@@ -89,6 +90,48 @@ namespace OpenTelemetry.Exporter.Zipkin.Tests
             Assert.Throws<ArgumentNullException>(() => builder.AddZipkinExporter());
         }
 
+        [Fact]
+        public void ZipkinExporter_SuppresssesInstrumentation()
+        {
+            const string ActivitySourceName = "zipkin.test";
+            Guid requestId = Guid.NewGuid();
+            TestActivityProcessor testActivityProcessor = new TestActivityProcessor();
+
+            int endCalledCount = 0;
+
+            testActivityProcessor.EndAction =
+                (a) =>
+                {
+                    endCalledCount++;
+                };
+
+            var exporterOptions = new ZipkinExporterOptions();
+            exporterOptions.ServiceName = "test-zipkin";
+            exporterOptions.Endpoint = new Uri($"http://{this.testServerHost}:{this.testServerPort}/api/v2/spans?requestId={requestId}");
+            var zipkinExporter = new ZipkinExporter(exporterOptions);
+            var exportActivityProcessor = new BatchExportActivityProcessor(zipkinExporter);
+
+            var openTelemetrySdk = Sdk.CreateTracerProviderBuilder()
+                .AddSource(ActivitySourceName)
+                .AddProcessor(testActivityProcessor)
+                .AddProcessor(exportActivityProcessor)
+                .AddHttpClientInstrumentation()
+                .Build();
+
+            var source = new ActivitySource(ActivitySourceName);
+            var activity = source.StartActivity("Test Zipkin Activity");
+            activity?.Stop();
+
+            // We call ForceFlush on the exporter twice, so that in the event
+            // of a regression, this should give any operations performed in
+            // the Zipkin exporter itself enough time to be instrumented and
+            // loop back through the exporter.
+            exportActivityProcessor.ForceFlush();
+            exportActivityProcessor.ForceFlush();
+
+            Assert.Equal(1, endCalledCount);
+        }
+
         [Theory]
         [InlineData(true)]
         [InlineData(false)]

test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.netcore31.cs

@@ -250,31 +250,6 @@ namespace OpenTelemetry.Instrumentation.Http.Tests
             Assert.Equal(2, processor.Invocations.Count); // OnShutdown/Dispose called.
         }
 
-        [Fact]
-        public async Task HttpClientInstrumentationFiltersOutRequestsToExporterEndpoints()
-        {
-            var processor = new Mock<ActivityProcessor>();
-
-            using (Sdk.CreateTracerProviderBuilder()
-                               .AddHttpClientInstrumentation()
-                               .AddProcessor(processor.Object)
-                               .Build())
-            {
-                using var c = new HttpClient();
-                using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(100));
-                try
-                {
-                    await c.PostAsync("https://localhost:9411/api/v2/spans", new StringContent(string.Empty), cts.Token);
-                }
-                catch
-                {
-                    // ignore all, whatever response is, we don't want anything tracked
-                }
-            }
-
-            Assert.Equal(2, processor.Invocations.Count); // OnShutdown/Dispose called.
-        }
-
         public void Dispose()
         {
             this.serverLifeTime?.Dispose();