Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Status Improvements #1579

Merged
merged 10 commits into from
Nov 18, 2020
4 changes: 1 addition & 3 deletions examples/Console/TestRedis.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,7 @@ private static void DoWork(IDatabase db, ActivitySource activitySource)
}
catch (ArgumentOutOfRangeException e)
{
// Set status upon error
activity.SetTag(SpanAttributeConstants.StatusCodeKey, (int)Status.Error.StatusCode);
activity.SetTag(SpanAttributeConstants.StatusDescriptionKey, e.ToString());
activity.SetStatus(Status.Error.WithDescription(e.ToString()));
}

// Annotate our activity to capture metadata about our operation
Expand Down
7 changes: 7 additions & 0 deletions src/OpenTelemetry.Api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

## Unreleased

* In order to align with the
[spec](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#set-status)
the `Status` (otel.status_code) tag (added on `Activity` using the `SetStatus`
extension) will now be set as the `Unset`, `Error`, or `Ok` string
representation instead of the `0`, `1`, or `2` integer representation.
([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579))

## 1.0.0-rc1.1

Released 2020-Nov-17
Expand Down
45 changes: 14 additions & 31 deletions src/OpenTelemetry.Api/Internal/ActivityHelperExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
// limitations under the License.
// </copyright>

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Reflection;
Expand All @@ -34,42 +33,29 @@ internal static class ActivityHelperExtensions
/// This extension provides a workaround to retrieve Status from special tags with key name otel.status_code and otel.status_description.
/// </summary>
/// <param name="activity">Activity instance.</param>
/// <returns>Activity execution status.</returns>
/// <param name="statusCode"><see cref="StatusCode"/>.</param>
/// <param name="statusDescription">Status description.</param>
/// <returns><see langword="true"/> if <see cref="Status"/> was found on the supplied Activity.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "ActivityProcessor is hot path")]
public static Status GetStatusHelper(this Activity activity)
public static bool TryGetStatus(this Activity activity, out StatusCode statusCode, out string statusDescription)
{
Debug.Assert(activity != null, "Activity should not be null");

ActivityStatusTagEnumerator state = default;

ActivityTagsEnumeratorFactory<ActivityStatusTagEnumerator>.Enumerate(activity, ref state);

if (!state.IsValid)
if (!state.StatusCode.HasValue)
{
return default;
statusCode = default;
statusDescription = null;
return false;
}

Status status;
if (state.StatusCode == StatusCode.Error)
{
status = Status.Error;
}
else if (state.StatusCode == StatusCode.Ok)
{
status = Status.Ok;
}
else
{
status = Status.Unset;
}

if (!string.IsNullOrEmpty(state.StatusDescription))
{
return status.WithDescription(state.StatusDescription);
}

return status;
statusCode = state.StatusCode.Value;
statusDescription = state.StatusDescription;
return true;
}

/// <summary>
Expand Down Expand Up @@ -193,9 +179,7 @@ public bool ForEach(KeyValuePair<string, object> item)

private struct ActivityStatusTagEnumerator : IActivityEnumerator<KeyValuePair<string, object>>
{
public bool IsValid;

public StatusCode StatusCode;
public StatusCode? StatusCode;

public string StatusDescription;

Expand All @@ -204,15 +188,14 @@ public bool ForEach(KeyValuePair<string, object> item)
switch (item.Key)
{
case SpanAttributeConstants.StatusCodeKey:
this.StatusCode = (StatusCode)item.Value;
this.IsValid = this.StatusCode == StatusCode.Error || this.StatusCode == StatusCode.Ok || this.StatusCode == StatusCode.Unset;
this.StatusCode = StatusHelper.GetStatusCodeForStringName(item.Value as string);
break;
case SpanAttributeConstants.StatusDescriptionKey:
this.StatusDescription = item.Value as string;
break;
}

return this.IsValid || this.StatusDescription == null;
return !this.StatusCode.HasValue || this.StatusDescription == null;
}
}

Expand Down
6 changes: 2 additions & 4 deletions src/OpenTelemetry.Api/Internal/SpanHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,12 @@ internal static class SpanHelper
/// <returns>Resolved span <see cref="Status"/> for the Http status code.</returns>
public static Status ResolveSpanStatusForHttpStatusCode(int httpStatusCode)
{
var status = Status.Error;

if (httpStatusCode >= 100 && httpStatusCode <= 399)
{
status = Status.Unset;
return Status.Unset;
}

return status;
return Status.Error;
}
}
}
58 changes: 58 additions & 0 deletions src/OpenTelemetry.Api/Internal/StatusHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// <copyright file="StatusHelper.cs" company="OpenTelemetry Authors">
// 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.
// </copyright>

