Moved HTTP request telemetry to HttpClient.SendAsync

src/libraries/System.Net.Http/src/System/Net/Http/HttpMessageInvoker.cs

@@ -41,7 +41,26 @@ public virtual HttpResponseMessage Send(HttpRequestMessage request,
             }
             CheckDisposed();
 
-            return _handler.Send(request, cancellationToken);
+            if (HttpTelemetry.Log.IsEnabled() && !request.WasSentByHttpClient())
+            {
+                try
+                {
+                    return _handler.Send(request, cancellationToken);
+                }
+                catch when (LogRequestAborted(telemetryStarted: true))
+                {
+                    // Unreachable as LogRequestAborted will return false
+                    throw;
+                }
+                finally
+                {
+                    HttpTelemetry.Log.RequestStop();
+                }
+            }
+            else
+            {
+                return _handler.Send(request, cancellationToken);
+            }
         }
 
         public virtual Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,
@@ -53,7 +72,40 @@ public virtual Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,
             }
             CheckDisposed();
 
+            if (HttpTelemetry.Log.IsEnabled() && !request.WasSentByHttpClient())
+            {
+                return SendAsyncWithTelemetry(_handler, request, cancellationToken);
+            }
+
             return _handler.SendAsync(request, cancellationToken);
+
+            static async Task<HttpResponseMessage> SendAsyncWithTelemetry(HttpMessageHandler handler, HttpRequestMessage request, CancellationToken cancellationToken)
+            {
+                HttpTelemetry.Log.RequestStart(request);
+
+                try
+                {
+                    return await handler.SendAsync(request, cancellationToken).ConfigureAwait(false);
+                }
+                catch when (LogRequestAborted(telemetryStarted: true))
+                {
+                    // Unreachable as LogRequestAborted will return false
+                    throw;
+                }
+                finally
+                {
+                    HttpTelemetry.Log.RequestStop();
+                }
+            }
+        }
+
+        internal static bool LogRequestAborted(bool telemetryStarted)
+        {
+            if (HttpTelemetry.Log.IsEnabled() && telemetryStarted)
+            {
+                HttpTelemetry.Log.RequestAborted();
+            }
+            return false;
         }
 
         public void Dispose()

src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs

@@ -14,10 +14,9 @@ public class HttpRequestMessage : IDisposable
     {
         private const int MessageNotYetSent = 0;
         private const int MessageAlreadySent = 1;
-        private const int MessageAlreadySent_StopNotYetCalled = 2;
 
         // Track whether the message has been sent.
-        // The message should only be sent if this field is equal to MessageNotYetSent.
+        // The message shouldn't be sent again if this field is equal to MessageAlreadySent.
         private int _sendStatus = MessageNotYetSent;
 
         private HttpMethod _method;
@@ -201,32 +200,10 @@ private void InitializeValues(HttpMethod method, Uri? requestUri)
 
         internal bool MarkAsSent()
         {
-            return Interlocked.CompareExchange(ref _sendStatus, MessageAlreadySent, MessageNotYetSent) == MessageNotYetSent;
+            return Interlocked.Exchange(ref _sendStatus, MessageAlreadySent) == MessageNotYetSent;
         }
 
-        internal void MarkAsTrackedByTelemetry()
-        {
-            Debug.Assert(_sendStatus != MessageAlreadySent_StopNotYetCalled);
-            _sendStatus = MessageAlreadySent_StopNotYetCalled;
-        }
-
-        internal void OnAborted() => OnStopped(aborted: true);
-
-        internal void OnStopped(bool aborted = false)
-        {
-            if (HttpTelemetry.Log.IsEnabled())
-            {
-                if (Interlocked.Exchange(ref _sendStatus, MessageAlreadySent) == MessageAlreadySent_StopNotYetCalled)
-                {
-                    if (aborted)
-                    {
-                        HttpTelemetry.Log.RequestAborted();
-                    }
-
-                    HttpTelemetry.Log.RequestStop();
-                }
-            }
-        }
+        internal bool WasSentByHttpClient() => _sendStatus == MessageAlreadySent;
 
         #region IDisposable Members
 
@@ -242,8 +219,6 @@ protected virtual void Dispose(bool disposing)
                     _content.Dispose();
                 }
             }
-
-            OnStopped();
         }
 
         public void Dispose()

src/libraries/System.Net.Http/src/System/Net/Http/HttpTelemetry.cs

@@ -36,12 +36,26 @@ internal sealed class HttpTelemetry : EventSource
         // - A stop event's event id must be next one after its start event.
 
         [Event(1, Level = EventLevel.Informational)]
-        public void RequestStart(string scheme, string host, int port, string pathAndQuery, int versionMajor, int versionMinor)
+        private void RequestStart(string scheme, string host, int port, string pathAndQuery, int versionMajor, int versionMinor)
         {
             Interlocked.Increment(ref _startedRequests);
             WriteEvent(eventId: 1, scheme, host, port, pathAndQuery, versionMajor, versionMinor);
         }
 
