diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 4091d5b9f..b259c980e 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +* Removed the Activity Status Description that was being set during + exceptions. Activity Status will continue to be reported as `Error`. + This is a **breaking change**. `EnrichWithException` can be leveraged + to restore this behavior. + ([#5025](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5025)) + * Updated `http.request.method` to match specification guidelines. * For activity, if the method does not belong to one of the [known values](https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#:~:text=http.request.method%20has%20the%20following%20list%20of%20well%2Dknown%20values) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index fbf391794..7160ab666 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -435,7 +435,7 @@ internal class HttpInListener : ListenerHandler activity.RecordException(exc); } - activity.SetStatus(ActivityStatusCode.Error, exc.Message); + activity.SetStatus(ActivityStatusCode.Error); try { diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index bc19fe9c4..25bdfb76a 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +* Removed the Activity Status Description that was being set during + exceptions. Activity Status will continue to be reported as `Error`. + This is a **breaking change**. `EnrichWithException` can be leveraged + to restore this behavior. + ([#5025](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5025)) + * Updated `http.request.method` to match specification guidelines. * For activity, if the method does not belong to one of the [known values](https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#:~:text=http.request.method%20has%20the%20following%20list%20of%20well%2Dknown%20values) diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index c9234e4cd..f8fe389a9 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -353,7 +353,7 @@ internal sealed class HttpHandlerDiagnosticListener : ListenerHandler if (exc is HttpRequestException) { - activity.SetStatus(ActivityStatusCode.Error, exc.Message); + activity.SetStatus(ActivityStatusCode.Error); } try diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index d2c23f6c6..095ace34d 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -230,57 +230,30 @@ internal static class HttpWebRequestActivitySource } ActivityStatusCode status; - string exceptionMessage = null; - if (exception is WebException wexc) + if (exception is WebException wexc && wexc.Response is HttpWebResponse response) { - if (wexc.Response is HttpWebResponse response) + statusCode = response.StatusCode; + + if (tracingEmitOldAttributes) { - statusCode = response.StatusCode; - - if (tracingEmitOldAttributes) - { - activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)statusCode); - } - - if (tracingEmitNewAttributes) - { - activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, (int)statusCode); - } - - status = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)statusCode); + activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)statusCode); } - else + + if (tracingEmitNewAttributes) { - switch (wexc.Status) - { - case WebExceptionStatus.Timeout: - case WebExceptionStatus.RequestCanceled: - status = ActivityStatusCode.Error; - break; - case WebExceptionStatus.SendFailure: - case WebExceptionStatus.ConnectFailure: - case WebExceptionStatus.SecureChannelFailure: - case WebExceptionStatus.TrustFailure: - case WebExceptionStatus.ServerProtocolViolation: - case WebExceptionStatus.MessageLengthLimitExceeded: - status = ActivityStatusCode.Error; - exceptionMessage = exception.Message; - break; - default: - status = ActivityStatusCode.Error; - exceptionMessage = exception.Message; - break; - } + activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, (int)statusCode); } + + status = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)statusCode); } else { status = ActivityStatusCode.Error; - exceptionMessage = exception.Message; } - activity.SetStatus(status, exceptionMessage); + activity.SetStatus(status); + if (TracingOptions.RecordException) { activity.RecordException(exception); diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs index 586883017..1059bee92 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs @@ -131,14 +131,7 @@ public class IncomingRequestsCollectionsIsAccordingToTheSpecTests // Instrumentation is not expected to set status description // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - if (!urlPath.EndsWith("exception")) - { - Assert.True(string.IsNullOrEmpty(activity.StatusDescription)); - } - else - { - Assert.Equal("exception description", activity.StatusDescription); - } + Assert.Null(activity.StatusDescription); if (recordException) { diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe.cs index 622498876..8bfe675ed 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe.cs @@ -138,14 +138,7 @@ public class IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe // Instrumentation is not expected to set status description // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - if (!urlPath.EndsWith("exception")) - { - Assert.True(string.IsNullOrEmpty(activity.StatusDescription)); - } - else - { - Assert.Equal("exception description", activity.StatusDescription); - } + Assert.Null(activity.StatusDescription); if (recordException) { diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_New.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_New.cs index cf66df98e..5c56ffb67 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_New.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_New.cs @@ -132,14 +132,7 @@ public class IncomingRequestsCollectionsIsAccordingToTheSpecTests_New // Instrumentation is not expected to set status description // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - if (!urlPath.EndsWith("exception")) - { - Assert.True(string.IsNullOrEmpty(activity.StatusDescription)); - } - else - { - Assert.Equal("exception description", activity.StatusDescription); - } + Assert.Null(activity.StatusDescription); if (recordException) { diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs index 9ff51ebef..ad3344d8a 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs @@ -337,12 +337,7 @@ public partial class HttpClientTests // Assert.Equal(tc.SpanStatus, d[span.Status.CanonicalCode]); Assert.Equal(tc.SpanStatus, activity.Status.ToString()); - - if (tc.SpanStatusHasDescription.HasValue) - { - var desc = activity.StatusDescription; - Assert.Equal(tc.SpanStatusHasDescription.Value, !string.IsNullOrEmpty(desc)); - } + Assert.Null(activity.StatusDescription); var normalizedAttributes = activity.TagObjects.Where(kv => !kv.Key.StartsWith("otel.")).ToDictionary(x => x.Key, x => x.Value.ToString()); diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpTestData.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpTestData.cs index 86daf630d..705c7c9f8 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpTestData.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpTestData.cs @@ -72,8 +72,6 @@ public static class HttpTestData public string SpanStatus { get; set; } - public bool? SpanStatusHasDescription { get; set; } - public Dictionary SpanAttributes { get; set; } } } diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs index 6eca1d40e..074ae7df5 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs @@ -549,7 +549,7 @@ public class HttpWebRequestActivitySourceTests : IDisposable Assert.Equal("Stop", exceptionEvent.Key); Assert.True(activity.Status != ActivityStatusCode.Unset); - Assert.NotNull(activity.StatusDescription); + Assert.Null(activity.StatusDescription); } /// @@ -627,7 +627,7 @@ public class HttpWebRequestActivitySourceTests : IDisposable Assert.Equal("Stop", exceptionEvent.Key); Assert.True(exceptionEvent.Value.Status != ActivityStatusCode.Unset); - Assert.NotNull(exceptionEvent.Value.StatusDescription); + Assert.Null(exceptionEvent.Value.StatusDescription); } /// @@ -669,7 +669,7 @@ public class HttpWebRequestActivitySourceTests : IDisposable Assert.Equal("Stop", exceptionEvent.Key); Assert.True(exceptionEvent.Value.Status != ActivityStatusCode.Unset); - Assert.NotNull(exceptionEvent.Value.StatusDescription); + Assert.Null(exceptionEvent.Value.StatusDescription); } [Fact] diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs index 88e5b6e38..44d798608 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs @@ -123,11 +123,7 @@ public partial class HttpWebRequestTests if (tag.Key == SpanAttributeConstants.StatusDescriptionKey) { - if (tc.SpanStatusHasDescription.HasValue) - { - Assert.Equal(tc.SpanStatusHasDescription.Value, !string.IsNullOrEmpty(tagValue)); - } - + Assert.Null(tagValue); continue; } diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json b/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json index 8ebbe29ff..d808d5c00 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json @@ -93,7 +93,6 @@ "url": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/", "spanName": "HTTP GET", "spanStatus": "Error", - "spanStatusHasDescription": true, "responseExpected": false, "recordException": false, "spanAttributes": { @@ -111,7 +110,6 @@ "url": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/", "spanName": "HTTP GET", "spanStatus": "Error", - "spanStatusHasDescription": true, "responseExpected": false, "recordException": true, "spanAttributes": {