using System.Runtime.CompilerServices;
using OpenTelemetry.Trace;

namespace OpenTelemetry.Internal
{
internal static class StatusHelper
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static string GetStringNameForStatusCode(StatusCode statusCode)
{
return statusCode switch
{
/*
* Note: Order here does matter for perf. Unset is
* first because assumption is most spans will be
* Unset, then Error. Ok is not set by the SDK.
*/
StatusCode.Unset => "Unset",
StatusCode.Error => "Error",
StatusCode.Ok => "Ok",
_ => null,
};
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static StatusCode? GetStatusCodeForStringName(string statusCodeName)
{
return statusCodeName switch
{
/*
* Note: Order here does matter for perf. Unset is
* first because assumption is most spans will be
* Unset, then Error. Ok is not set by the SDK.
*/
"Unset" => StatusCode.Unset,
"Error" => StatusCode.Error,
"Ok" => StatusCode.Ok,
_ => (StatusCode?)null,
};
}
}
}
9 changes: 7 additions & 2 deletions src/OpenTelemetry.Api/Trace/ActivityExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public static void SetStatus(this Activity activity, Status status)
{
Debug.Assert(activity != null, "Activity should not be null");

activity.SetTag(SpanAttributeConstants.StatusCodeKey, (int)status.StatusCode);
activity.SetTag(SpanAttributeConstants.StatusCodeKey, StatusHelper.GetStringNameForStatusCode(status.StatusCode));
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
activity.SetTag(SpanAttributeConstants.StatusDescriptionKey, status.Description);
}

Expand All @@ -55,7 +55,12 @@ public static void SetStatus(this Activity activity, Status status)
[System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "ActivityProcessor is hot path")]
public static Status GetStatus(this Activity activity)
{
return activity.GetStatusHelper();
if (!activity.TryGetStatus(out StatusCode statusCode, out string statusDescription))
{
return Status.Unset;
}

return new Status(statusCode, statusDescription);
}

/// <summary>
Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Jaeger will now set the `error` tag when `otel.status_code` is set to `Error`.
([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579))

## 1.0.0-rc1.1

