diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 4091d5b9f1f..b259c980e9d 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 fbf39179482..7160ab666c2 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -435,7 +435,7 @@ public void OnException(Activity activity, object payload) 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 bc19fe9c4fb..25bdfb76a51 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 c9234e4cdcc..f8fe389a943 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -353,7 +353,7 @@ public void OnException(Activity activity, object payload) 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 d2c23f6c6bf..095ace34d67 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -230,57 +230,30 @@ private static void AddExceptionTags(Exception exception, Activity activity, out } 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) - { - activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)statusCode); - } + statusCode = response.StatusCode; - if (tracingEmitNewAttributes) - { - activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, (int)statusCode); - } - - status = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)statusCode); + if (tracingEmitOldAttributes) + { + 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 58688301760..1059bee9281 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs @@ -131,14 +131,7 @@ public async Task SuccessfulTemplateControllerCallGeneratesASpan_Old( // 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 622498876cd..8bfe675ed5d 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe.cs @@ -138,14 +138,7 @@ public async Task SuccessfulTemplateControllerCallGeneratesASpan_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 cf66df98ee7..5c56ffb6719 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_New.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests_New.cs @@ -132,14 +132,7 @@ public async Task SuccessfulTemplateControllerCallGeneratesASpan_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 9ff51ebef41..ad3344d8a29 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs @@ -337,12 +337,7 @@ private static async Task HttpOutCallsAreCollectedSuccessfullyBodyAsync( // 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 86daf630db5..705c7c9f876 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpTestData.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpTestData.cs @@ -72,8 +72,6 @@ public class HttpOutTestCase 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 6eca1d40eed..074ae7df55c 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs @@ -549,7 +549,7 @@ public async Task TestRequestWithException(string method) Assert.Equal("Stop", exceptionEvent.Key); Assert.True(activity.Status != ActivityStatusCode.Unset); - Assert.NotNull(activity.StatusDescription); + Assert.Null(activity.StatusDescription); } /// @@ -627,7 +627,7 @@ public async Task TestSecureTransportFailureRequest(string method) 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 async Task TestSecureTransportRetryFailureRequest(string method) 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 88e5b6e3813..44d79860889 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.cs @@ -123,11 +123,7 @@ public void HttpOutCallsAreCollectedSuccessfully(HttpTestData.HttpOutTestCase tc 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 8ebbe29ff1e..d808d5c00f2 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": {