+        [NonEvent]
+        public void RequestStart(HttpRequestMessage request)
+        {
+            Debug.Assert(request.RequestUri != null);
+
+            RequestStart(
+                request.RequestUri.Scheme,
+                request.RequestUri.IdnHost,
+                request.RequestUri.Port,
+                request.RequestUri.PathAndQuery,
+                request.Version.Major,
+                request.Version.Minor);
+        }
+
         [Event(2, Level = EventLevel.Informational)]
         public void RequestStop()
         {
@@ -99,6 +113,12 @@ public void ResponseHeadersBegin()
             WriteEvent(eventId: 9);
         }
 
+        [Event(10, Level = EventLevel.Informational)]
+        public void ResponseContentBegin()
+        {
+            WriteEvent(eventId: 10);
+        }
+
         protected override void OnEventCommand(EventCommandEventArgs command)
         {
             if (command.Command == EventCommand.Enable)

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs

@@ -351,12 +351,6 @@ private void Complete()
                     w.Dispose();
                     _creditWaiter = null;
                 }
-
-                if (HttpTelemetry.Log.IsEnabled())
-                {
-                    bool aborted = _requestCompletionState == StreamCompletionState.Failed || _responseCompletionState == StreamCompletionState.Failed;
-                    _request.OnStopped(aborted);
-                }
             }
 
             private void Cancel()
@@ -391,8 +385,6 @@ private void Cancel()
                 {
                     _waitSource.SetResult(true);
                 }
-
-                if (HttpTelemetry.Log.IsEnabled()) _request.OnAborted();
             }
 
             // Returns whether the waiter should be signalled or not.
@@ -1155,10 +1147,6 @@ private void CloseResponseBody()
                 {
                     Cancel();
                 }
-                else
-                {
-                    _request.OnStopped();
-                }
 
                 _responseBuffer.Dispose();
             }

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs

@@ -120,8 +120,6 @@ protected void Dispose(bool disposing)
 
                 _pool.DecrementConnectionCount();
 
-                if (HttpTelemetry.Log.IsEnabled()) _currentRequest?.OnAborted();
-
                 if (disposing)
                 {
                     GC.SuppressFinalize(this);
@@ -1872,8 +1870,6 @@ private void CompleteResponse()
             Debug.Assert(_currentRequest != null, "Expected the connection to be associated with a request.");
             Debug.Assert(_writeOffset == 0, "Everything in write buffer should have been flushed.");
 
-            if (HttpTelemetry.Log.IsEnabled()) _currentRequest.OnStopped();
-
             // Disassociate the connection from a request.
             _currentRequest = null;
 

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs

@@ -358,53 +358,6 @@ public ValueTask<HttpResponseMessage> SendProxyConnectAsync(HttpRequestMessage r
         }
 
         public ValueTask<HttpResponseMessage> SendAsync(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken)
-        {
-            if (HttpTelemetry.Log.IsEnabled())
-            {
-                // [ActiveIssue("https://github.com/dotnet/runtime/issues/40896")]
-                if (request.Version.Major < 3 && request.RequestUri != null)
-                {
-                    return SendAsyncWithLogging(request, async, doRequestAuth, cancellationToken);
-                }
-            }
-
-            return SendAsyncHelper(request, async, doRequestAuth, cancellationToken);
-        }
-
-        private async ValueTask<HttpResponseMessage> SendAsyncWithLogging(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken)
-        {
-            Debug.Assert(request.RequestUri != null);
-            HttpTelemetry.Log.RequestStart(
-                request.RequestUri.Scheme,
-                request.RequestUri.IdnHost,
-                request.RequestUri.Port,
-                request.RequestUri.PathAndQuery,
-                request.Version.Major,
-                request.Version.Minor);
-
-            request.MarkAsTrackedByTelemetry();
-
-            try
-            {
-                return await SendAsyncHelper(request, async, doRequestAuth, cancellationToken).ConfigureAwait(false);
-            }
-            catch when (LogException(request))
-            {
-                // This code should never run.
-                throw;
-            }
-
-            static bool LogException(HttpRequestMessage request)
-            {
-                request.OnAborted();
-
-                // Returning false means the catch handler isn't run.
-                // So the exception isn't considered to be caught so it will now propagate up the stack.
-                return false;
-            }
-        }
-
-        private ValueTask<HttpResponseMessage> SendAsyncHelper(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken)
         {
             if (_proxy == null)
             {

src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs

@@ -327,12 +327,12 @@ static void Verify(HttpResponseMessage message)
         }
 
         [Fact]
-        public void GetAsync_CustomException_Synchronous_ThrowsException()
+        public async Task GetAsync_CustomException_Synchronous_ThrowsException()
         {
             var e = new FormatException();
             using (var client = new HttpClient(new CustomResponseHandler((r, c) => { throw e; })))
             {
-                FormatException thrown = Assert.Throws<FormatException>(() => { client.GetAsync(CreateFakeUri()); });
+                FormatException thrown = await Assert.ThrowsAsync<FormatException>(() => client.GetAsync(CreateFakeUri()));
                 Assert.Same(e, thrown);
             }
         }