Add httpClient .net core instrumentation

samples/Exporters/Web/Controllers/WeatherForecastController.cs

@@ -1,6 +1,8 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Net;
+using System.Net.Http;
 using Microsoft.AspNetCore.Mvc;
 
 namespace API.Controllers
@@ -14,9 +16,12 @@ public class WeatherForecastController : ControllerBase
             "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"
         };
 
+        private static HttpClient httpClient = new HttpClient();
+
         [HttpGet]
         public IEnumerable<WeatherForecast> Get()
         {
+            var res = httpClient.GetStringAsync("http://google.com").Result;
             var rng = new Random();
             return Enumerable.Range(1, 5).Select(index => new WeatherForecast
             {

samples/Exporters/Web/Startup.cs

@@ -38,7 +38,7 @@ public void ConfigureServices(IServiceCollection services)
             });
 
             OpenTelemetrySdk.EnableOpenTelemetry(
-                (builder) => builder.AddRequestInstrumentation()
+                (builder) => builder.AddRequestInstrumentation().AddDependencyInstrumentation()
                 .UseConsoleActivityExporter(opt => opt.DisplayAsJson = opt.DisplayAsJson));
 
             services.AddOpenTelemetry((sp, builder) =>

src/OpenTelemetry.Instrumentation.Dependencies/DependenciesInstrumentation.cs

@@ -41,12 +41,10 @@ public DependenciesInstrumentation(TracerFactoryBase tracerFactory, HttpClientIn
 
             var assemblyVersion = typeof(DependenciesInstrumentation).Assembly.GetName().Version;
 
-            var httpClientListener = new HttpClientInstrumentation(tracerFactory.GetTracer(nameof(HttpClientInstrumentation), "semver:" + assemblyVersion), httpOptions ?? new HttpClientInstrumentationOptions());
             var azureClientsListener = new AzureClientsInstrumentation(tracerFactory.GetTracer(nameof(AzureClientsInstrumentation), "semver:" + assemblyVersion));
             var azurePipelineListener = new AzurePipelineInstrumentation(tracerFactory.GetTracer(nameof(AzurePipelineInstrumentation), "semver:" + assemblyVersion));
             var sqlClientListener = new SqlClientInstrumentation(tracerFactory.GetTracer(nameof(SqlClientInstrumentation), "semver:" + assemblyVersion), sqlOptions ?? new SqlClientInstrumentationOptions());
 
-            this.instrumentations.Add(httpClientListener);
             this.instrumentations.Add(azureClientsListener);
             this.instrumentations.Add(azurePipelineListener);
             this.instrumentations.Add(sqlClientListener);

src/OpenTelemetry.Instrumentation.Dependencies/HttpClientInstrumentation.cs

@@ -15,7 +15,6 @@
 // </copyright>
 using System;
 using OpenTelemetry.Instrumentation.Dependencies.Implementation;
-using OpenTelemetry.Trace;
 
 namespace OpenTelemetry.Instrumentation.Dependencies
 {
@@ -29,20 +28,18 @@ public class HttpClientInstrumentation : IDisposable
         /// <summary>
         /// Initializes a new instance of the <see cref="HttpClientInstrumentation"/> class.
         /// </summary>
-        /// <param name="tracer">Tracer to record traced with.</param>
-        public HttpClientInstrumentation(Tracer tracer)
-            : this(tracer, new HttpClientInstrumentationOptions())
+        public HttpClientInstrumentation()
+            : this(new HttpClientInstrumentationOptions())
         {
         }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="HttpClientInstrumentation"/> class.
         /// </summary>
-        /// <param name="tracer">Tracer to record traced with.</param>
         /// <param name="options">Configuration options for dependencies instrumentation.</param>
-        public HttpClientInstrumentation(Tracer tracer, HttpClientInstrumentationOptions options)
+        public HttpClientInstrumentation(HttpClientInstrumentationOptions options)
         {
-            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpHandlerDiagnosticListener(tracer, options), options.EventFilter);
+            this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpHandlerDiagnosticListener(options), options.EventFilter);
             this.diagnosticSourceSubscriber.Subscribe();
         }
 

src/OpenTelemetry.Instrumentation.Dependencies/Implementation/HttpHandlerDiagnosticListener.cs

@@ -23,22 +23,28 @@
 using System.Threading.Tasks;
 using OpenTelemetry.Context.Propagation;
 using OpenTelemetry.Trace;
+using OpenTelemetry.Trace.Samplers;
 
 namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
 {
     internal class HttpHandlerDiagnosticListener : ListenerHandler
     {
         private static readonly Regex CoreAppMajorVersionCheckRegex = new Regex("^\\.NETCoreApp,Version=v(\\d+)\\.", RegexOptions.Compiled | RegexOptions.IgnoreCase);
 
+        // hard-coded Sampler here, just to prototype.
+        // Either .NET will provide an new API to avoid Instrumentation being aware of sampling.
+        // or we'll move the Sampler to come from OpenTelemetryBuilder, and not hardcoded.
+        private readonly ActivitySampler sampler = new AlwaysOnActivitySampler();
+
         private readonly PropertyFetcher startRequestFetcher = new PropertyFetcher("Request");
         private readonly PropertyFetcher stopResponseFetcher = new PropertyFetcher("Response");
         private readonly PropertyFetcher stopExceptionFetcher = new PropertyFetcher("Exception");
         private readonly PropertyFetcher stopRequestStatusFetcher = new PropertyFetcher("RequestTaskStatus");
         private readonly bool httpClientSupportsW3C = false;
         private readonly HttpClientInstrumentationOptions options;
 
-        public HttpHandlerDiagnosticListener(Tracer tracer, HttpClientInstrumentationOptions options)
-            : base("HttpHandlerDiagnosticListener", tracer)
+        public HttpHandlerDiagnosticListener(HttpClientInstrumentationOptions options)
+            : base("HttpHandlerDiagnosticListener", null)
         {
             var framework = Assembly
                 .GetEntryAssembly()?
@@ -73,84 +79,84 @@ public override void OnStartActivity(Activity activity, object payload)
                 return;
             }
 
-            this.Tracer.StartActiveSpanFromActivity(HttpTagHelper.GetOperationNameForHttpMethod(request.Method), activity, SpanKind.Client, out var span);
+            // TODO: Avoid the reflection hack once .NET ships new Activity with Kind settable.
+            activity.GetType().GetProperty("Kind").SetValue(activity, ActivityKind.Client);
+            activity.DisplayName = HttpTagHelper.GetOperationNameForHttpMethod(request.Method);
+
+            var samplingParameters = new ActivitySamplingParameters(
+                activity.Context,
+                activity.TraceId,
+                activity.DisplayName,
+                activity.Kind,
+                activity.Tags,
+                activity.Links);
+
+            // TODO: Find a way to avoid Instrumentation being tied to Sampler
+            var samplingDecision = this.sampler.ShouldSample(samplingParameters);
+            activity.IsAllDataRequested = samplingDecision.IsSampled;
 
-            if (span.IsRecording)
+            if (activity.IsAllDataRequested)
             {
-                span.PutComponentAttribute("http");
-                span.PutHttpMethodAttribute(HttpTagHelper.GetNameForHttpMethod(request.Method));
-                span.PutHttpHostAttribute(HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri));
-                span.PutHttpRawUrlAttribute(request.RequestUri.OriginalString);
+                activity.AddTag(SpanAttributeConstants.ComponentKey, "http");
+                activity.AddTag(SpanAttributeConstants.HttpMethodKey, HttpTagHelper.GetNameForHttpMethod(request.Method));
+                activity.AddTag(SpanAttributeConstants.HttpHostKey, HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri));
+                activity.AddTag(SpanAttributeConstants.HttpUrlKey, request.RequestUri.OriginalString);
 
                 if (this.options.SetHttpFlavor)
                 {
-                    span.PutHttpFlavorAttribute(HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version));
+                    activity.AddTag(SpanAttributeConstants.HttpFlavorKey, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version));
                 }
             }
 
             if (!(this.httpClientSupportsW3C && this.options.TextFormat is TraceContextFormat))
             {
-                this.options.TextFormat.Inject(span.Context, request, (r, k, v) => r.Headers.Add(k, v));
+                // TODO: implement this
+                // this.options.TextFormat.Inject(span.Context, request, (r, k, v) => r.Headers.Add(k, v));
             }
         }
 
         public override void OnStopActivity(Activity activity, object payload)
         {
-            const string EventNameSuffix = ".OnStopActivity";
-            var span = this.Tracer.CurrentSpan;
-            try
+            if (activity.IsAllDataRequested)
             {
-                if (span == null || !span.Context.IsValid)
-                {
-                    InstrumentationEventSource.Log.NullOrBlankSpan(nameof(HttpHandlerDiagnosticListener) + EventNameSuffix);
-                    return;
-                }
+                var requestTaskStatus = this.stopRequestStatusFetcher.Fetch(payload) as TaskStatus?;
 
-                if (span.IsRecording)
+                if (requestTaskStatus.HasValue)
                 {
-                    var requestTaskStatus = this.stopRequestStatusFetcher.Fetch(payload) as TaskStatus?;
-
-                    if (requestTaskStatus.HasValue)
+                    if (requestTaskStatus != TaskStatus.RanToCompletion)
                     {
-                        if (requestTaskStatus != TaskStatus.RanToCompletion)
+                        if (requestTaskStatus == TaskStatus.Canceled)
                         {
-                            if (requestTaskStatus == TaskStatus.Canceled)
-                            {
-                                span.Status = Status.Cancelled;
-                            }
-                            else if (requestTaskStatus != TaskStatus.Faulted)
-                            {
-                                // Faults are handled in OnException and should already have a span.Status of Unknown w/ Description.
-                                span.Status = Status.Unknown;
-                            }
+                            Status status = Status.Cancelled;
+                            activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
+                        }
+                        else if (requestTaskStatus != TaskStatus.Faulted)
+                        {
+                            // Faults are handled in OnException and should already have a span.Status of Unknown w/ Description.
+                            Status status = Status.Unknown;
+                            activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
                         }
                     }
+                }
 
-                    if (this.stopResponseFetcher.Fetch(payload) is HttpResponseMessage response)
-                    {
-                        // response could be null for DNS issues, timeouts, etc...
-                        span.PutHttpStatusCode((int)response.StatusCode, response.ReasonPhrase);
-                    }
+                if (this.stopResponseFetcher.Fetch(payload) is HttpResponseMessage response)
+                {
+                    // response could be null for DNS issues, timeouts, etc...
+                    activity.AddTag(SpanAttributeConstants.HttpStatusCodeKey, response.StatusCode.ToString());
+
+                    Status status = SpanHelper.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode);
+                    activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
+                    activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, response.ReasonPhrase);
                 }
             }