Released 2020-Nov-17
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,11 @@ private static void ProcessJaegerTag(ref TagEnumerationState state, string key,
if (jaegerTag.VStr != null)
{
PeerServiceResolver.InspectTag(ref state, key, jaegerTag.VStr);

if (key == SpanAttributeConstants.StatusCodeKey && jaegerTag.VStr == "Error")
{
PooledList<JaegerTag>.Add(ref state.Tags, new JaegerTag("error", JaegerTagType.BOOL, vBool: true));
}
}
else if (jaegerTag.VLong.HasValue)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SemanticConventions.cs" Link="Includes\SemanticConventions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SpanAttributeConstants.cs" Link="Includes\SpanAttributeConstants.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\ActivityHelperExtensions.cs" Link="Includes\ActivityHelperExtensions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\StatusHelper.cs" Link="Includes\StatusHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\IActivityEnumerator.cs" Link="Includes\IActivityEnumerator.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\EnumerationHelper.cs" Link="Includes\EnumerationHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\PooledList.cs" Link="Includes\PooledList.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,15 +239,17 @@ internal static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair<string, ob
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static OtlpTrace.Status ToOtlpStatus(ref TagEnumerationState otlpTags)
{
if (!otlpTags.StatusCode.HasValue)
var status = StatusHelper.GetStatusCodeForStringName(otlpTags.StatusCode);

if (!status.HasValue)
{
return null;
}

var otlpStatus = new OtlpTrace.Status
{
// The numerical values of the two enumerations match, a simple cast is enough.
Code = (OtlpTrace.Status.Types.StatusCode)otlpTags.StatusCode,
Code = (OtlpTrace.Status.Types.StatusCode)(int)status,
};

if (otlpStatus.Code != OtlpTrace.Status.Types.StatusCode.Error)
Expand Down Expand Up @@ -370,7 +372,7 @@ private struct TagEnumerationState : IActivityEnumerator<KeyValuePair<string, ob

public PooledList<OtlpCommon.KeyValue> Tags;

public int? StatusCode;
public string StatusCode;

public string StatusDescription;

Expand All @@ -396,7 +398,7 @@ public bool ForEach(KeyValuePair<string, object> activityTag)
switch (key)
{
case SpanAttributeConstants.StatusCodeKey:
this.StatusCode = activityTag.Value as int?;
this.StatusCode = activityTag.Value as string;
return true;
case SpanAttributeConstants.StatusDescriptionKey:
this.StatusDescription = activityTag.Value as string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SemanticConventions.cs" Link="Includes\SemanticConventions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SpanAttributeConstants.cs" Link="Includes\SpanAttributeConstants.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\ActivityHelperExtensions.cs" Link="Includes\ActivityHelperExtensions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\StatusHelper.cs" Link="Includes\StatusHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\IActivityEnumerator.cs" Link="Includes\IActivityEnumerator.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\EnumerationHelper.cs" Link="Includes\EnumerationHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\PooledList.cs" Link="Includes\PooledList.cs" />
Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Zipkin will now set the `error` tag when `otel.status_code` is set to `Error`.
([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579))

## 1.0.0-rc1.1

Released 2020-Nov-17
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ public bool ForEach(KeyValuePair<string, object> activityTag)
if (activityTag.Value is string strVal)
{
PeerServiceResolver.InspectTag(ref this, key, strVal);

if (key == SpanAttributeConstants.StatusCodeKey && strVal == "Error")
{
PooledList<KeyValuePair<string, object>>.Add(ref this.Tags, new KeyValuePair<string, object>("error", "true"));
}
}
else if (activityTag.Value is int intVal && activityTag.Key == SemanticConventions.AttributeNetPeerPort)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SemanticConventions.cs" Link="Includes\SemanticConventions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SpanAttributeConstants.cs" Link="Includes\SpanAttributeConstants.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\ActivityHelperExtensions.cs" Link="Includes\ActivityHelperExtensions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\StatusHelper.cs" Link="Includes\StatusHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\IActivityEnumerator.cs" Link="Includes\IActivityEnumerator.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\EnumerationHelper.cs" Link="Includes\EnumerationHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry\Internal\PooledList.cs" Link="Includes\PooledList.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@ public override void OnStopActivity(Activity activity, object payload)

if (activityToEnrich.GetStatus().StatusCode == StatusCode.Unset)
{
Status status = SpanHelper.ResolveSpanStatusForHttpStatusCode(response.StatusCode);
activityToEnrich.SetStatus(status);
activityToEnrich.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(response.StatusCode));
}

var routeData = context.Request.RequestContext.RouteData;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,13 @@ public override void OnStopActivity(Activity activity, object payload)
{
if (activity.GetStatus().StatusCode == StatusCode.Unset)
{
SetStatusFromHttpStatusCode(activity, response.StatusCode);
activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(response.StatusCode));
}
}
#else
if (activity.GetStatus().StatusCode == StatusCode.Unset)
{
SetStatusFromHttpStatusCode(activity, response.StatusCode);
activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(response.StatusCode));
}
#endif

Expand Down Expand Up @@ -304,13 +304,6 @@ private static string GetUri(HttpRequest request)
return builder.ToString();
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void SetStatusFromHttpStatusCode(Activity activity, int statusCode)
{
var status = SpanHelper.ResolveSpanStatusForHttpStatusCode(statusCode);
activity.SetStatus(status);
}

#if NETSTANDARD2_1
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static bool TryGetGrpcMethod(Activity activity, out string grpcMethod)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SpanAttributeConstants.cs" Link="Includes\SpanAttributeConstants.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\ExceptionExtensions.cs" Link="Includes\ExceptionExtensions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\ActivityHelperExtensions.cs" Link="Includes\ActivityHelperExtensions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\StatusHelper.cs" Link="Includes\StatusHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\IActivityEnumerator.cs" Link="Includes\IActivityEnumerator.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\EnumerationHelper.cs" Link="Includes\EnumerationHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Instrumentation.GrpcNetClient\GrpcTagHelper.cs" Link="Includes\GrpcTagHelper.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\ExceptionExtensions.cs" Link="Includes\ExceptionExtensions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SemanticConventions.cs" Link="Includes\SemanticConventions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\ActivityHelperExtensions.cs" Link="Includes\ActivityHelperExtensions.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\StatusHelper.cs" Link="Includes\StatusHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\IActivityEnumerator.cs" Link="Includes\IActivityEnumerator.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\EnumerationHelper.cs" Link="Includes\EnumerationHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Api\Internal\SpanAttributeConstants.cs" Link="Includes\SpanAttributeConstants.cs" />
Expand Down
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* `otel.status_description` tag will no longer be set to the http status
description/reason phrase for outgoing http spans.
([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579))

## 1.0.0-rc1.1

Released 2020-Nov-17
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,7 @@ public override void OnStopActivity(Activity activity, object payload)

if (currentStatusCode == StatusCode.Unset)
{
activity.SetStatus(
SpanHelper
.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode)
.WithDescription(response.ReasonPhrase));
activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode));
}

try
Expand Down
Loading