add meaningful http tags in tracing handler

Src/zipkin4net/Src/Transport/Http/TracingHandler.cs

@@ -3,6 +3,7 @@
 using System.Net.Http.Headers;
 using System.Threading.Tasks;
 using zipkin4net.Propagation;
+using zipkin4net.Tracers.Zipkin.Thrift;
 
 namespace zipkin4net.Transport.Http
 {
@@ -11,32 +12,41 @@ public class TracingHandler : DelegatingHandler
         private readonly IInjector<HttpHeaders> _injector;
         private readonly string _serviceName;
         private readonly Func<HttpRequestMessage, string> _getClientTraceRpc;
+        private readonly bool _logHttpHost;
 
         /// <summary>
         /// Create a Tracing Handler
         /// </summary>
         /// <param name="serviceName"></param>
         /// <param name="httpMessageHandler">if not set or null then it set an <see cref="HttpClientHandler"/> as inner handler</param>
         /// <param name="getClientTraceRpc"></param>
-        public TracingHandler(string serviceName, HttpMessageHandler httpMessageHandler = null, Func<HttpRequestMessage, string> getClientTraceRpc = null)
-        : this(Propagations.B3String.Injector<HttpHeaders>((carrier, key, value) => carrier.Add(key, value)), serviceName, httpMessageHandler ?? new HttpClientHandler(), getClientTraceRpc)
+        /// <param name="logHttpHost"></param>
+        public TracingHandler(string serviceName, HttpMessageHandler httpMessageHandler = null,
+            Func<HttpRequestMessage, string> getClientTraceRpc = null, bool logHttpHost = false)
+        : this(Propagations.B3String.Injector<HttpHeaders>((carrier, key, value) => carrier.Add(key, value)),
+              serviceName, httpMessageHandler ?? new HttpClientHandler(), getClientTraceRpc, logHttpHost)
         { }
 
         /// <summary>
         /// Create a TracingHandler for injection purpose like HttpClientFactory in AspNetCore
         /// </summary>
         /// <param name="serviceName"></param>
         /// <param name="getClientTraceRpc"></param>
+        /// <param name="logHttpHost"></param>
         /// <returns></returns>
-        public static TracingHandler WithoutInnerHandler(string serviceName, Func<HttpRequestMessage, string> getClientTraceRpc = null)
-         =>  new TracingHandler(Propagations.B3String.Injector<HttpHeaders>((carrier, key, value) => carrier.Add(key, value)), serviceName, getClientTraceRpc);
+        public static TracingHandler WithoutInnerHandler(string serviceName,
+            Func<HttpRequestMessage, string> getClientTraceRpc = null, bool logHttpHost = false)
+         =>  new TracingHandler(Propagations.B3String.Injector<HttpHeaders>((carrier, key, value) => carrier.Add(key, value)),
+                serviceName, getClientTraceRpc, logHttpHost);
 
-        private TracingHandler(IInjector<HttpHeaders> injector, string serviceName, HttpMessageHandler httpMessageHandler, Func<HttpRequestMessage, string> getClientTraceRpc = null)
+        private TracingHandler(IInjector<HttpHeaders> injector, string serviceName, HttpMessageHandler httpMessageHandler,
+            Func<HttpRequestMessage, string> getClientTraceRpc = null, bool logHttpHost = false)
             : base(httpMessageHandler)
         {
             _injector = injector;
             _serviceName = serviceName;
             _getClientTraceRpc = getClientTraceRpc ?? (request => request.Method.ToString());
+            _logHttpHost = logHttpHost;
         }
 
         /// <summary>
@@ -45,12 +55,16 @@ private TracingHandler(IInjector<HttpHeaders> injector, string serviceName, Http
         /// <param name="injector"></param>
         /// <param name="serviceName"></param>
         /// <param name="getClientTraceRpc"></param>
-        private TracingHandler(IInjector<HttpHeaders> injector, string serviceName, Func<HttpRequestMessage, string> getClientTraceRpc = null)
+        /// <param name="logHttpHost"></param>
+        private TracingHandler(IInjector<HttpHeaders> injector, string serviceName,
+            Func<HttpRequestMessage, string> getClientTraceRpc = null, bool logHttpHost = false)
         {
             _injector = injector;
             _serviceName = serviceName;
             _getClientTraceRpc = getClientTraceRpc ?? (request => request.Method.ToString());
+            _logHttpHost = logHttpHost;
         }
+
         protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, System.Threading.CancellationToken cancellationToken)
         {
             using (var clientTrace = new ClientTrace(_serviceName, _getClientTraceRpc(request)))
@@ -59,7 +73,24 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
                 {
                     _injector.Inject(clientTrace.Trace.CurrentSpan, request.Headers);
                 }
-                return await clientTrace.TracedActionAsync(base.SendAsync(request, cancellationToken));
+
+                var result = await clientTrace.TracedActionAsync(base.SendAsync(request, cancellationToken));
+
+                if (clientTrace.Trace != null)
+                {
+                    clientTrace.AddAnnotation(Annotations.Tag(zipkinCoreConstants.HTTP_PATH, result.RequestMessage.RequestUri.LocalPath));
+                    clientTrace.AddAnnotation(Annotations.Tag(zipkinCoreConstants.HTTP_METHOD, result.RequestMessage.Method.Method));
+                    if (_logHttpHost)
+                    {
+                        clientTrace.AddAnnotation(Annotations.Tag(zipkinCoreConstants.HTTP_HOST, result.RequestMessage.RequestUri.Host));
+                    }
+                    if (!result.IsSuccessStatusCode)
+                    {
+                        clientTrace.AddAnnotation(Annotations.Tag(zipkinCoreConstants.HTTP_STATUS_CODE, ((int)result.StatusCode).ToString()));
+                    }
+                }
+
+                return result;
             }
         }
     }