-            finally
-            {
-                span?.End();
-            }
         }
 
         public override void OnException(Activity activity, object payload)
         {
             const string EventNameSuffix = ".OnException";
             var span = this.Tracer.CurrentSpan;
 
-            if (span == null || !span.Context.IsValid)
-            {
-                InstrumentationEventSource.Log.NullOrBlankSpan(nameof(HttpHandlerDiagnosticListener) + EventNameSuffix);
-                return;
-            }
-
-            if (span.IsRecording)
+            if (activity.IsAllDataRequested)
             {
                 if (!(this.stopExceptionFetcher.Fetch(payload) is Exception exc))
                 {
@@ -165,19 +171,22 @@ public override void OnException(Activity activity, object payload)
                         switch (exception.SocketErrorCode)
                         {
                             case SocketError.HostNotFound:
-                                span.Status = Status.InvalidArgument.WithDescription(exc.Message);
+                                Status status = Status.InvalidArgument;
+                                activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
+                                activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, exc.Message);
                                 return;
                         }
                     }
 
                     if (exc.InnerException != null)
                     {
                         span.Status = Status.Unknown.WithDescription(exc.Message);
+                        Status status = Status.Unknown;
+                        activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
+                        activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, exc.Message);
                     }
                 }
             }
-
-            // Note: Span.End() is not called here on purpose, OnStopActivity is called after OnException for this listener.
         }
     }
 }

