ZipkinExporter: HttpClientFactory option (#2654)

src/OpenTelemetry.Exporter.Jaeger/JaegerExporterHelperExtensions.cs

@@ -56,38 +56,33 @@ private static TracerProviderBuilder AddJaegerExporter(
         {
             configure?.Invoke(options);
 
-            if (options.Protocol == JaegerExportProtocol.HttpBinaryThrift && options.HttpClientFactory == null)
+            if (serviceProvider != null
+                && options.Protocol == JaegerExportProtocol.HttpBinaryThrift
+                && options.HttpClientFactory == JaegerExporterOptions.DefaultHttpClientFactory)
             {
-                if (serviceProvider != null)
+                options.HttpClientFactory = () =>
                 {
-                    options.HttpClientFactory = () =>
+                    Type httpClientFactoryType = Type.GetType("System.Net.Http.IHttpClientFactory, Microsoft.Extensions.Http", throwOnError: false);
+                    if (httpClientFactoryType != null)
                     {
-                        Type httpClientFactoryType = Type.GetType("System.Net.Http.IHttpClientFactory, Microsoft.Extensions.Http", throwOnError: false);
-                        if (httpClientFactoryType != null)
+                        object httpClientFactory = serviceProvider.GetService(httpClientFactoryType);
+                        if (httpClientFactory != null)
                         {
-                            object httpClientFactory = serviceProvider.GetService(httpClientFactoryType);
-                            if (httpClientFactory != null)
+                            MethodInfo createClientMethod = httpClientFactoryType.GetMethod(
+                                "CreateClient",
+                                BindingFlags.Public | BindingFlags.Instance,
+                                binder: null,
+                                new Type[] { typeof(string) },
+                                modifiers: null);
+                            if (createClientMethod != null)
                             {
-                                MethodInfo createClientMethod = httpClientFactoryType.GetMethod(
-                                    "CreateClient",
-                                    BindingFlags.Public | BindingFlags.Instance,
-                                    binder: null,
-                                    new Type[] { typeof(string) },
-                                    modifiers: null);
-                                if (createClientMethod != null)
-                                {
-                                    return (HttpClient)createClientMethod.Invoke(httpClientFactory, new object[] { "JaegerExporter" });
-                                }
+                                return (HttpClient)createClientMethod.Invoke(httpClientFactory, new object[] { "JaegerExporter" });
                             }
                         }
+                    }
 
-                        return new HttpClient();
-                    };
-                }
-                else
-                {
-                    options.HttpClientFactory = () => new HttpClient();
-                }
+                    return new HttpClient();
+                };
             }
 
             var jaegerExporter = new JaegerExporter(options);

src/OpenTelemetry.Exporter.Jaeger/JaegerExporterOptions.cs

@@ -40,6 +40,8 @@ public class JaegerExporterOptions
         internal const string OTelAgentPortEnvVarKey = "OTEL_EXPORTER_JAEGER_AGENT_PORT";
         internal const string OTelEndpointEnvVarKey = "OTEL_EXPORTER_JAEGER_ENDPOINT";
 
+        internal static readonly Func<HttpClient> DefaultHttpClientFactory = () => new HttpClient();
+
         public JaegerExporterOptions()
         {
             if (EnvironmentVariableHelper.LoadString(OtelProtocolEnvVarKey, out string protocolEnvVar)
@@ -115,9 +117,9 @@ public JaegerExporterOptions()
         /// instance can be resolved through the application <see
         /// cref="IServiceProvider"/> then an <see cref="HttpClient"/> will be
         /// created through the factory with the name "JaegerExporter" otherwise
-        /// an <see cref="HttpClient"/> will be instantiated directly."/></item>
+        /// an <see cref="HttpClient"/> will be instantiated directly.</item>
         /// </list>
         /// </remarks>
-        public Func<HttpClient> HttpClientFactory { get; set; }
+        public Func<HttpClient> HttpClientFactory { get; set; } = DefaultHttpClientFactory;
     }
 }

src/OpenTelemetry.Exporter.Zipkin/.publicApi/net461/PublicAPI.Unshipped.txt

@@ -0,0 +1,2 @@
+OpenTelemetry.Exporter.ZipkinExporterOptions.HttpClientFactory.get -> System.Func<System.Net.Http.HttpClient>
+OpenTelemetry.Exporter.ZipkinExporterOptions.HttpClientFactory.set -> void

src/OpenTelemetry.Exporter.Zipkin/.publicApi/net5.0/PublicAPI.Unshipped.txt

@@ -0,0 +1,2 @@
+OpenTelemetry.Exporter.ZipkinExporterOptions.HttpClientFactory.get -> System.Func<System.Net.Http.HttpClient>
+OpenTelemetry.Exporter.ZipkinExporterOptions.HttpClientFactory.set -> void

src/OpenTelemetry.Exporter.Zipkin/.publicApi/netstandard2.0/PublicAPI.Unshipped.txt

@@ -0,0 +1,2 @@
+OpenTelemetry.Exporter.ZipkinExporterOptions.HttpClientFactory.get -> System.Func<System.Net.Http.HttpClient>
+OpenTelemetry.Exporter.ZipkinExporterOptions.HttpClientFactory.set -> void

src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md

@@ -10,6 +10,9 @@ Released 2021-Nov-19
   `FormatException` if it fails to parse any of the supported environment
   variables.
 
+* Added `HttpClientFactory` option
+  ([#2654](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2654))
+
 ## 1.2.0-beta1
 
 Released 2021-Oct-08

src/OpenTelemetry.Exporter.Zipkin/README.md

@@ -26,19 +26,29 @@ take precedence over the environment variables.
 
 ### Configuration using Properties
 
+* `BatchExportProcessorOptions`: Configuration options for the batch exporter.
+  Only used if ExportProcessorType is set to Batch.
+
+* `Endpoint`: URI address to receive telemetry (default
+  `http://localhost:9411/api/v2/spans`).
+
+* `ExportProcessorType`: Whether the exporter should use [Batch or Simple
+  exporting
+  processor](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#built-in-span-processors).
+
+* `HttpClientFactory`: A factory function called to create the `HttpClient`
+  instance that will be used at runtime to transmit spans over HTTP. See
+  [Configure HttpClient](#configure-httpclient) for more details.
+
+* `MaxPayloadSizeInBytes`: Maximum payload size - for .NET versions **other**
+   than 4.5.2 (default 4096).
+
 * `ServiceName`: Name of the service reporting telemetry. If the `Resource`
    associated with the telemetry has "service.name" defined, then it'll be
    preferred over this option.
-* `Endpoint`: URI address to receive telemetry (default `http://localhost:9411/api/v2/spans`).
-* `UseShortTraceIds`: Whether the trace's ID should be shortened before
-   sending to Zipkin (default false).
-* `MaxPayloadSizeInBytes`: Maximum payload size - for .NET versions
-   **other** than 4.5.2 (default 4096).
-* `ExportProcessorType`: Whether the exporter should use
-  [Batch or Simple exporting processor](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#built-in-span-processors)
-  .
-* `BatchExportProcessorOptions`: Configuration options for the batch exporter.
-  Only used if ExportProcessorType is set to Batch.
+
+* `UseShortTraceIds`: Whether the trace's ID should be shortened before sending
+   to Zipkin (default false).
 
 See
 [`TestZipkinExporter.cs`](../../examples/Console/TestZipkinExporter.cs)
@@ -65,6 +75,38 @@ values of the `ZipkinExporterOptions`.
 `FormatException` is thrown in case of an invalid value for any of the
 supported environment variables.
 
+## Configure HttpClient
+
+The `HttpClientFactory` option is provided on `ZipkinExporterOptions` for users
+who want to configure the `HttpClient` used by the `ZipkinExporter`. Simply
+replace the function with your own implementation if you want to customize the
+generated `HttpClient`:
+
+```csharp
+services.AddOpenTelemetryTracing((builder) => builder
+    .AddZipkinExporter(o => o.HttpClientFactory = () =>
+    {
+        HttpClient client = new HttpClient();
+        client.DefaultRequestHeaders.Add("X-MyCustomHeader", "value");
+        return client;
+    }));
+```
+
+For users using
+[IHttpClientFactory](https://docs.microsoft.com/dotnet/architecture/microservices/implement-resilient-applications/use-httpclientfactory-to-implement-resilient-http-requests)
+you may also customize the named "ZipkinExporter" `HttpClient` using the
+built-in `AddHttpClient` extension:
+
+```csharp
+services.AddHttpClient(
+    "ZipkinExporter",
+     configureClient: (client) =>
+        client.DefaultRequestHeaders.Add("X-MyCustomHeader", "value"));
+```
+
+Note: The single instance returned by `HttpClientFactory` is reused by all
+export requests.
+
 ## References
 
 * [OpenTelemetry Project](https://opentelemetry.io/)

src/OpenTelemetry.Exporter.Zipkin/ZipkinExporter.cs

@@ -52,7 +52,7 @@ public ZipkinExporter(ZipkinExporterOptions options, HttpClient client = null)
 
             this.options = options;
             this.maxPayloadSizeInBytes = (!options.MaxPayloadSizeInBytes.HasValue || options.MaxPayloadSizeInBytes <= 0) ? ZipkinExporterOptions.DefaultMaxPayloadSizeInBytes : options.MaxPayloadSizeInBytes.Value;
-            this.httpClient = client ?? new HttpClient();
+            this.httpClient = client ?? options.HttpClientFactory?.Invoke() ?? throw new InvalidOperationException("ZipkinExporter was missing HttpClientFactory or it returned null.");
         }
 
         internal ZipkinEndpoint LocalEndpoint { get; private set; }

src/OpenTelemetry.Exporter.Zipkin/ZipkinExporterHelperExtensions.cs

@@ -15,6 +15,8 @@
 // </copyright>
 
 using System;
+using System.Net.Http;
+using System.Reflection;
 using OpenTelemetry.Exporter;
 using OpenTelemetry.Internal;
 
@@ -40,17 +42,48 @@ public static TracerProviderBuilder AddZipkinExporter(this TracerProviderBuilder
             {
                 return deferredTracerProviderBuilder.Configure((sp, builder) =>
                 {
-                    AddZipkinExporter(builder, sp.GetOptions<ZipkinExporterOptions>(), configure);
+                    AddZipkinExporter(builder, sp.GetOptions<ZipkinExporterOptions>(), configure, sp);
                 });
             }
 
-            return AddZipkinExporter(builder, new ZipkinExporterOptions(), configure);
+            return AddZipkinExporter(builder, new ZipkinExporterOptions(), configure, serviceProvider: null);
         }
 
-        private static TracerProviderBuilder AddZipkinExporter(TracerProviderBuilder builder, ZipkinExporterOptions options, Action<ZipkinExporterOptions> configure = null)
+        private static TracerProviderBuilder AddZipkinExporter(
+            TracerProviderBuilder builder,
+            ZipkinExporterOptions options,
+            Action<ZipkinExporterOptions> configure,
+            IServiceProvider serviceProvider)
         {
             configure?.Invoke(options);
 
+            if (serviceProvider != null && options.HttpClientFactory == ZipkinExporterOptions.DefaultHttpClientFactory)
+            {
+                options.HttpClientFactory = () =>
+                {
+                    Type httpClientFactoryType = Type.GetType("System.Net.Http.IHttpClientFactory, Microsoft.Extensions.Http", throwOnError: false);
+                    if (httpClientFactoryType != null)
+                    {
+                        object httpClientFactory = serviceProvider.GetService(httpClientFactoryType);
+                        if (httpClientFactory != null)
+                        {
+                            MethodInfo createClientMethod = httpClientFactoryType.GetMethod(
+                                "CreateClient",
+                                BindingFlags.Public | BindingFlags.Instance,
+                                binder: null,
+                                new Type[] { typeof(string) },
+                                modifiers: null);
+                            if (createClientMethod != null)
+                            {
+                                return (HttpClient)createClientMethod.Invoke(httpClientFactory, new object[] { "ZipkinExporter" });
+                            }
+                        }
+                    }
+
+                    return new HttpClient();
+                };
+            }
+
             var zipkinExporter = new ZipkinExporter(options);
 
             if (options.ExportProcessorType == ExportProcessorType.Simple)

src/OpenTelemetry.Exporter.Zipkin/ZipkinExporterOptions.cs

@@ -16,6 +16,7 @@
 
 using System;
 using System.Diagnostics;
+using System.Net.Http;
 using OpenTelemetry.Internal;
 using OpenTelemetry.Trace;
 
@@ -36,6 +37,8 @@ public sealed class ZipkinExporterOptions
         internal const string ZipkinEndpointEnvVar = "OTEL_EXPORTER_ZIPKIN_ENDPOINT";
         internal const string DefaultZipkinEndpoint = "http://localhost:9411/api/v2/spans";
 
+        internal static readonly Func<HttpClient> DefaultHttpClientFactory = () => new HttpClient();
+
         /// <summary>
         /// Initializes a new instance of the <see cref="ZipkinExporterOptions"/> class.
         /// Initializes zipkin endpoint.
@@ -73,5 +76,23 @@ public ZipkinExporterOptions()
         /// Gets or sets the BatchExportProcessor options. Ignored unless ExportProcessorType is BatchExporter.
         /// </summary>
         public BatchExportProcessorOptions<Activity> BatchExportProcessorOptions { get; set; } = new BatchExportActivityProcessorOptions();
+
+        /// <summary>
+        /// Gets or sets the factory function called to create the <see
+        /// cref="HttpClient"/> instance that will be used at runtime to
+        /// transmit spans over HTTP. The returned instance will be reused for
+        /// all export invocations.
+        /// </summary>
+        /// <remarks>
+        /// Note: The default behavior when using the <see
+        /// cref="ZipkinExporterHelperExtensions.AddZipkinExporter(TracerProviderBuilder,
+        /// Action{ZipkinExporterOptions})"/> extension is if an <a
+        /// href="https://docs.microsoft.com/dotnet/api/system.net.http.ihttpclientfactory">IHttpClientFactory</a>
+        /// instance can be resolved through the application <see
+        /// cref="IServiceProvider"/> then an <see cref="HttpClient"/> will be
+        /// created through the factory with the name "ZipkinExporter" otherwise
+        /// an <see cref="HttpClient"/> will be instantiated directly.
+        /// </remarks>
+        public Func<HttpClient> HttpClientFactory { get; set; } = DefaultHttpClientFactory;
     }
 }

test/OpenTelemetry.Exporter.Jaeger.Tests/JaegerExporterTests.cs

@@ -18,6 +18,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
+using Microsoft.Extensions.DependencyInjection;
 using OpenTelemetry.Exporter.Jaeger.Implementation;
 using OpenTelemetry.Exporter.Jaeger.Implementation.Tests;
 using OpenTelemetry.Resources;
@@ -43,6 +44,71 @@ public void JaegerTraceExporter_ctor_NullServiceNameAllowed()
             Assert.NotNull(jaegerTraceExporter);
         }
 
+        [Fact]
+        public void UserHttpFactoryCalled()
+        {
+            JaegerExporterOptions options = new JaegerExporterOptions();
+
+            var defaultFactory = options.HttpClientFactory;
+
+            int invocations = 0;
+            options.Protocol = JaegerExportProtocol.HttpBinaryThrift;
+            options.HttpClientFactory = () =>
+            {
+                invocations++;
+                return defaultFactory();
+            };
+
+            using (var exporter = new JaegerExporter(options))
+            {
+                Assert.Equal(1, invocations);
+            }
+
+            using (var provider = Sdk.CreateTracerProviderBuilder()
+                .AddJaegerExporter(o =>
+                {
+                    o.Protocol = JaegerExportProtocol.HttpBinaryThrift;
+                    o.HttpClientFactory = options.HttpClientFactory;
+                })
+                .Build())
+            {
+                Assert.Equal(2, invocations);
+            }
+
+            options.HttpClientFactory = null;
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                using var exporter = new JaegerExporter(options);
+            });
+
+            options.HttpClientFactory = () => null;
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                using var exporter = new JaegerExporter(options);
+            });
+        }
+
+        [Fact]
+        public void ServiceProviderHttpClientFactoryInvoked()
+        {
+            IServiceCollection services = new ServiceCollection();
+
+            services.AddHttpClient();
+
+            int invocations = 0;
+
+            services.AddHttpClient("JaegerExporter", configureClient: (client) => invocations++);
+
+            services.AddOpenTelemetryTracing(builder => builder.AddJaegerExporter(
+                o => o.Protocol = JaegerExportProtocol.HttpBinaryThrift));
+
+            using var serviceProvider = services.BuildServiceProvider();
+
+            var tracerProvider = serviceProvider.GetRequiredService<TracerProvider>();
+
+            Assert.Equal(1, invocations);
+        }
+
         [Fact]
         public void JaegerTraceExporter_SetResource_UpdatesServiceName()
         {

test/OpenTelemetry.Exporter.Jaeger.Tests/OpenTelemetry.Exporter.Jaeger.Tests.csproj

@@ -16,10 +16,13 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
     </PackageReference>
     <DotNetCliToolReference Include="dotnet-xunit" Version="$(DotNetXUnitCliVer)" />
+    <PackageReference Include="Microsoft.Extensions.Hosting" Version="$(MicrosoftExtensionsHostingPkgVer)" />
+    <PackageReference Include="Microsoft.Extensions.Http" Version="3.1.20" />
   </ItemGroup>
 
   <ItemGroup>
     <ProjectReference Include="$(RepoRoot)\src\OpenTelemetry.Exporter.Jaeger\OpenTelemetry.Exporter.Jaeger.csproj" />
+    <ProjectReference Include="$(RepoRoot)\src\OpenTelemetry.Extensions.Hosting\OpenTelemetry.Extensions.Hosting.csproj" />
 
     <Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\EventSourceTestHelper.cs" Link="Includes\EventSourceTestHelper.cs" />
     <Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\TestActivityProcessor.cs" Link="Includes\TestActivityProcessor.cs" />

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

@@ -22,10 +22,13 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
     </PackageReference>
     <DotNetCliToolReference Include="dotnet-xunit" Version="$(DotNetXUnitCliVer)" />
+    <PackageReference Include="Microsoft.Extensions.Hosting" Version="$(MicrosoftExtensionsHostingPkgVer)" />
+    <PackageReference Include="Microsoft.Extensions.Http" Version="3.1.20" />
   </ItemGroup>
 
   <ItemGroup>
     <ProjectReference Include="$(RepoRoot)\src\OpenTelemetry.Exporter.Zipkin\OpenTelemetry.Exporter.Zipkin.csproj" />
+    <ProjectReference Include="$(RepoRoot)\src\OpenTelemetry.Extensions.Hosting\OpenTelemetry.Extensions.Hosting.csproj" />
     <ProjectReference Include="$(RepoRoot)\src\OpenTelemetry.Instrumentation.Http\OpenTelemetry.Instrumentation.Http.csproj" />
   </ItemGroup>