diff --git a/src/OpenTelemetry.Api/Internal/SpanHelper.cs b/src/OpenTelemetry.Api/Internal/SpanHelper.cs index a69e3579281..6f909db58c0 100644 --- a/src/OpenTelemetry.Api/Internal/SpanHelper.cs +++ b/src/OpenTelemetry.Api/Internal/SpanHelper.cs @@ -30,15 +30,15 @@ internal static class SpanHelper /// The span kind. /// Http status code. /// Resolved span for the Http status code. - public static Status ResolveSpanStatusForHttpStatusCode(ActivityKind kind, int httpStatusCode) + public static ActivityStatusCode ResolveSpanStatusForHttpStatusCode(ActivityKind kind, int httpStatusCode) { var upperBound = kind == ActivityKind.Client ? 399 : 499; if (httpStatusCode >= 100 && httpStatusCode <= upperBound) { - return Status.Unset; + return ActivityStatusCode.Unset; } - return Status.Error; + return ActivityStatusCode.Error; } } } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 7dd26e20f5d..40127aacd7d 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +* Updated to use Activity native support from `System.Diagnostics.DiagnosticSource` + to set activity status. + ([#3118](https://github.com/open-telemetry/opentelemetry-dotnet/issues/3118)) + ([#3555](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3555)) + ## 1.0.0-rc9.5 Released 2022-Aug-02 diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 6f848651386..25229662de3 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -207,12 +207,12 @@ public override void OnStopActivity(Activity activity, object payload) { AddGrpcAttributes(activity, grpcMethod, context); } - else if (activity.GetStatus().StatusCode == StatusCode.Unset) + else if (activity.Status == ActivityStatusCode.Unset) { activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, response.StatusCode)); } #else - if (activity.GetStatus().StatusCode == StatusCode.Unset) + if (activity.Status == ActivityStatusCode.Unset) { activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, response.StatusCode)); } @@ -298,7 +298,7 @@ public override void OnException(Activity activity, object payload) activity.RecordException(exc); } - activity.SetStatus(Status.Error.WithDescription(exc.Message)); + activity.SetStatus(ActivityStatusCode.Error, exc.Message); try { diff --git a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md index 51919bb3e00..90f445f087e 100644 --- a/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +* Updated to use Activity native support from `System.Diagnostics.DiagnosticSource` + to set activity status. + ([#3118](https://github.com/open-telemetry/opentelemetry-dotnet/issues/3118)) + ([#3555](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3555)) + * Changed activity source name from `OpenTelemetry.HttpWebRequest` to `OpenTelemetry.Instrumentation.Http.HttpWebRequest` for `HttpWebRequest`s and from `OpenTelemetry.Instrumentation.Http` diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs index a5814a44fcf..6e0f982b034 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs @@ -165,22 +165,22 @@ public override void OnStopActivity(Activity activity, object payload) // requestTaskStatus is not null _ = this.stopRequestStatusFetcher.TryFetch(payload, out var requestTaskStatus); - StatusCode currentStatusCode = activity.GetStatus().StatusCode; + ActivityStatusCode currentStatusCode = activity.Status; if (requestTaskStatus != TaskStatus.RanToCompletion) { if (requestTaskStatus == TaskStatus.Canceled) { - if (currentStatusCode == StatusCode.Unset) + if (currentStatusCode == ActivityStatusCode.Unset) { - activity.SetStatus(Status.Error); + activity.SetStatus(ActivityStatusCode.Error); } } else if (requestTaskStatus != TaskStatus.Faulted) { - if (currentStatusCode == StatusCode.Unset) + if (currentStatusCode == ActivityStatusCode.Unset) { // Faults are handled in OnException and should already have a span.Status of Error w/ Description. - activity.SetStatus(Status.Error); + activity.SetStatus(ActivityStatusCode.Error); } } } @@ -189,7 +189,7 @@ public override void OnStopActivity(Activity activity, object payload) { activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode); - if (currentStatusCode == StatusCode.Unset) + if (currentStatusCode == ActivityStatusCode.Unset) { activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)response.StatusCode)); } @@ -232,7 +232,7 @@ public override void OnException(Activity activity, object payload) if (exc is HttpRequestException) { - activity.SetStatus(Status.Error.WithDescription(exc.Message)); + activity.SetStatus(ActivityStatusCode.Error, exc.Message); } try diff --git a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs index 48a2b584c04..1eb7c9cd2dc 100644 --- a/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs @@ -141,7 +141,9 @@ private static void AddExceptionTags(Exception exception, Activity activity) return; } - Status status; + ActivityStatusCode status; + string exceptionMessage = null; + if (exception is WebException wexc) { if (wexc.Response is HttpWebResponse response) @@ -156,7 +158,7 @@ private static void AddExceptionTags(Exception exception, Activity activity) { case WebExceptionStatus.Timeout: case WebExceptionStatus.RequestCanceled: - status = Status.Error; + status = ActivityStatusCode.Error; break; case WebExceptionStatus.SendFailure: case WebExceptionStatus.ConnectFailure: @@ -164,20 +166,23 @@ private static void AddExceptionTags(Exception exception, Activity activity) case WebExceptionStatus.TrustFailure: case WebExceptionStatus.ServerProtocolViolation: case WebExceptionStatus.MessageLengthLimitExceeded: - status = Status.Error.WithDescription(exception.Message); + status = ActivityStatusCode.Error; + exceptionMessage = exception.Message; break; default: - status = Status.Error.WithDescription(exception.Message); + status = ActivityStatusCode.Error; + exceptionMessage = exception.Message; break; } } } else { - status = Status.Error.WithDescription(exception.Message); + status = ActivityStatusCode.Error; + exceptionMessage = exception.Message; } - activity.SetStatus(status); + activity.SetStatus(status, exceptionMessage); if (Options.RecordException) { activity.RecordException(exception); diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index 225b9bc49cd..9021ce9e912 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -86,10 +86,7 @@ void ConfigureTestServices(IServiceCollection services) var activity = exportedItems[0]; Assert.Equal(200, activity.GetTagValue(SemanticConventions.AttributeHttpStatusCode)); - - var status = activity.GetStatus(); - Assert.Equal(status, Status.Unset); - + Assert.Equal(ActivityStatusCode.Unset, activity.Status); ValidateAspNetCoreActivity(activity, "/api/values"); } diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/InProcServerTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/InProcServerTests.cs index 20b3ef3c602..9f39e9c2035 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/InProcServerTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/InProcServerTests.cs @@ -75,6 +75,8 @@ public async void ExampleTest() Assert.Equal("GET", activity.GetTagValue(SemanticConventions.AttributeHttpMethod)); Assert.Equal("1.1", activity.GetTagValue(SemanticConventions.AttributeHttpFlavor)); Assert.Equal(200, activity.GetTagValue(SemanticConventions.AttributeHttpStatusCode)); + Assert.True(activity.Status == ActivityStatusCode.Unset); + Assert.True(activity.StatusDescription is null); } public async void Dispose() diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs index fcc706cb218..fd946019d8d 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs @@ -115,22 +115,22 @@ public async Task SuccessfulTemplateControllerCallGeneratesASpan( if (statusCode == 503) { - Assert.Equal(Status.Error.StatusCode, activity.GetStatus().StatusCode); + Assert.Equal(ActivityStatusCode.Error, activity.Status); } else { - Assert.Equal(Status.Unset, activity.GetStatus()); + Assert.Equal(ActivityStatusCode.Unset, activity.Status); } // 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.GetStatus().Description)); + Assert.True(string.IsNullOrEmpty(activity.StatusDescription)); } else { - Assert.Equal("exception description", activity.GetStatus().Description); + Assert.Equal("exception description", activity.StatusDescription); } if (recordException) diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs index db24a44050b..7407e134527 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.cs @@ -110,13 +110,11 @@ public async Task HttpOutCallsAreCollectedSuccessfullyAsync(HttpTestData.HttpOut Assert.Equal(tc.SpanName, activity.DisplayName); // Assert.Equal(tc.SpanStatus, d[span.Status.CanonicalCode]); - Assert.Equal( - tc.SpanStatus, - activity.GetTagValue(SpanAttributeConstants.StatusCodeKey) as string); + Assert.Equal(tc.SpanStatus, activity.Status.ToString()); if (tc.SpanStatusHasDescription.HasValue) { - var desc = activity.GetTagValue(SpanAttributeConstants.StatusDescriptionKey) as string; + var desc = activity.StatusDescription; Assert.Equal(tc.SpanStatusHasDescription.Value, !string.IsNullOrEmpty(desc)); } @@ -194,7 +192,7 @@ public async Task DebugIndividualTestAsync() ""responseCode"": 399, ""responseExpected"": true, ""spanName"": ""HTTP GET"", - ""spanStatus"": ""UNSET"", + ""spanStatus"": ""Unset"", ""spanKind"": ""Client"", ""spanAttributes"": { ""http.scheme"": ""http"", diff --git a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs index 8f42f6b59c9..1d10b03d65d 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestActivitySourceTests.netfx.cs @@ -544,8 +544,8 @@ public async Task TestRequestWithException(string method) Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair exceptionEvent)); Assert.Equal("Stop", exceptionEvent.Key); - Assert.NotNull(activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - Assert.NotNull(activity.GetTagValue(SpanAttributeConstants.StatusDescriptionKey)); + Assert.True(activity.Status != ActivityStatusCode.Unset); + Assert.NotNull(activity.StatusDescription); } /// @@ -583,8 +583,8 @@ public async Task TestCanceledRequest(string method) Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair exceptionEvent)); Assert.Equal("Stop", exceptionEvent.Key); - Assert.NotNull(exceptionEvent.Value.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - Assert.Null(exceptionEvent.Value.GetTagValue(SpanAttributeConstants.StatusDescriptionKey)); + Assert.True(exceptionEvent.Value.Status != ActivityStatusCode.Unset); + Assert.True(exceptionEvent.Value.StatusDescription == null); } /// @@ -622,8 +622,8 @@ public async Task TestSecureTransportFailureRequest(string method) Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair exceptionEvent)); Assert.Equal("Stop", exceptionEvent.Key); - Assert.NotNull(exceptionEvent.Value.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - Assert.NotNull(exceptionEvent.Value.GetTagValue(SpanAttributeConstants.StatusDescriptionKey)); + Assert.True(exceptionEvent.Value.Status != ActivityStatusCode.Unset); + Assert.NotNull(exceptionEvent.Value.StatusDescription); } /// @@ -664,8 +664,8 @@ public async Task TestSecureTransportRetryFailureRequest(string method) Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair exceptionEvent)); Assert.Equal("Stop", exceptionEvent.Key); - Assert.NotNull(exceptionEvent.Value.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - Assert.NotNull(exceptionEvent.Value.GetTagValue(SpanAttributeConstants.StatusDescriptionKey)); + Assert.True(exceptionEvent.Value.Status != ActivityStatusCode.Unset); + Assert.NotNull(exceptionEvent.Value.StatusDescription); } [Fact] 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 3441e152d92..a0b9eb51984 100644 --- a/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json +++ b/test/OpenTelemetry.Instrumentation.Http.Tests/http-out-test-cases.json @@ -4,7 +4,7 @@ "method": "GET", "url": "http://{host}:{port}/", "spanName": "HTTP GET", - "spanStatus": "UNSET", + "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -20,7 +20,7 @@ "method": "POST", "url": "http://{host}:{port}/", "spanName": "HTTP POST", - "spanStatus": "UNSET", + "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -37,7 +37,7 @@ "url": "http://{host}:{port}/path/to/resource/", "responseCode": 200, "spanName": "HTTP GET", - "spanStatus": "UNSET", + "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -54,7 +54,7 @@ "url": "http://{host}:{port}/path/to/resource#fragment", "responseCode": 200, "spanName": "HTTP GET", - "spanStatus": "UNSET", + "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -71,7 +71,7 @@ "url": "http://username:password@{host}:{port}/path/to/resource#fragment", "responseCode": 200, "spanName": "HTTP GET", - "spanStatus": "UNSET", + "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -87,7 +87,7 @@ "method": "GET", "url": "https://sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com/", "spanName": "HTTP GET", - "spanStatus": "ERROR", + "spanStatus": "Error", "spanStatusHasDescription": true, "responseExpected": false, "recordException": false, @@ -104,7 +104,7 @@ "method": "GET", "url": "https://sdlfaldfjalkdfjlkajdflkajlsdjf.sdlkjafsdjfalfadslkf.com/", "spanName": "HTTP GET", - "spanStatus": "ERROR", + "spanStatus": "Error", "spanStatusHasDescription": true, "responseExpected": false, "recordException": true, @@ -122,7 +122,7 @@ "url": "http://{host}:{port}/", "responseCode": 200, "spanName": "HTTP GET", - "spanStatus": "UNSET", + "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -139,7 +139,7 @@ "url": "http://{host}:{port}/", "responseCode": 200, "spanName": "HTTP GET", - "spanStatus": "UNSET", + "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -156,7 +156,7 @@ "url": "http://{host}:{port}/", "responseCode": 399, "spanName": "HTTP GET", - "spanStatus": "UNSET", + "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -173,7 +173,7 @@ "url": "http://{host}:{port}/", "responseCode": 400, "spanName": "HTTP GET", - "spanStatus": "ERROR", + "spanStatus": "Error", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -190,7 +190,7 @@ "url": "http://{host}:{port}/", "responseCode": 401, "spanName": "HTTP GET", - "spanStatus": "ERROR", + "spanStatus": "Error", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -207,7 +207,7 @@ "url": "http://{host}:{port}/", "responseCode": 403, "spanName": "HTTP GET", - "spanStatus": "ERROR", + "spanStatus": "Error", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -224,7 +224,7 @@ "url": "http://{host}:{port}/", "responseCode": 404, "spanName": "HTTP GET", - "spanStatus": "ERROR", + "spanStatus": "Error", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -241,7 +241,7 @@ "url": "http://{host}:{port}/", "responseCode": 429, "spanName": "HTTP GET", - "spanStatus": "ERROR", + "spanStatus": "Error", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -258,7 +258,7 @@ "url": "http://{host}:{port}/", "responseCode": 501, "spanName": "HTTP GET", - "spanStatus": "ERROR", + "spanStatus": "Error", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -275,7 +275,7 @@ "url": "http://{host}:{port}/", "responseCode": 503, "spanName": "HTTP GET", - "spanStatus": "ERROR", + "spanStatus": "Error", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -292,7 +292,7 @@ "url": "http://{host}:{port}/", "responseCode": 504, "spanName": "HTTP GET", - "spanStatus": "ERROR", + "spanStatus": "Error", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -309,7 +309,7 @@ "url": "http://{host}:{port}/", "responseCode": 600, "spanName": "HTTP GET", - "spanStatus": "ERROR", + "spanStatus": "Error", "responseExpected": true, "spanAttributes": { "http.scheme": "http", @@ -326,7 +326,7 @@ "url": "http://{host}:{port}/", "responseCode": 200, "spanName": "HTTP GET", - "spanStatus": "UNSET", + "spanStatus": "Unset", "responseExpected": true, "spanAttributes": { "http.scheme": "http",