src/OpenTelemetry.Instrumentation.Dependencies/OpenTelemetryBuilderExtensions.cs

@@ -15,6 +15,7 @@
 // </copyright>
 
 using System;
+using OpenTelemetry.Instrumentation.Dependencies;
 using OpenTelemetry.Instrumentation.Dependencies.Implementation;
 
 namespace OpenTelemetry.Trace.Configuration
@@ -36,12 +37,59 @@ public static OpenTelemetryBuilder AddDependencyInstrumentation(this OpenTelemet
                 throw new ArgumentNullException(nameof(builder));
             }
 
+            builder.AddHttpClientDependencyInstrumentation(null);
 #if NET461
             builder.AddHttpWebRequestDependencyInstrumentation();
 #endif
             return builder;
         }
 
+        /// <summary>
+        /// Enables the outgoing requests automatic data collection for all supported activity sources.
+        /// </summary>
+        /// <param name="builder"><see cref="OpenTelemetryBuilder"/> being configured.</param>
+        /// <param name="configureHttpClientInstrumentationOptions">HttpClient configuration options.</param>
+        /// <returns>The instance of <see cref="OpenTelemetryBuilder"/> to chain the calls.</returns>
+        public static OpenTelemetryBuilder AddDependencyInstrumentation(
+            this OpenTelemetryBuilder builder,
+            Action<HttpClientInstrumentationOptions> configureHttpClientInstrumentationOptions = null)
+        {
+            if (builder == null)
+            {
+                throw new ArgumentNullException(nameof(builder));
+            }
+
+            builder.AddHttpClientDependencyInstrumentation(configureHttpClientInstrumentationOptions);
+#if NET461
+            builder.AddHttpWebRequestDependencyInstrumentation();
+#endif
+            return builder;
+        }
+
+        /// <summary>
+        /// Enables the outgoing requests automatic data collection for HttpClient.
+        /// </summary>
+        /// <param name="builder"><see cref="OpenTelemetryBuilder"/> being configured.</param>
+        /// <param name="configureHttpClientInstrumentationOptions">HttpClient configuration options.</param>
+        /// <returns>The instance of <see cref="OpenTelemetryBuilder"/> to chain the calls.</returns>
+        public static OpenTelemetryBuilder AddHttpClientDependencyInstrumentation(
+            this OpenTelemetryBuilder builder,
+            Action<HttpClientInstrumentationOptions> configureHttpClientInstrumentationOptions)
+        {
+            if (builder == null)
+            {
+                throw new ArgumentNullException(nameof(builder));
+            }
+
+            builder.AddActivitySource(string.Empty);
+            var httpClientOptions = new HttpClientInstrumentationOptions();
+            configureHttpClientInstrumentationOptions?.Invoke(httpClientOptions);
+
+            // TODO: decide who is responsible for dispose upon shutdown.
+            new HttpClientInstrumentation(httpClientOptions);
+            return builder;
+        }
+
 #if NET461
         /// <summary>
         /// Enables the outgoing requests automatic data collection for .NET Framework HttpWebRequest activity source.

