fix: http 400 status is not error on server (#2904)

open-telemetry · Feb 16, 2022 · af44186 · af44186
1 parent a2c1b6e
commit af44186
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 8 deletions.
diff --git a/src/OpenTelemetry.Api/Internal/SpanHelper.cs b/src/OpenTelemetry.Api/Internal/SpanHelper.cs
@@ -14,6 +14,8 @@
 // limitations under the License.
 // </copyright>
 
+using System.Diagnostics;
+
 namespace OpenTelemetry.Trace
 {
     /// <summary>
@@ -25,11 +27,13 @@ internal static class SpanHelper
         /// Helper method that populates span properties from http status code according
         /// to https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#status.
         /// </summary>
+        /// <param name="kind">The span kind.</param>
         /// <param name="httpStatusCode">Http status code.</param>
         /// <returns>Resolved span <see cref="Status"/> for the Http status code.</returns>
-        public static Status ResolveSpanStatusForHttpStatusCode(int httpStatusCode)
+        public static Status ResolveSpanStatusForHttpStatusCode(ActivityKind kind, int httpStatusCode)
         {
-            if (httpStatusCode >= 100 && httpStatusCode <= 399)
+            var upperBound = kind == ActivityKind.Client ? 399 : 499;
+            if (httpStatusCode >= 100 && httpStatusCode <= upperBound)
             {
                 return Status.Unset;
             }

diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Changelog
 
+* Fix: Http server span status is now unset for `400`-`499`.
+  ([#2904](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2904))
+
 ## Unreleased
 
 ## 1.0.0-rc9

diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs
@@ -135,7 +135,7 @@ private void OnStopActivity(Activity activity, HttpContext context)
 
                 if (activity.GetStatus().StatusCode == StatusCode.Unset)
                 {
-                    activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(response.StatusCode));
+                    activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, response.StatusCode));
                 }
 
                 var routeData = context.Request.RequestContext.RouteData;

diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Changelog
 
+* Fix: Http server span status is now unset for `400`-`499`.
+  ([#2904](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2904))
+
 ## Unreleased
 
 ## 1.0.0-rc9

diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
@@ -196,12 +196,12 @@ public override void OnStopActivity(Activity activity, object payload)
                 }
                 else if (activity.GetStatus().StatusCode == StatusCode.Unset)
                 {
-                    activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(response.StatusCode));
+                    activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, response.StatusCode));
                 }
 #else
                 if (activity.GetStatus().StatusCode == StatusCode.Unset)
                 {
-                    activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(response.StatusCode));
+                    activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, response.StatusCode));
                 }
 #endif
 

diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
@@ -183,7 +183,7 @@ public override void OnStopActivity(Activity activity, object payload)
 
                     if (currentStatusCode == StatusCode.Unset)
                     {
-                        activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode));
+                        activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode));
                     }
 
                     try

diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
@@ -122,7 +122,7 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity)
             {
                 activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode);
 
-                activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode));
+                activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode));
 
                 try
                 {
@@ -150,7 +150,7 @@ private static void AddExceptionTags(Exception exception, Activity activity)
                 {
                     activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode);
 
-                    status = SpanHelper.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode);
+                    status = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode);
                 }
                 else
                 {