From 643c2f12abde813cbbbd47f0c4e583c20c4eb708 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 19 Sep 2022 17:15:41 -0700 Subject: [PATCH 1/6] Migrate to native Activity Status and Status Desc --- .../Api/SpanHelper.cs | 20 +++++++++++++++++++ .../Implementation/HttpInListener.cs | 6 +++--- .../HttpInListenerTests.cs | 18 ++++++++--------- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs b/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs index f7bb9880ff..6397b7aee1 100644 --- a/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs +++ b/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs @@ -61,4 +61,24 @@ public static Status ResolveSpanStatusForHttpStatusCode(ActivityKind kind, int h return Status.Error; } + +#if NET462 || NET6_0_OR_GREATER + /// + /// Helper method that populates Activity properties from http status code according + /// to https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#status. + /// + /// The span kind. + /// Http status code. + /// Resolved span for the Http status code. + public static ActivityStatusCode ResolveActivityStatusForHttpStatusCode(ActivityKind kind, int httpStatusCode) + { + var upperBound = kind == ActivityKind.Client ? 399 : 499; + if (httpStatusCode >= 100 && httpStatusCode <= upperBound) + { + return ActivityStatusCode.Unset; + } + + return ActivityStatusCode.Error; + } +#endif } diff --git a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs index 716b9c0136..a8c1ddb9a4 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs @@ -128,9 +128,9 @@ private void OnStopActivity(Activity activity, HttpContext context) activity.SetTag(SemanticConventions.AttributeHttpStatusCode, response.StatusCode); - if (activity.GetStatus().StatusCode == StatusCode.Unset) + if (activity.Status == ActivityStatusCode.Unset) { - activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, response.StatusCode)); + activity.SetStatus(SpanHelper.ResolveActivityStatusForHttpStatusCode(activity.Kind, response.StatusCode)); } var routeData = context.Request.RequestContext.RouteData; @@ -181,7 +181,7 @@ private void OnException(Activity activity, HttpContext context, Exception excep activity.RecordException(exception); } - activity.SetStatus(Status.Error.WithDescription(exception.Message)); + activity.SetStatus(ActivityStatusCode.Error, exception.Message); try { diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs index 0e3bd5eef2..70d477c627 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs @@ -137,7 +137,7 @@ public void AspNetRequestsAreCollectedSuccessfully( return httpContext.Request.Path != filter; }; - options.Enrich = GetEnrichmentAction(setStatusToErrorInEnrich ? Status.Error : default); + options.Enrich = GetEnrichmentAction(setStatusToErrorInEnrich ? ActivityStatusCode.Error : default); options.RecordException = recordException; }) @@ -217,27 +217,27 @@ public void AspNetRequestsAreCollectedSuccessfully( if (recordException) { - var status = span.GetStatus(); - Assert.Equal(Status.Error.StatusCode, status.StatusCode); - Assert.Equal("Operation is not valid due to the current state of the object.", status.Description); + var status = span.Status; + Assert.Equal(ActivityStatusCode.Error, span.Status); + Assert.Equal("Operation is not valid due to the current state of the object.", span.StatusDescription); } else if (setStatusToErrorInEnrich) { // This validates that users can override the // status in Enrich. - Assert.Equal(Status.Error, span.GetStatus()); + Assert.Equal(ActivityStatusCode.Error, span.Status); // Instrumentation is not expected to set status description // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - Assert.True(string.IsNullOrEmpty(span.GetStatus().Description)); + Assert.True(string.IsNullOrEmpty(span.StatusDescription)); } else { - Assert.Equal(Status.Unset, span.GetStatus()); + Assert.Equal(ActivityStatusCode.Unset, span.Status); // Instrumentation is not expected to set status description // as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode - Assert.True(string.IsNullOrEmpty(span.GetStatus().Description)); + Assert.True(string.IsNullOrEmpty(span.StatusDescription)); } } @@ -324,7 +324,7 @@ public void ExtractContextIrrespectiveOfTheFilterApplied() Assert.True(isPropagatorCalled); } - private static Action GetEnrichmentAction(Status statusToBeSet) + private static Action GetEnrichmentAction(ActivityStatusCode statusToBeSet) { void EnrichAction(Activity activity, string method, object obj) { From 3f5e8a1c39727a0166f8f965d162589fda82fcea Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 19 Sep 2022 17:18:52 -0700 Subject: [PATCH 2/6] changelog --- src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md index 71a4bfa708..764af3ba93 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +* Migrate to native Activity `Status` and + `StatusDesciption`.([#651](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/651)) + ## 1.0.0-rc9.5 (source code moved to contrib repo) Released 2022-Jun-21 From c1243607e57a426bba6cd64c86a0e131826c8c14 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 19 Sep 2022 17:20:57 -0700 Subject: [PATCH 3/6] update comment --- src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs b/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs index 6397b7aee1..cfb749aaaa 100644 --- a/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs +++ b/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs @@ -64,7 +64,7 @@ public static Status ResolveSpanStatusForHttpStatusCode(ActivityKind kind, int h #if NET462 || NET6_0_OR_GREATER /// - /// Helper method that populates Activity properties from http status code according + /// Helper method that populates Activity Status from http status code according /// to https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#status. /// /// The span kind. From b31ec066250b1a5b33fba469c14a673ec6b76071 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Mon, 19 Sep 2022 19:49:41 -0700 Subject: [PATCH 4/6] fix changelog formatting --- src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md index 764af3ba93..76782773cf 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md @@ -2,8 +2,8 @@ ## Unreleased -* Migrate to native Activity `Status` and - `StatusDesciption`.([#651](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/651)) +* Migrate to native Activity `Status` and `StatusDesciption`. + ([#651](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/651)) ## 1.0.0-rc9.5 (source code moved to contrib repo) From e2aced0af51e0910bf27bf3ed6c9a45a74ce9109 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Thu, 22 Sep 2022 13:09:28 -0700 Subject: [PATCH 5/6] Add custom spanHelper for aspnet --- ...penTelemetry.Instrumentation.AspNet.csproj | 1 - .../SpanHelper.cs | 43 +++++++++++++++++++ ...emetry.Instrumentation.AspNet.Tests.csproj | 1 - 3 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 src/OpenTelemetry.Instrumentation.AspNet/SpanHelper.cs diff --git a/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj b/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj index 03f775a3d7..060706d0c6 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj +++ b/src/OpenTelemetry.Instrumentation.AspNet/OpenTelemetry.Instrumentation.AspNet.csproj @@ -14,7 +14,6 @@ - diff --git a/src/OpenTelemetry.Instrumentation.AspNet/SpanHelper.cs b/src/OpenTelemetry.Instrumentation.AspNet/SpanHelper.cs new file mode 100644 index 0000000000..14e6bc4e2c --- /dev/null +++ b/src/OpenTelemetry.Instrumentation.AspNet/SpanHelper.cs @@ -0,0 +1,43 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Diagnostics; + +namespace OpenTelemetry.Instrumentation.AspNet; + +/// +/// A collection of helper methods to be used when building spans. +/// +internal static class SpanHelper +{ + /// + /// Helper method that populates Activity Status from http status code according + /// to https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#status. + /// + /// The span kind. + /// Http status code. + /// Resolved span for the Http status code. + public static ActivityStatusCode ResolveActivityStatusForHttpStatusCode(ActivityKind kind, int httpStatusCode) + { + var upperBound = kind == ActivityKind.Client ? 399 : 499; + if (httpStatusCode >= 100 && httpStatusCode <= upperBound) + { + return ActivityStatusCode.Unset; + } + + return ActivityStatusCode.Error; + } +} diff --git a/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj b/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj index e3499a281e..50803ac337 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.AspNet.Tests/OpenTelemetry.Instrumentation.AspNet.Tests.csproj @@ -27,7 +27,6 @@ - From ce3da5237ec41354dc26ad24784b8a05e8e37313 Mon Sep 17 00:00:00 2001 From: Vishwesh Bankwar Date: Thu, 22 Sep 2022 13:23:34 -0700 Subject: [PATCH 6/6] rmv custom method --- .../Api/SpanHelper.cs | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs b/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs index cfb749aaaa..f7bb9880ff 100644 --- a/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs +++ b/src/OpenTelemetry.Contrib.Shared/Api/SpanHelper.cs @@ -61,24 +61,4 @@ public static Status ResolveSpanStatusForHttpStatusCode(ActivityKind kind, int h return Status.Error; } - -#if NET462 || NET6_0_OR_GREATER - /// - /// Helper method that populates Activity Status from http status code according - /// to https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#status. - /// - /// The span kind. - /// Http status code. - /// Resolved span for the Http status code. - public static ActivityStatusCode ResolveActivityStatusForHttpStatusCode(ActivityKind kind, int httpStatusCode) - { - var upperBound = kind == ActivityKind.Client ? 399 : 499; - if (httpStatusCode >= 100 && httpStatusCode <= upperBound) - { - return ActivityStatusCode.Unset; - } - - return ActivityStatusCode.Error; - } -#endif }