src/OpenTelemetry.Instrumentation.Dependencies/TracerBuilderExtensions.cs

@@ -39,7 +39,6 @@ public static TracerBuilder AddDependencyInstrumentation(this TracerBuilder buil
             return builder
                 .AddInstrumentation((t) => new AzureClientsInstrumentation(t))
                 .AddInstrumentation((t) => new AzurePipelineInstrumentation(t))
-                .AddInstrumentation((t) => new HttpClientInstrumentation(t))
                 .AddInstrumentation((t) => new SqlClientInstrumentation(t));
         }
 
@@ -69,7 +68,6 @@ public static TracerBuilder AddDependencyInstrumentation(this TracerBuilder buil
             return builder
                 .AddInstrumentation((t) => new AzureClientsInstrumentation(t))
                 .AddInstrumentation((t) => new AzurePipelineInstrumentation(t))
-                .AddInstrumentation((t) => new HttpClientInstrumentation(t, httpOptions))
                 .AddInstrumentation((t) => new SqlClientInstrumentation(t, sqlOptions));
         }
     }

src/OpenTelemetry/Trace/Export/SimpleActivityProcessor.cs

@@ -18,7 +18,6 @@
 using System.Threading;
 using System.Threading.Tasks;
 using OpenTelemetry.Internal;
-using OpenTelemetry.Trace.Samplers;
 
 namespace OpenTelemetry.Trace.Export
 {