From 2a228f9234988294f37d024bcdd898786636c0a7 Mon Sep 17 00:00:00 2001
From: Vishwesh Bankwar <vishweshbankwar@users.noreply.github.com>
Date: Fri, 17 Nov 2023 12:07:00 -0800
Subject: [PATCH] Refactor HttpWebRequestActivitySource (#5055)

---
 .../HttpWebRequestActivitySource.netfx.cs     | 75 ++++++++-----------
 1 file changed, 33 insertions(+), 42 deletions(-)

diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
index 9a0fd30e78d..d37dd07070d 100644
--- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
+++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
@@ -204,8 +204,6 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity)
                 activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode));
             }
 
-            activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode));
-
             try
             {
                 TracingOptions.EnrichWithHttpWebResponse?.Invoke(activity, response);
@@ -243,45 +241,15 @@ private static string GetErrorType(Exception exception)
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    private static void AddExceptionTags(Exception exception, Activity activity, out HttpStatusCode? statusCode, out Version protocolVersion)
+    private static void AddExceptionEvent(Exception exception, Activity activity)
     {
         Debug.Assert(activity != null, "Activity must not be null");
 
-        statusCode = null;
-        protocolVersion = null;
-
         if (!activity.IsAllDataRequested)
         {
             return;
         }
 
-        ActivityStatusCode status;
-
-        if (exception is WebException wexc && wexc.Response is HttpWebResponse response)
-        {
-            statusCode = response.StatusCode;
-            protocolVersion = response.ProtocolVersion;
-
-            if (tracingEmitOldAttributes)
-            {
-                activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)statusCode);
-            }
-
-            if (tracingEmitNewAttributes)
-            {
-                activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, HttpTagHelper.GetProtocolVersionString(protocolVersion));
-                activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, (int)statusCode);
-            }
-
-            status = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)statusCode);
-        }
-        else
-        {
-            status = ActivityStatusCode.Error;
-        }
-
-        activity.SetStatus(status);
-
         if (TracingOptions.RecordException)
         {
             activity.RecordException(exception);
@@ -401,6 +369,7 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
         HttpStatusCode? httpStatusCode = null;
         string errorType = null;
         Version protocolVersion = null;
+        ActivityStatusCode activityStatus = ActivityStatusCode.Unset;
 
         // Activity may be null if we are not tracing in these cases:
         // 1. No listeners
@@ -417,14 +386,30 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
             if (result is Exception ex)
             {
                 errorType = GetErrorType(ex);
-                if (activity != null)
-                {
-                    AddExceptionTags(ex, activity, out httpStatusCode, out protocolVersion);
-                }
-                else if (ex is WebException wexc && wexc.Response is HttpWebResponse response)
+                if (ex is WebException wexc && wexc.Response is HttpWebResponse response)
                 {
                     httpStatusCode = response.StatusCode;
                     protocolVersion = response.ProtocolVersion;
+                    activityStatus = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode);
+                    if (activityStatus == ActivityStatusCode.Error)
+                    {
+                        // override the errorType to statusCode for failures.
+                        errorType = TelemetryHelper.GetStatusCodeString(response.StatusCode);
+                    }
+
+                    if (activity != null)
+                    {
+                        AddResponseTags(response, activity);
+                        AddExceptionEvent(ex, activity);
+                    }
+                }
+                else
+                {
+                    activityStatus = ActivityStatusCode.Error;
+                    if (activity != null)
+                    {
+                        AddExceptionEvent(ex, activity);
+                    }
                 }
             }
             else
@@ -465,9 +450,11 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
                     protocolVersion = response.ProtocolVersion;
                 }
 
-                if (SpanHelper.ResolveSpanStatusForHttpStatusCode(ActivityKind.Client, (int)httpStatusCode.Value) == ActivityStatusCode.Error)
+                activityStatus = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)httpStatusCode.Value);
+
+                if (activityStatus == ActivityStatusCode.Error)
                 {
-                    // override the errorType to statusCode for failures.
+                    // set the errorType to statusCode for failures.
                     errorType = TelemetryHelper.GetStatusCodeString(httpStatusCode.Value);
                 }
             }
@@ -477,9 +464,13 @@ private static void ProcessResult(IAsyncResult asyncResult, AsyncCallback asyncC
             HttpInstrumentationEventSource.Log.FailedProcessResult(ex);
         }
 
-        if (tracingEmitNewAttributes && errorType != null)
+        if (activity != null && activity.IsAllDataRequested)
         {
-            activity?.SetTag(SemanticConventions.AttributeErrorType, errorType);
+            activity.SetStatus(activityStatus);
+            if (tracingEmitNewAttributes && errorType != null)
+            {
+                activity.SetTag(SemanticConventions.AttributeErrorType, errorType);
+            }
         }
 
         activity?.Stop();