Src/zipkin4net/Tests/Transport/Http/T_TracingHandler.cs

@@ -0,0 +1,153 @@
+using Moq;
+using NUnit.Framework;
+using System;
+using System.Net;
+using System.Net.Http;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+using zipkin4net.Annotation;
+using zipkin4net.Dispatcher;
+using zipkin4net.Logger;
+using zipkin4net.Tracers.Zipkin.Thrift;
+using zipkin4net.Transport.Http;
+
+namespace zipkin4net.UTest.Transport.Http
+{
+    [TestFixture]
+    public class T_TracingHandler
+    {
+        private HttpClient httpClient;
+        private Mock<IRecordDispatcher> dispatcher;
+
+        [SetUp]
+        public void SetUp()
+        {
+            dispatcher = new Mock<IRecordDispatcher>();
+
+            TraceManager.ClearTracers();
+            TraceManager.Stop();
+            TraceManager.SamplingRate = 1.0f;
+            TraceManager.Start(new VoidLogger(), dispatcher.Object);
+        }
+
+        [Test]
+        public async Task ShouldLogTagAnnotations()
+        {
+            // Arrange
+            dispatcher
+                .Setup(h => h.Dispatch(It.IsAny<Record>()))
+                .Returns(true);
+
+            var returnStatusCode = HttpStatusCode.BadRequest;
+            var tracingHandler = new TracingHandler("abc", null, null, logHttpHost: true)
+            {
+                InnerHandler = new TestHandler(returnStatusCode)
+            };
+            httpClient = new HttpClient(tracingHandler);
+
+            // Act
+            Trace.Current = Trace.Create();
+            var uri = new Uri("https://abc.com/");
+            var method = HttpMethod.Get;
+            await httpClient.SendAsync(new HttpRequestMessage(HttpMethod.Get, uri));
+
+            // Assert
+            dispatcher
+                .Verify(h =>
+                    h.Dispatch(It.Is<Record>(m =>
+                        m.Annotation is TagAnnotation
+                        && ((TagAnnotation)m.Annotation).Key == zipkinCoreConstants.HTTP_HOST
+                        && ((TagAnnotation)m.Annotation).Value.ToString() == uri.Host)));
+
+            dispatcher
+                .Verify(h =>
+                    h.Dispatch(It.Is<Record>(m =>
+                        m.Annotation is TagAnnotation
+                        && ((TagAnnotation)m.Annotation).Key == zipkinCoreConstants.HTTP_PATH
+                        && ((TagAnnotation)m.Annotation).Value.ToString() == uri.LocalPath)));
+
+            dispatcher
+                .Verify(h =>
+                    h.Dispatch(It.Is<Record>(m =>
+                        m.Annotation is TagAnnotation
+                        && ((TagAnnotation)m.Annotation).Key == zipkinCoreConstants.HTTP_METHOD
+                        && ((TagAnnotation)m.Annotation).Value.ToString() == method.Method)));
+
+            dispatcher
+                .Verify(h =>
+                    h.Dispatch(It.Is<Record>(m =>
+                        m.Annotation is TagAnnotation
+                        && ((TagAnnotation)m.Annotation).Key == zipkinCoreConstants.HTTP_STATUS_CODE
+                        && ((TagAnnotation)m.Annotation).Value.ToString() == ((int)returnStatusCode).ToString())));
+        }
+
+        [Test]
+        public async Task ShouldLogHttpHost()
+        {
+            // Arrange
+            var returnStatusCode = HttpStatusCode.BadRequest;
+            var tracingHandler = new TracingHandler("abc", null, null, logHttpHost: false)
+            {
+                InnerHandler = new TestHandler(returnStatusCode)
+            };
+            httpClient = new HttpClient(tracingHandler);
+
+            dispatcher
+                .Setup(h => h.Dispatch(It.Is<Record>(m =>
+                    m.Annotation is TagAnnotation
+                    && ((TagAnnotation)m.Annotation).Key == zipkinCoreConstants.HTTP_HOST)))
+                .Throws(new Exception("HTTP_HOST Shouldn't be logged."));
+
+            // Act
+            Trace.Current = Trace.Create();
+            var uri = new Uri("https://abc.com/");
+            var method = HttpMethod.Get;
+            await httpClient.SendAsync(new HttpRequestMessage(HttpMethod.Get, uri));
+        }
+
+        [Test]
+        public async Task ShouldNotLogStatusCodeOnHttpCodeSuccess()
+        {
+            // Arrange
+            var tracingHandler = new TracingHandler("abc")
+            {
+                InnerHandler = new TestHandler(HttpStatusCode.OK)
+            };
+            httpClient = new HttpClient(tracingHandler);
+
+
+            dispatcher
+                .Setup(h => h.Dispatch(It.Is<Record>(m =>
+                    m.Annotation is TagAnnotation
+                    && ((TagAnnotation)m.Annotation).Key == zipkinCoreConstants.HTTP_STATUS_CODE)))
+                .Throws(new Exception("HTTP_STATUS_CODE Shouldn't be logged."));
+
+            // Act and assert
+            Trace.Current = Trace.Create();
+            var uri = new Uri("https://abc.com/");
+            var method = HttpMethod.Get;
+            await httpClient.SendAsync(new HttpRequestMessage(HttpMethod.Get, uri));
+        }
+
+        private class TestHandler : DelegatingHandler
+        {
+            private HttpStatusCode _returnStatusCode;
+
+            public TestHandler(HttpStatusCode returnStatusCode)
+            {
+                _returnStatusCode = returnStatusCode;
+            }
+
+            protected override Task<HttpResponseMessage> SendAsync(
+                HttpRequestMessage request, CancellationToken cancellationToken)
+            {
+                return Task.FromResult(new HttpResponseMessage(_returnStatusCode)
+                {
+                    Content = new StringContent("OK", Encoding.UTF8, "application/json"),
+                    RequestMessage = request
+                });
+            }
+        }
+    }
+}

Src/zipkin4net/Tests/Transport/T_ZipkinHttpTraceExtractor.cs

@@ -18,6 +18,8 @@ internal class T_ZipkinHttpTraceExtractor
         public void Setup()
         {
             _mockLogger = new Mock<ILogger>();
+            TraceManager.ClearTracers();
+            TraceManager.Stop();
             TraceManager.Start(_mockLogger.Object);
         }