From 01933208422ba36b72e0db98038c0ac186cecc3c Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Fri, 9 Aug 2024 10:40:15 +0200 Subject: [PATCH 01/16] Mark ActivityExtensions.SetStatus obsolete --- src/OpenTelemetry.Api/CHANGELOG.md | 6 ++++++ src/OpenTelemetry.Api/Trace/ActivityExtensions.cs | 5 +++++ src/OpenTelemetry.Api/Trace/TelemetrySpan.cs | 3 +++ src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs | 2 ++ .../OtlpTraceExporterTests.cs | 4 ++++ .../ZipkinExporterTests.cs | 2 ++ 6 files changed, 22 insertions(+) diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 11f7ded10e4..6ceecb520e2 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -6,6 +6,12 @@ Notes](../../RELEASENOTES.md). ## Unreleased +* Marked `ActivityExtensions.SetStatus` method as `[Obsolete]`. + Users are now encouraged to use the `Activity.SetStatus(ActivityStatusCode)` + method from `System.Diagnostics.DiagnosticSource` + for setting the status of an `Activity`. + ([]()) + * **Breaking change:** CompositeTextMapPropagator.Fields now returns a unioned set of fields from all combined propagators. Previously this always returned an empty set. diff --git a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs index d8ac769b9c9..49093511971 100644 --- a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs +++ b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs @@ -25,9 +25,14 @@ public static class ActivityExtensions /// This extension provides a workaround to store Status as special tags with key name of otel.status_code and otel.status_description. /// Read more about SetStatus here https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status. /// + /// + /// This method is obsolete. + /// For more details see: . + /// /// Activity instance. /// Activity execution status. [MethodImpl(MethodImplOptions.AggressiveInlining)] + [Obsolete("This method is obsolete. Use Activity.SetStatus(ActivityStatusCode) instead.")] public static void SetStatus(this Activity activity, Status status) { if (activity != null) diff --git a/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs b/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs index f70598d078b..9e484bd99dc 100644 --- a/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs +++ b/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs @@ -47,7 +47,10 @@ public ActivitySpanId ParentSpanId /// Status to be set. public void SetStatus(Status value) { +#pragma warning disable + // TODO: Remove this when SetStatus is deprecated this.Activity?.SetStatus(value); +#pragma warning disable } /// diff --git a/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs b/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs index 3473c4292ad..d584bf23082 100644 --- a/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs +++ b/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs @@ -62,8 +62,10 @@ public override void OnEnd(Activity activity) if (snapshot != pointers) { +#pragma warning disable // TODO: Remove this when SetStatus is deprecated activity.SetStatus(Status.Error); +#pragma warning disable // For processors/exporters checking `Status` property. activity.SetStatus(ActivityStatusCode.Error); diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs index 84d3076500a..f6d681f63a0 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs @@ -370,7 +370,11 @@ public void ToOtlpSpanTest() links: childLinks); Assert.NotNull(childActivity); +#pragma warning disable + // TODO: Remove this when SetStatus is deprecated childActivity.SetStatus(Status.Error); +#pragma warning disable + childActivity.SetStatus(ActivityStatusCode.Error); var childEvents = new List { new("e0"), new("e1", default, new ActivityTagsCollection(attributes)) }; childActivity.AddEvent(childEvents[0]); diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index 01e40b66b16..95c5181f939 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -554,7 +554,9 @@ internal static Activity CreateTestActivity( if (status.HasValue) { +#pragma warning disable activity.SetStatus(status.Value); +#pragma warning enable } activity.SetEndTime(endTimestamp); From 3dde1fccb0998ce8e5631653c49505c82975e717 Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Fri, 9 Aug 2024 10:49:36 +0200 Subject: [PATCH 02/16] use ActivityStatusCode --- src/OpenTelemetry.Api/Trace/TelemetrySpan.cs | 1 + test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs b/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs index 9e484bd99dc..1bd0b4fcd89 100644 --- a/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs +++ b/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs @@ -51,6 +51,7 @@ public void SetStatus(Status value) // TODO: Remove this when SetStatus is deprecated this.Activity?.SetStatus(value); #pragma warning disable + this.Activity?.SetStatus((ActivityStatusCode)value.StatusCode); } /// diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index 95c5181f939..a43ff7a484a 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -557,6 +557,7 @@ internal static Activity CreateTestActivity( #pragma warning disable activity.SetStatus(status.Value); #pragma warning enable + activity.SetStatus((ActivityStatusCode)status.Value.StatusCode); } activity.SetEndTime(endTimestamp); From 61d50f07a1f5e56f9ae02c4370c7efe8bec40b8d Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Fri, 9 Aug 2024 10:53:44 +0200 Subject: [PATCH 03/16] Update Changelog.md --- src/OpenTelemetry.Api/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 6ceecb520e2..e835a9f0264 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -10,7 +10,7 @@ Notes](../../RELEASENOTES.md). Users are now encouraged to use the `Activity.SetStatus(ActivityStatusCode)` method from `System.Diagnostics.DiagnosticSource` for setting the status of an `Activity`. - ([]()) + ([#5781](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5781)) * **Breaking change:** CompositeTextMapPropagator.Fields now returns a unioned set of fields from all combined propagators. Previously this always From a1390b415390cfc4c57e42b4e5fe0542bd3c6436 Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Fri, 9 Aug 2024 11:44:50 +0200 Subject: [PATCH 04/16] Update ActivityExtensions.cs by suggestions --- src/OpenTelemetry.Api/Trace/ActivityExtensions.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs index 49093511971..99708944dfb 100644 --- a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs +++ b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs @@ -21,9 +21,6 @@ public static class ActivityExtensions { /// /// Sets the status of activity execution. - /// Activity class in .NET does not support 'Status'. - /// This extension provides a workaround to store Status as special tags with key name of otel.status_code and otel.status_description. - /// Read more about SetStatus here https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status. /// /// /// This method is obsolete. @@ -32,7 +29,7 @@ public static class ActivityExtensions /// Activity instance. /// Activity execution status. [MethodImpl(MethodImplOptions.AggressiveInlining)] - [Obsolete("This method is obsolete. Use Activity.SetStatus(ActivityStatusCode) instead.")] + [Obsolete("Use Activity.SetStatus(ActivityStatusCode) instead.")] public static void SetStatus(this Activity activity, Status status) { if (activity != null) From 934895f57c2dcef8ae19ff23655c8d677fbdc1d6 Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Mon, 12 Aug 2024 09:10:44 +0200 Subject: [PATCH 05/16] Update remarks --- src/OpenTelemetry.Api/Trace/ActivityExtensions.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs index 99708944dfb..5f7399212da 100644 --- a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs +++ b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs @@ -24,6 +24,8 @@ public static class ActivityExtensions /// /// /// This method is obsolete. + /// Use the `Activity.SetStatus(ActivityStatusCode)` method + /// from `System.Diagnostics.DiagnosticSource` for setting the status of an `Activity`. /// For more details see: . /// /// Activity instance. From 395d46df750fb7604a7aebebbc741a3c5a053f76 Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Wed, 14 Aug 2024 09:28:55 +0200 Subject: [PATCH 06/16] wp --- .../OtlpTraceExporterTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs index f6d681f63a0..9feef4a567e 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs @@ -394,7 +394,7 @@ public void ToOtlpSpanTest() // Assert.Equal(OtlpTrace.Status.Types.StatusCode.NotFound, otlpSpan.Status.Code); - Assert.Equal(Status.Error.Description ?? string.Empty, otlpSpan.Status.Message); + //Assert.Equal(Status.Error.Description ?? string.Empty, otlpSpan.Status.Message); Assert.Empty(otlpSpan.Attributes); Assert.Equal(childEvents.Count, otlpSpan.Events.Count); From 6c8002a4741c0d9c8b7b04db77d852eb037910e1 Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Wed, 14 Aug 2024 12:51:35 +0200 Subject: [PATCH 07/16] add status code assertion --- .../OtlpTraceExporterTests.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs index 9feef4a567e..8681b1de860 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs @@ -370,10 +370,7 @@ public void ToOtlpSpanTest() links: childLinks); Assert.NotNull(childActivity); -#pragma warning disable - // TODO: Remove this when SetStatus is deprecated - childActivity.SetStatus(Status.Error); -#pragma warning disable + childActivity.SetStatus(ActivityStatusCode.Error); var childEvents = new List { new("e0"), new("e1", default, new ActivityTagsCollection(attributes)) }; @@ -392,9 +389,10 @@ public void ToOtlpSpanTest() Assert.Equal(traceId, otlpSpan.TraceId); Assert.Equal(parentId, otlpSpan.ParentSpanId); - // Assert.Equal(OtlpTrace.Status.Types.StatusCode.NotFound, otlpSpan.Status.Code); + Assert.NotNull(otlpSpan.Status); + Assert.Equal(OtlpTrace.Status.Types.StatusCode.Error, otlpSpan.Status.Code); - //Assert.Equal(Status.Error.Description ?? string.Empty, otlpSpan.Status.Message); + Assert.Equal(Status.Error.Description ?? string.Empty, otlpSpan.Status.Message); Assert.Empty(otlpSpan.Attributes); Assert.Equal(childEvents.Count, otlpSpan.Events.Count); From 5dca544c7cae72e5d037ba956a9c1e7023cedf7e Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Fri, 16 Aug 2024 09:15:10 +0200 Subject: [PATCH 08/16] apply suggestion in zipkintest --- src/OpenTelemetry.Api/Trace/TelemetrySpan.cs | 2 +- src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs | 2 +- .../ZipkinExporterTests.cs | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs b/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs index 1bd0b4fcd89..ab8a9cdf320 100644 --- a/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs +++ b/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs @@ -50,7 +50,7 @@ public void SetStatus(Status value) #pragma warning disable // TODO: Remove this when SetStatus is deprecated this.Activity?.SetStatus(value); -#pragma warning disable +#pragma warning restore this.Activity?.SetStatus((ActivityStatusCode)value.StatusCode); } diff --git a/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs b/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs index d584bf23082..a4b8df44f41 100644 --- a/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs +++ b/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs @@ -65,7 +65,7 @@ public override void OnEnd(Activity activity) #pragma warning disable // TODO: Remove this when SetStatus is deprecated activity.SetStatus(Status.Error); -#pragma warning disable +#pragma warning restore // For processors/exporters checking `Status` property. activity.SetStatus(ActivityStatusCode.Error); diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index a43ff7a484a..ce84134eaa7 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -556,8 +556,9 @@ internal static Activity CreateTestActivity( { #pragma warning disable activity.SetStatus(status.Value); -#pragma warning enable - activity.SetStatus((ActivityStatusCode)status.Value.StatusCode); +#pragma warning restore + +// activity.SetStatus((ActivityStatusCode)status.Value.StatusCode); } activity.SetEndTime(endTimestamp); From a04f5d6ea6d7e87ebec8d69980c093a7e189b6d6 Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Fri, 16 Aug 2024 09:17:11 +0200 Subject: [PATCH 09/16] update Changelog --- src/OpenTelemetry.Api/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 78d01710798..b0d6a2e1332 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -7,7 +7,8 @@ Notes](../../RELEASENOTES.md). ## Unreleased * Marked `ActivityExtensions.SetStatus` method as `[Obsolete]`. - Users are now encouraged to use the `Activity.SetStatus(ActivityStatusCode)` + Users are now encouraged to use the [Activity.SetStatus(ActivityStatusCode] + (https://learn.microsoft.com/dotnet/api/system.diagnostics.activity.setstatus) method from `System.Diagnostics.DiagnosticSource` for setting the status of an `Activity`. ([#5781](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5781)) From 36bb8e273718e97229c17a44cb18202ac308a625 Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Fri, 16 Aug 2024 09:23:02 +0200 Subject: [PATCH 10/16] Fix changelog --- src/OpenTelemetry.Api/CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index b0d6a2e1332..17850c08254 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -7,8 +7,7 @@ Notes](../../RELEASENOTES.md). ## Unreleased * Marked `ActivityExtensions.SetStatus` method as `[Obsolete]`. - Users are now encouraged to use the [Activity.SetStatus(ActivityStatusCode] - (https://learn.microsoft.com/dotnet/api/system.diagnostics.activity.setstatus) + Users are now encouraged to use the [Activity.SetStatus(ActivityStatusCode](https://learn.microsoft.com/dotnet/api/system.diagnostics.activity.setstatus) method from `System.Diagnostics.DiagnosticSource` for setting the status of an `Activity`. ([#5781](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5781)) From 8e455e32a6141b7b63eb18d46e850708401796fd Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Fri, 16 Aug 2024 09:38:42 +0200 Subject: [PATCH 11/16] remove commented code --- test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index ce84134eaa7..2ea73d2cdab 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -557,8 +557,6 @@ internal static Activity CreateTestActivity( #pragma warning disable activity.SetStatus(status.Value); #pragma warning restore - -// activity.SetStatus((ActivityStatusCode)status.Value.StatusCode); } activity.SetEndTime(endTimestamp); From f6a4ebff8534fcf1aa49c5f0c55cd4f250f94861 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 21 Aug 2024 10:31:05 -0700 Subject: [PATCH 12/16] Code review. --- src/OpenTelemetry.Api/CHANGELOG.md | 17 +++++--- .../Trace/ActivityExtensions.cs | 20 +++++---- src/OpenTelemetry.Api/Trace/TelemetrySpan.cs | 1 - .../Trace/Processor/ExceptionProcessor.cs | 1 - src/Shared/ActivityHelperExtensions.cs | 1 + .../ZipkinActivityConversionTest.cs | 4 ++ .../ZipkinExporterTests.cs | 41 ++++++------------- .../SpanBuilderShimTests.cs | 14 +++++-- .../SpanShimTests.cs | 36 ++++++---------- 9 files changed, 64 insertions(+), 71 deletions(-) diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 17850c08254..b3b2738a111 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -6,12 +6,6 @@ Notes](../../RELEASENOTES.md). ## Unreleased -* Marked `ActivityExtensions.SetStatus` method as `[Obsolete]`. - Users are now encouraged to use the [Activity.SetStatus(ActivityStatusCode](https://learn.microsoft.com/dotnet/api/system.diagnostics.activity.setstatus) - method from `System.Diagnostics.DiagnosticSource` - for setting the status of an `Activity`. - ([#5781](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5781)) - * **Breaking change:** CompositeTextMapPropagator.Fields now returns a unioned set of fields from all combined propagators. Previously this always returned an empty set. @@ -20,6 +14,17 @@ Notes](../../RELEASENOTES.md). * Optimize performance of `TraceContextPropagator.Extract`. ([#5749](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5749)) +* Obsoleted the `ActivityExtensions.GetStatus` and + `ActivityExtensions.SetStatus` extension methods. Users should migrate to the + `System.Diagnostics.DiagnosticSource` + [Activity.SetStatus](https://learn.microsoft.com/dotnet/api/system.diagnostics.activity.setstatus) + API for setting the status and + [Activity.Status](https://learn.microsoft.com/dotnet/api/system.diagnostics.activity.status) + & + [Activity.StatusDescription](https://learn.microsoft.com/dotnet/api/system.diagnostics.activity.statusdescription) + APIs for reading the status of an `Activity` instance. + ([#5781](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5781)) + ## 1.9.0 Released 2024-Jun-14 diff --git a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs index 5f7399212da..a90743b06b7 100644 --- a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs +++ b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs @@ -23,15 +23,15 @@ public static class ActivityExtensions /// Sets the status of activity execution. /// /// - /// This method is obsolete. - /// Use the `Activity.SetStatus(ActivityStatusCode)` method - /// from `System.Diagnostics.DiagnosticSource` for setting the status of an `Activity`. - /// For more details see: . + /// Note: This method is obsolete. Call the + /// method instead. For more details see: . /// /// Activity instance. /// Activity execution status. [MethodImpl(MethodImplOptions.AggressiveInlining)] - [Obsolete("Use Activity.SetStatus(ActivityStatusCode) instead.")] + [Obsolete("Call Activity.SetStatus instead this method will be removed in a future version.")] public static void SetStatus(this Activity activity, Status status) { if (activity != null) @@ -43,12 +43,18 @@ public static void SetStatus(this Activity activity, Status status) /// /// Gets the status of activity execution. - /// Activity class in .NET does not support 'Status'. - /// This extension provides a workaround to retrieve Status from special tags with key name otel.status_code and otel.status_description. /// + /// + /// Note: This method is obsolete. Use the and + /// properties instead. For more + /// details see: . + /// /// Activity instance. /// Activity execution status. [MethodImpl(MethodImplOptions.AggressiveInlining)] + [Obsolete("Use Activity.Status and Activity.StatusDescription instead this method will be removed in a future version.")] public static Status GetStatus(this Activity activity) { if (activity == null diff --git a/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs b/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs index ab8a9cdf320..b809f439e48 100644 --- a/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs +++ b/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs @@ -48,7 +48,6 @@ public ActivitySpanId ParentSpanId public void SetStatus(Status value) { #pragma warning disable - // TODO: Remove this when SetStatus is deprecated this.Activity?.SetStatus(value); #pragma warning restore this.Activity?.SetStatus((ActivityStatusCode)value.StatusCode); diff --git a/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs b/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs index a4b8df44f41..45f21bbf1cc 100644 --- a/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs +++ b/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs @@ -63,7 +63,6 @@ public override void OnEnd(Activity activity) if (snapshot != pointers) { #pragma warning disable - // TODO: Remove this when SetStatus is deprecated activity.SetStatus(Status.Error); #pragma warning restore diff --git a/src/Shared/ActivityHelperExtensions.cs b/src/Shared/ActivityHelperExtensions.cs index b237ccd5457..0540ce7c5ca 100644 --- a/src/Shared/ActivityHelperExtensions.cs +++ b/src/Shared/ActivityHelperExtensions.cs @@ -24,6 +24,7 @@ internal static class ActivityHelperExtensions /// Status description. /// if was found on the supplied Activity. [MethodImpl(MethodImplOptions.AggressiveInlining)] + [Obsolete] public static bool TryGetStatus(this Activity activity, out StatusCode statusCode, out string? statusDescription) { Debug.Assert(activity != null, "Activity should not be null"); diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs index da6cdcbfcbb..bfe69520fea 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs @@ -79,6 +79,7 @@ public void ToZipkinSpan_NoEvents() [InlineData(StatusCode.Ok, "Ok")] [InlineData(StatusCode.Error, "ERROR")] [InlineData(StatusCode.Unset, "iNvAlId")] + [Obsolete("Remove when ActivityExtensions status APIs are removed")] public void ToZipkinSpan_Status_ErrorFlagTest(StatusCode expectedStatusCode, string statusCodeTagValue) { // Arrange @@ -159,6 +160,7 @@ public void ToZipkinSpan_Activity_Status_And_StatusDescription_is_Set(ActivitySt } [Fact] + [Obsolete("Remove when ActivityExtensions status APIs are removed")] public void ActivityStatus_Takes_precedence_Over_Status_Tags_ActivityStatusCodeIsOk() { // Arrange. @@ -184,6 +186,7 @@ public void ActivityStatus_Takes_precedence_Over_Status_Tags_ActivityStatusCodeI } [Fact] + [Obsolete("Remove when ActivityExtensions status APIs are removed")] public void ActivityStatus_Takes_precedence_Over_Status_Tags_ActivityStatusCodeIsError() { // Arrange. @@ -219,6 +222,7 @@ public void ActivityStatus_Takes_precedence_Over_Status_Tags_ActivityStatusCodeI } [Fact] + [Obsolete("Remove when ActivityExtensions status APIs are removed")] public void ActivityStatus_Takes_precedence_Over_Status_Tags_ActivityStatusCodeIsError_SettingTagFirst() { // Arrange. diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index 2ea73d2cdab..b1481ac0541 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -322,31 +322,18 @@ public void UpdatesServiceNameFromIConfiguration() [InlineData(false, false, false)] [InlineData(false, true, false)] [InlineData(false, false, true)] - [InlineData(false, false, false, StatusCode.Ok)] - [InlineData(false, false, false, StatusCode.Ok, null, true)] - [InlineData(false, false, false, StatusCode.Error)] - [InlineData(false, false, false, StatusCode.Error, "Error description")] + [InlineData(false, false, false, ActivityStatusCode.Ok)] + [InlineData(false, false, false, ActivityStatusCode.Ok, null, true)] + [InlineData(false, false, false, ActivityStatusCode.Error)] + [InlineData(false, false, false, ActivityStatusCode.Error, "Error description")] public void IntegrationTest( bool useShortTraceIds, bool useTestResource, bool isRootSpan, - StatusCode statusCode = StatusCode.Unset, + ActivityStatusCode statusCode = ActivityStatusCode.Unset, string statusDescription = null, bool addErrorTag = false) { - var status = statusCode switch - { - StatusCode.Unset => Status.Unset, - StatusCode.Ok => Status.Ok, - StatusCode.Error => Status.Error, - _ => throw new InvalidOperationException(), - }; - - if (!string.IsNullOrEmpty(statusDescription)) - { - status = status.WithDescription(statusDescription); - } - Guid requestId = Guid.NewGuid(); ZipkinExporter exporter = new ZipkinExporter( @@ -360,7 +347,7 @@ public void IntegrationTest( .Where(pair => pair.Key == ResourceSemanticConventions.AttributeServiceName).FirstOrDefault().Value; var resourceTags = string.Empty; var dateTime = DateTime.UtcNow; - var activity = CreateTestActivity(isRootSpan: isRootSpan, status: status, dateTime: dateTime); + var activity = CreateTestActivity(isRootSpan: isRootSpan, statusCode: statusCode, statusDescription: statusDescription, dateTime: dateTime); if (useTestResource) { serviceName = "MyService"; @@ -409,13 +396,13 @@ public void IntegrationTest( string errorTag = string.Empty; switch (statusCode) { - case StatusCode.Ok: + case ActivityStatusCode.Ok: statusTag = $@"""{SpanAttributeConstants.StatusCodeKey}"":""OK"","; break; - case StatusCode.Unset: + case ActivityStatusCode.Unset: statusTag = string.Empty; break; - case StatusCode.Error: + case ActivityStatusCode.Error: statusTag = $@"""{SpanAttributeConstants.StatusCodeKey}"":""ERROR"","; errorTag = $@"""{ZipkinActivityConversionExtensions.ZipkinErrorFlagTagName}"":""{statusDescription}"","; break; @@ -464,7 +451,8 @@ internal static Activity CreateTestActivity( bool addLinks = true, Resource resource = null, ActivityKind kind = ActivityKind.Client, - Status? status = null, + ActivityStatusCode statusCode = ActivityStatusCode.Unset, + string statusDescription = null, DateTime? dateTime = null) { var startTimestamp = DateTime.UtcNow; @@ -552,12 +540,7 @@ internal static Activity CreateTestActivity( } } - if (status.HasValue) - { -#pragma warning disable - activity.SetStatus(status.Value); -#pragma warning restore - } + activity.SetStatus(statusCode, statusDescription); activity.SetEndTime(endTimestamp); activity.Stop(); diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs index ce546c868bf..9843e7f6132 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs @@ -216,8 +216,11 @@ public void WithTag_KeyIsErrorStringValue() // build var spanShim = (SpanShim)shim.Start(); - // Span status should be set - Assert.Equal(Status.Error, spanShim.Span.Activity.GetStatus()); + // Legacy span status tag should be set + Assert.Equal("ERROR", spanShim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); + + // Activity status code should also be set + Assert.Equal(ActivityStatusCode.Error, spanShim.Span.Activity.Status); } [Fact] @@ -262,8 +265,11 @@ public void WithTag_KeyIsErrorBoolValue() // build var spanShim = (SpanShim)shim.Start(); - // Span status should be set - Assert.Equal(Status.Error, spanShim.Span.Activity.GetStatus()); + // Legacy span status tag should be set + Assert.Equal("ERROR", spanShim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); + + // Activity status code should also be set + Assert.Equal(ActivityStatusCode.Error, spanShim.Span.Activity.Status); } [Fact] diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs index 80178f405be..3664ed97731 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs @@ -1,6 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +using System.Diagnostics; using OpenTelemetry.Trace; using OpenTracing.Tag; using Xunit; @@ -209,10 +210,20 @@ public void SetTagBoolValue() Assert.True((bool)shim.Span.Activity.TagObjects.First().Value); // A boolean tag named "error" is a special case that must be checked - Assert.Equal(Status.Error, shim.Span.Activity.GetStatus()); + + // Legacy span status tag should be set + Assert.Equal("ERROR", shim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); + + // Activity status code should also be set + Assert.Equal(ActivityStatusCode.Error, shim.Span.Activity.Status); shim.SetTag(Tags.Error.Key, false); - Assert.Equal(Status.Ok, shim.Span.Activity.GetStatus()); + + // Legacy span status tag should be set + Assert.Equal("OK", shim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); + + // Activity status code should also be set + Assert.Equal(ActivityStatusCode.Ok, shim.Span.Activity.Status); } [Fact] @@ -245,27 +256,6 @@ public void SetTagDoubleValue() Assert.Equal(1, (double)shim.Span.Activity.TagObjects.First().Value); } - [Fact] - public void SetTagBooleanTagValue() - { - var tracer = TracerProvider.Default.GetTracer(TracerName); - var shim = new SpanShim(tracer.StartSpan(SpanName)); - - Assert.Throws(() => shim.SetTag((BooleanTag)null, true)); - - shim.SetTag(new BooleanTag("foo"), true); - shim.SetTag(new BooleanTag(Tags.Error.Key), true); - - Assert.Equal("foo", shim.Span.Activity.TagObjects.First().Key); - Assert.True((bool)shim.Span.Activity.TagObjects.First().Value); - - // A boolean tag named "error" is a special case that must be checked - Assert.Equal(Status.Error, shim.Span.Activity.GetStatus()); - - shim.SetTag(Tags.Error.Key, false); - Assert.Equal(Status.Ok, shim.Span.Activity.GetStatus()); - } - [Fact] public void SetTagStringTagValue() { From af15a9378785a367a8a9e63e58e52484352db382 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 21 Aug 2024 10:35:11 -0700 Subject: [PATCH 13/16] Tweaks. --- .../OtlpTraceExporterTests.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs index 8681b1de860..9f5da4cd8d7 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpTraceExporterTests.cs @@ -476,6 +476,7 @@ public void ToOtlpSpanNativeActivityStatusTest(ActivityStatusCode expectedStatus [InlineData(StatusCode.Unset, "Unset", "Description will be ignored if status is Unset.")] [InlineData(StatusCode.Ok, "Ok", "Description must only be used with the Error StatusCode.")] [InlineData(StatusCode.Error, "Error", "Error description.")] + [Obsolete("Remove when ActivityExtensions status APIs are removed")] public void ToOtlpSpanStatusTagTest(StatusCode expectedStatusCode, string statusCodeTagValue, string statusDescription) { using var activitySource = new ActivitySource(nameof(this.ToOtlpSpanTest)); @@ -504,6 +505,7 @@ public void ToOtlpSpanStatusTagTest(StatusCode expectedStatusCode, string status [InlineData(StatusCode.Unset, "uNsET")] [InlineData(StatusCode.Ok, "oK")] [InlineData(StatusCode.Error, "ERROR")] + [Obsolete("Remove when ActivityExtensions status APIs are removed")] public void ToOtlpSpanStatusTagIsCaseInsensitiveTest(StatusCode expectedStatusCode, string statusCodeTagValue) { using var activitySource = new ActivitySource(nameof(this.ToOtlpSpanTest)); @@ -519,6 +521,7 @@ public void ToOtlpSpanStatusTagIsCaseInsensitiveTest(StatusCode expectedStatusCo } [Fact] + [Obsolete("Remove when ActivityExtensions status APIs are removed")] public void ToOtlpSpanActivityStatusTakesPrecedenceOverStatusTagsWhenActivityStatusCodeIsOk() { using var activitySource = new ActivitySource(nameof(this.ToOtlpSpanTest)); @@ -538,6 +541,7 @@ public void ToOtlpSpanActivityStatusTakesPrecedenceOverStatusTagsWhenActivitySta } [Fact] + [Obsolete("Remove when ActivityExtensions status APIs are removed")] public void ToOtlpSpanActivityStatusTakesPrecedenceOverStatusTagsWhenActivityStatusCodeIsError() { using var activitySource = new ActivitySource(nameof(this.ToOtlpSpanTest)); From 9779ed4b52a7b92eae532e97811211c7665b2209 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 21 Aug 2024 11:17:12 -0700 Subject: [PATCH 14/16] Code review. --- .../Trace/ActivityExtensions.cs | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs index a90743b06b7..95310e171e8 100644 --- a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs +++ b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs @@ -36,6 +36,19 @@ public static void SetStatus(this Activity activity, Status status) { if (activity != null) { + switch (status.StatusCode) + { + case StatusCode.Ok: + activity.SetStatus(ActivityStatusCode.Ok); + break; + case StatusCode.Unset: + activity.SetStatus(ActivityStatusCode.Unset); + break; + case StatusCode.Error: + activity.SetStatus(ActivityStatusCode.Error, status.Description); + break; + } + activity.SetTag(SpanAttributeConstants.StatusCodeKey, StatusHelper.GetTagValueForStatusCode(status.StatusCode)); activity.SetTag(SpanAttributeConstants.StatusDescriptionKey, status.Description); } @@ -57,13 +70,23 @@ public static void SetStatus(this Activity activity, Status status) [Obsolete("Use Activity.Status and Activity.StatusDescription instead this method will be removed in a future version.")] public static Status GetStatus(this Activity activity) { - if (activity == null - || !activity.TryGetStatus(out var statusCode, out var statusDescription)) + if (activity != null) { - return Status.Unset; + switch (activity.Status) + { + case ActivityStatusCode.Ok: + return Status.Ok; + case ActivityStatusCode.Error: + return new Status(StatusCode.Error, activity.StatusDescription); + } + + if (activity.TryGetStatus(out var statusCode, out var statusDescription)) + { + return new Status(statusCode, statusDescription); + } } - return new Status(statusCode, statusDescription); + return Status.Unset; } /// From ebfc47b4da1d0c736f567cc8813477411277586f Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 21 Aug 2024 12:28:24 -0700 Subject: [PATCH 15/16] Test fixes when running against released packages. --- .../.publicApi/Stable/PublicAPI.Shipped.txt | 8 ++--- .../Trace/ActivityExtensions.cs | 8 ++--- src/OpenTelemetry.Api/Trace/TelemetrySpan.cs | 3 +- .../OpenTelemetry.Shims.OpenTracing.csproj | 2 ++ .../Trace/Processor/ExceptionProcessor.cs | 3 -- src/Shared/AssemblyVersionExtensions.cs | 31 +++++++++++++++++-- .../SpanBuilderShimTests.cs | 21 ++++++++++--- .../SpanShimTests.cs | 18 ++++++++--- 8 files changed, 70 insertions(+), 24 deletions(-) diff --git a/src/OpenTelemetry.Api/.publicApi/Stable/PublicAPI.Shipped.txt b/src/OpenTelemetry.Api/.publicApi/Stable/PublicAPI.Shipped.txt index 7b02ca885de..94729fd41bd 100644 --- a/src/OpenTelemetry.Api/.publicApi/Stable/PublicAPI.Shipped.txt +++ b/src/OpenTelemetry.Api/.publicApi/Stable/PublicAPI.Shipped.txt @@ -208,10 +208,10 @@ static OpenTelemetry.Baggage.operator !=(OpenTelemetry.Baggage left, OpenTelemet static OpenTelemetry.Baggage.operator ==(OpenTelemetry.Baggage left, OpenTelemetry.Baggage right) -> bool static OpenTelemetry.Context.Propagation.PropagationContext.operator !=(OpenTelemetry.Context.Propagation.PropagationContext left, OpenTelemetry.Context.Propagation.PropagationContext right) -> bool static OpenTelemetry.Context.Propagation.PropagationContext.operator ==(OpenTelemetry.Context.Propagation.PropagationContext left, OpenTelemetry.Context.Propagation.PropagationContext right) -> bool -static OpenTelemetry.Trace.ActivityExtensions.GetStatus(this System.Diagnostics.Activity! activity) -> OpenTelemetry.Trace.Status -static OpenTelemetry.Trace.ActivityExtensions.RecordException(this System.Diagnostics.Activity! activity, System.Exception? ex, in System.Diagnostics.TagList tags) -> void -static OpenTelemetry.Trace.ActivityExtensions.RecordException(this System.Diagnostics.Activity! activity, System.Exception? ex) -> void -static OpenTelemetry.Trace.ActivityExtensions.SetStatus(this System.Diagnostics.Activity! activity, OpenTelemetry.Trace.Status status) -> void +static OpenTelemetry.Trace.ActivityExtensions.GetStatus(this System.Diagnostics.Activity? activity) -> OpenTelemetry.Trace.Status +static OpenTelemetry.Trace.ActivityExtensions.RecordException(this System.Diagnostics.Activity? activity, System.Exception? ex, in System.Diagnostics.TagList tags) -> void +static OpenTelemetry.Trace.ActivityExtensions.RecordException(this System.Diagnostics.Activity? activity, System.Exception? ex) -> void +static OpenTelemetry.Trace.ActivityExtensions.SetStatus(this System.Diagnostics.Activity? activity, OpenTelemetry.Trace.Status status) -> void static OpenTelemetry.Trace.Link.operator !=(OpenTelemetry.Trace.Link link1, OpenTelemetry.Trace.Link link2) -> bool static OpenTelemetry.Trace.Link.operator ==(OpenTelemetry.Trace.Link link1, OpenTelemetry.Trace.Link link2) -> bool static OpenTelemetry.Trace.SpanContext.implicit operator System.Diagnostics.ActivityContext(OpenTelemetry.Trace.SpanContext spanContext) -> System.Diagnostics.ActivityContext diff --git a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs index 95310e171e8..a94a635055b 100644 --- a/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs +++ b/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs @@ -32,7 +32,7 @@ public static class ActivityExtensions /// Activity execution status. [MethodImpl(MethodImplOptions.AggressiveInlining)] [Obsolete("Call Activity.SetStatus instead this method will be removed in a future version.")] - public static void SetStatus(this Activity activity, Status status) + public static void SetStatus(this Activity? activity, Status status) { if (activity != null) { @@ -68,7 +68,7 @@ public static void SetStatus(this Activity activity, Status status) /// Activity execution status. [MethodImpl(MethodImplOptions.AggressiveInlining)] [Obsolete("Use Activity.Status and Activity.StatusDescription instead this method will be removed in a future version.")] - public static Status GetStatus(this Activity activity) + public static Status GetStatus(this Activity? activity) { if (activity != null) { @@ -98,7 +98,7 @@ public static Status GetStatus(this Activity activity) /// "exception.stacktrace" is represented using the value of Exception.ToString. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void RecordException(this Activity activity, Exception? ex) + public static void RecordException(this Activity? activity, Exception? ex) => RecordException(activity, ex, default); /// @@ -111,7 +111,7 @@ public static void RecordException(this Activity activity, Exception? ex) /// "exception.stacktrace" is represented using the value of Exception.ToString. /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static void RecordException(this Activity activity, Exception? ex, in TagList tags) + public static void RecordException(this Activity? activity, Exception? ex, in TagList tags) { if (ex == null || activity == null) { diff --git a/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs b/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs index b809f439e48..cdae8d4292c 100644 --- a/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs +++ b/src/OpenTelemetry.Api/Trace/TelemetrySpan.cs @@ -48,9 +48,8 @@ public ActivitySpanId ParentSpanId public void SetStatus(Status value) { #pragma warning disable - this.Activity?.SetStatus(value); + this.Activity.SetStatus(value); #pragma warning restore - this.Activity?.SetStatus((ActivityStatusCode)value.StatusCode); } /// diff --git a/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj b/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj index 645c9e49e59..e746024d006 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj +++ b/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj @@ -22,7 +22,9 @@ + + diff --git a/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs b/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs index 45f21bbf1cc..f3845519408 100644 --- a/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs +++ b/src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs @@ -65,9 +65,6 @@ public override void OnEnd(Activity activity) #pragma warning disable activity.SetStatus(Status.Error); #pragma warning restore - - // For processors/exporters checking `Status` property. - activity.SetStatus(ActivityStatusCode.Error); } } } diff --git a/src/Shared/AssemblyVersionExtensions.cs b/src/Shared/AssemblyVersionExtensions.cs index 029a3c2451c..b3e43d5bc9f 100644 --- a/src/Shared/AssemblyVersionExtensions.cs +++ b/src/Shared/AssemblyVersionExtensions.cs @@ -4,6 +4,7 @@ #nullable enable using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Reflection; namespace OpenTelemetry.Internal; @@ -11,6 +12,33 @@ namespace OpenTelemetry.Internal; internal static class AssemblyVersionExtensions { public static string GetPackageVersion(this Assembly assembly) + { + Debug.Assert(assembly != null, "assembly was null"); + + var informationalVersion = assembly.GetCustomAttribute()?.InformationalVersion; + + Debug.Assert(!string.IsNullOrEmpty(informationalVersion), "AssemblyInformationalVersionAttribute was not found in assembly"); + + return ParsePackageVersion(informationalVersion!); + } + + public static bool TryGetPackageVersion(this Assembly assembly, [NotNullWhen(true)] out string? packageVersion) + { + Debug.Assert(assembly != null, "assembly was null"); + + var informationalVersion = assembly.GetCustomAttribute()?.InformationalVersion; + + if (string.IsNullOrEmpty(informationalVersion)) + { + packageVersion = null; + return false; + } + + packageVersion = ParsePackageVersion(informationalVersion!); + return true; + } + + private static string ParsePackageVersion(string informationalVersion) { // MinVer https://github.com/adamralph/minver?tab=readme-ov-file#version-numbers // together with Microsoft.SourceLink.GitHub https://github.com/dotnet/sourcelink @@ -20,9 +48,6 @@ public static string GetPackageVersion(this Assembly assembly) // The following parts are optional: pre-release label, pre-release version, git height, Git SHA of current commit // For package version, value of AssemblyInformationalVersionAttribute without commit hash is returned. - var informationalVersion = assembly.GetCustomAttribute()?.InformationalVersion; - Debug.Assert(!string.IsNullOrEmpty(informationalVersion), "AssemblyInformationalVersionAttribute was not found in assembly"); - var indexOfPlusSign = informationalVersion!.IndexOf('+'); return indexOfPlusSign > 0 ? informationalVersion.Substring(0, indexOfPlusSign) diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs index 9843e7f6132..7e9c9f71246 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics; +using OpenTelemetry.Internal; using OpenTelemetry.Trace; using Xunit; @@ -219,8 +220,14 @@ public void WithTag_KeyIsErrorStringValue() // Legacy span status tag should be set Assert.Equal("ERROR", spanShim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - // Activity status code should also be set - Assert.Equal(ActivityStatusCode.Error, spanShim.Span.Activity.Status); + bool isApiVersionGreaterThanOrEqualTo1_10 = !typeof(TracerProvider).Assembly.TryGetPackageVersion(out var packageVersion) + || Version.Parse(packageVersion) >= new Version(1, 10); + + if (isApiVersionGreaterThanOrEqualTo1_10) + { + // Activity status code should also be set + Assert.Equal(ActivityStatusCode.Error, spanShim.Span.Activity.Status); + } } [Fact] @@ -268,8 +275,14 @@ public void WithTag_KeyIsErrorBoolValue() // Legacy span status tag should be set Assert.Equal("ERROR", spanShim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - // Activity status code should also be set - Assert.Equal(ActivityStatusCode.Error, spanShim.Span.Activity.Status); + bool isApiVersionGreaterThanOrEqualTo1_10 = !typeof(TracerProvider).Assembly.TryGetPackageVersion(out var packageVersion) + || Version.Parse(packageVersion) >= new Version(1, 10); + + if (isApiVersionGreaterThanOrEqualTo1_10) + { + // Activity status code should also be set + Assert.Equal(ActivityStatusCode.Error, spanShim.Span.Activity.Status); + } } [Fact] diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs index 3664ed97731..93a0bf8f4c5 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics; +using OpenTelemetry.Internal; using OpenTelemetry.Trace; using OpenTracing.Tag; using Xunit; @@ -214,16 +215,25 @@ public void SetTagBoolValue() // Legacy span status tag should be set Assert.Equal("ERROR", shim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - // Activity status code should also be set - Assert.Equal(ActivityStatusCode.Error, shim.Span.Activity.Status); + bool isApiVersionGreaterThanOrEqualTo1_10 = !typeof(TracerProvider).Assembly.TryGetPackageVersion(out var packageVersion) + || Version.Parse(packageVersion) >= new Version(1, 10); + + if (isApiVersionGreaterThanOrEqualTo1_10) + { + // Activity status code should also be set + Assert.Equal(ActivityStatusCode.Error, shim.Span.Activity.Status); + } shim.SetTag(Tags.Error.Key, false); // Legacy span status tag should be set Assert.Equal("OK", shim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - // Activity status code should also be set - Assert.Equal(ActivityStatusCode.Ok, shim.Span.Activity.Status); + if (isApiVersionGreaterThanOrEqualTo1_10) + { + // Activity status code should also be set + Assert.Equal(ActivityStatusCode.Ok, shim.Span.Activity.Status); + } } [Fact] From 1eb54cac13a261d8db7cbb5e1d9ad892ed1a1dc7 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Wed, 21 Aug 2024 14:52:43 -0700 Subject: [PATCH 16/16] Tweaks. --- test/Directory.Packages.props | 3 +- ...enTelemetry.Shims.OpenTracing.Tests.csproj | 6 +-- .../SpanBuilderShimTests.cs | 19 +++++---- .../SpanShimTests.cs | 16 ++++--- .../VersionHelper.cs | 42 +++++++++++++++++++ 5 files changed, 67 insertions(+), 19 deletions(-) create mode 100644 test/OpenTelemetry.Shims.OpenTracing.Tests/VersionHelper.cs diff --git a/test/Directory.Packages.props b/test/Directory.Packages.props index 4800f76a845..b3769fc1459 100644 --- a/test/Directory.Packages.props +++ b/test/Directory.Packages.props @@ -1,8 +1,9 @@ - + + diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/OpenTelemetry.Shims.OpenTracing.Tests.csproj b/test/OpenTelemetry.Shims.OpenTracing.Tests/OpenTelemetry.Shims.OpenTracing.Tests.csproj index 07fe011a082..1aac45f10ca 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/OpenTelemetry.Shims.OpenTracing.Tests.csproj +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/OpenTelemetry.Shims.OpenTracing.Tests.csproj @@ -2,16 +2,16 @@ Unit test project for OpenTelemetry.Shims.OpenTracing $(TargetFrameworksForTests) + $(DefineConstants);BUILDING_USING_PROJECTS disable + - - runtime; build; native; contentfiles; analyzers - + diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs index 7e9c9f71246..a3a58facfd9 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics; -using OpenTelemetry.Internal; using OpenTelemetry.Trace; using Xunit; @@ -220,14 +219,15 @@ public void WithTag_KeyIsErrorStringValue() // Legacy span status tag should be set Assert.Equal("ERROR", spanShim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - bool isApiVersionGreaterThanOrEqualTo1_10 = !typeof(TracerProvider).Assembly.TryGetPackageVersion(out var packageVersion) - || Version.Parse(packageVersion) >= new Version(1, 10); - - if (isApiVersionGreaterThanOrEqualTo1_10) + if (VersionHelper.IsApiVersionGreaterThanOrEqualTo(1, 10)) { // Activity status code should also be set Assert.Equal(ActivityStatusCode.Error, spanShim.Span.Activity.Status); } + else + { + Assert.Equal(ActivityStatusCode.Unset, spanShim.Span.Activity.Status); + } } [Fact] @@ -275,14 +275,15 @@ public void WithTag_KeyIsErrorBoolValue() // Legacy span status tag should be set Assert.Equal("ERROR", spanShim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - bool isApiVersionGreaterThanOrEqualTo1_10 = !typeof(TracerProvider).Assembly.TryGetPackageVersion(out var packageVersion) - || Version.Parse(packageVersion) >= new Version(1, 10); - - if (isApiVersionGreaterThanOrEqualTo1_10) + if (VersionHelper.IsApiVersionGreaterThanOrEqualTo(1, 10)) { // Activity status code should also be set Assert.Equal(ActivityStatusCode.Error, spanShim.Span.Activity.Status); } + else + { + Assert.Equal(ActivityStatusCode.Unset, spanShim.Span.Activity.Status); + } } [Fact] diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs index 93a0bf8f4c5..9911d9f76fc 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics; -using OpenTelemetry.Internal; using OpenTelemetry.Trace; using OpenTracing.Tag; using Xunit; @@ -215,25 +214,30 @@ public void SetTagBoolValue() // Legacy span status tag should be set Assert.Equal("ERROR", shim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - bool isApiVersionGreaterThanOrEqualTo1_10 = !typeof(TracerProvider).Assembly.TryGetPackageVersion(out var packageVersion) - || Version.Parse(packageVersion) >= new Version(1, 10); - - if (isApiVersionGreaterThanOrEqualTo1_10) + if (VersionHelper.IsApiVersionGreaterThanOrEqualTo(1, 10)) { // Activity status code should also be set Assert.Equal(ActivityStatusCode.Error, shim.Span.Activity.Status); } + else + { + Assert.Equal(ActivityStatusCode.Unset, shim.Span.Activity.Status); + } shim.SetTag(Tags.Error.Key, false); // Legacy span status tag should be set Assert.Equal("OK", shim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - if (isApiVersionGreaterThanOrEqualTo1_10) + if (VersionHelper.IsApiVersionGreaterThanOrEqualTo(1, 10)) { // Activity status code should also be set Assert.Equal(ActivityStatusCode.Ok, shim.Span.Activity.Status); } + else + { + Assert.Equal(ActivityStatusCode.Unset, shim.Span.Activity.Status); + } } [Fact] diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/VersionHelper.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/VersionHelper.cs new file mode 100644 index 00000000000..57b1b8a9f53 --- /dev/null +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/VersionHelper.cs @@ -0,0 +1,42 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#nullable enable + +using NuGet.Versioning; +using OpenTelemetry.Internal; +using OpenTelemetry.Trace; + +namespace OpenTelemetry.Shims.OpenTracing.Tests; + +internal static class VersionHelper +{ +#if BUILDING_USING_PROJECTS + private static NuGetVersion? apiVersion = new(100, 0, 0); +#else + private static NuGetVersion? apiVersion; +#endif + + public static NuGetVersion ApiVersion + { + get + { + return apiVersion ??= ResolveApiVersion(); + + static NuGetVersion ResolveApiVersion() + { + if (!typeof(TracerProvider).Assembly.TryGetPackageVersion(out var packageVersion)) + { + throw new InvalidOperationException("OpenTelemetry.Api package version could not be resolved"); + } + + return NuGetVersion.Parse(packageVersion); + } + } + } + + public static bool IsApiVersionGreaterThanOrEqualTo(int major, int minor) + { + return ApiVersion >= new NuGetVersion(major, minor, 0); + } +}