Skip to content

Commit

Permalink
[api] Mark SetStatus & GetStatus ActivityExtensions obsolete (#5781)
Browse files Browse the repository at this point in the history
Co-authored-by: Vishwesh Bankwar <[email protected]>
Co-authored-by: Mikel Blanchard <[email protected]>
  • Loading branch information
3 people authored Aug 21, 2024
1 parent c483222 commit 1b3f189
Show file tree
Hide file tree
Showing 16 changed files with 219 additions and 85 deletions.
8 changes: 4 additions & 4 deletions src/OpenTelemetry.Api/.publicApi/Stable/PublicAPI.Shipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions src/OpenTelemetry.Api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,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
Expand Down
59 changes: 46 additions & 13 deletions src/OpenTelemetry.Api/Trace/ActivityExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,39 +21,72 @@ public static class ActivityExtensions
{
/// <summary>
/// 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.
/// </summary>
/// <remarks>
/// Note: This method is obsolete. Call the <see cref="Activity.SetStatus"/>
/// method instead. For more details see: <see
/// href="https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Api#setting-status"
/// />.
/// </remarks>
/// <param name="activity">Activity instance.</param>
/// <param name="status">Activity execution status.</param>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void SetStatus(this Activity activity, Status status)
[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)
{
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);
}
}

/// <summary>
/// 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.
/// </summary>
/// <remarks>
/// Note: This method is obsolete. Use the <see cref="Activity.Status"/> and
/// <see cref="Activity.StatusDescription"/> properties instead. For more
/// details see: <see
/// href="https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Api#setting-status"
/// />.
/// </remarks>
/// <param name="activity">Activity instance.</param>
/// <returns>Activity execution status.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static Status GetStatus(this Activity activity)
[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;
}

/// <summary>
Expand All @@ -65,7 +98,7 @@ public static Status GetStatus(this Activity activity)
/// "exception.stacktrace" is represented using the value of <a href="https://learn.microsoft.com/dotnet/api/system.exception.tostring">Exception.ToString</a>.
/// </remarks>
[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);

/// <summary>
Expand All @@ -78,7 +111,7 @@ public static void RecordException(this Activity activity, Exception? ex)
/// "exception.stacktrace" is represented using the value of <a href="https://learn.microsoft.com/dotnet/api/system.exception.tostring">Exception.ToString</a>.
/// </remarks>
[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)
{
Expand Down
4 changes: 3 additions & 1 deletion src/OpenTelemetry.Api/Trace/TelemetrySpan.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ public ActivitySpanId ParentSpanId
/// <param name="value">Status to be set.</param>
public void SetStatus(Status value)
{
this.Activity?.SetStatus(value);
#pragma warning disable
this.Activity.SetStatus(value);
#pragma warning restore
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
</ItemGroup>

<ItemGroup>
<Compile Include="$(RepoRoot)\src\Shared\AssemblyVersionExtensions.cs" Link="Includes\AssemblyVersionExtensions.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
</ItemGroup>

</Project>
6 changes: 2 additions & 4 deletions src/OpenTelemetry/Trace/Processor/ExceptionProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,9 @@ public override void OnEnd(Activity activity)

if (snapshot != pointers)
{
// TODO: Remove this when SetStatus is deprecated
#pragma warning disable
activity.SetStatus(Status.Error);

// For processors/exporters checking `Status` property.
activity.SetStatus(ActivityStatusCode.Error);
#pragma warning restore
}
}
}
1 change: 1 addition & 0 deletions src/Shared/ActivityHelperExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ internal static class ActivityHelperExtensions
/// <param name="statusDescription">Status description.</param>
/// <returns><see langword="true"/> if <see cref="Status"/> was found on the supplied Activity.</returns>
[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");
Expand Down
31 changes: 28 additions & 3 deletions src/Shared/AssemblyVersionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,41 @@
#nullable enable

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;

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<AssemblyInformationalVersionAttribute>()?.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<AssemblyInformationalVersionAttribute>()?.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
Expand All @@ -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<AssemblyInformationalVersionAttribute>()?.InformationalVersion;
Debug.Assert(!string.IsNullOrEmpty(informationalVersion), "AssemblyInformationalVersionAttribute was not found in assembly");

var indexOfPlusSign = informationalVersion!.IndexOf('+');
return indexOfPlusSign > 0
? informationalVersion.Substring(0, indexOfPlusSign)
Expand Down
3 changes: 2 additions & 1 deletion test/Directory.Packages.props
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<Project>
<Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Packages.props, $(MSBuildThisFileDirectory)..))" />
<ItemGroup>
<PackageVersion Update="System.Text.Json" Version="8.0.4" />
<PackageVersion Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
<PackageVersion Update="System.Text.Json" Version="8.0.4" />
<PackageVersion Include="NuGet.Versioning" Version="6.11.0" />
<PackageVersion Include="Microsoft.Coyote" Version="1.7.10" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,8 @@ public void ToOtlpSpanTest()
links: childLinks);

Assert.NotNull(childActivity);
childActivity.SetStatus(Status.Error);

childActivity.SetStatus(ActivityStatusCode.Error);

var childEvents = new List<ActivityEvent> { new("e0"), new("e1", default, new ActivityTagsCollection(attributes)) };
childActivity.AddEvent(childEvents[0]);
Expand All @@ -388,7 +389,8 @@ 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.Empty(otlpSpan.Attributes);
Expand Down Expand Up @@ -474,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));
Expand Down Expand Up @@ -502,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));
Expand All @@ -517,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));
Expand All @@ -536,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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 1b3f189

Please sign in to comment.