Status cleanup (#838)

* Cleaned up status usage.

* Unit test fixup.

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
This commit is contained in:
Mikel Blanchard 2020-07-18 07:56:38 -07:00 committed by GitHub
parent eb42d6b6d1
commit e13c33f596
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 41 additions and 68 deletions

View File

@ -35,12 +35,10 @@ namespace OpenTelemetry.Trace
/// </summary>
/// <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)
{
if (activity == null)
{
throw new ArgumentNullException(nameof(activity));
}
Debug.Assert(activity != null, "Activity should not be null");
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
if (!string.IsNullOrEmpty(status.Description))
@ -56,12 +54,10 @@ namespace OpenTelemetry.Trace
/// </summary>
/// <param name="activity">Activity instance.</param>
/// <returns>Activity execution status.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static Status GetStatus(this Activity activity)
{
if (activity == null)
{
throw new ArgumentNullException(nameof(activity));
}
Debug.Assert(activity != null, "Activity should not be null");
var statusCanonicalCode = activity.Tags.FirstOrDefault(k => k.Key == SpanAttributeConstants.StatusCodeKey).Value;
var statusDescription = activity.Tags.FirstOrDefault(d => d.Key == SpanAttributeConstants.StatusDescriptionKey).Value;

View File

@ -139,10 +139,13 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation
}
var response = context.Response;
activityToEnrich.AddTag(SemanticConventions.AttributeHTTPStatusCode, response.StatusCode.ToString());
Status status = SpanHelper.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode);
activityToEnrich.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
activityToEnrich.AddTag(SpanAttributeConstants.StatusDescriptionKey, response.StatusDescription);
activityToEnrich.SetStatus(
SpanHelper
.ResolveSpanStatusForHttpStatusCode(response.StatusCode)
.WithDescription(response.StatusDescription));
var routeData = context.Request.RequestContext.RouteData;

View File

@ -132,14 +132,8 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
var response = context.Response;
activity.AddTag(SemanticConventions.AttributeHTTPStatusCode, response.StatusCode.ToString());
Status status = SpanHelper.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode);
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
var statusDescription = response.HttpContext.Features.Get<IHttpResponseFeature>().ReasonPhrase;
if (!string.IsNullOrEmpty(statusDescription))
{
activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, statusDescription);
}
Status status = SpanHelper.ResolveSpanStatusForHttpStatusCode(response.StatusCode);
activity.SetStatus(status.WithDescription(response.HttpContext.Features.Get<IHttpResponseFeature>()?.ReasonPhrase));
}
if (activity.OperationName.Equals(ActivityNameByHttpInListener))

View File

@ -34,7 +34,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies
return activity.Tags.FirstOrDefault(tag => tag.Key == GrpcMethodTagName).Value;
}
public static string GetGrpcStatusCodeFromActivity(Activity activity)
public static Status GetGrpcStatusCodeFromActivity(Activity activity)
{
var status = Status.Unknown;
@ -44,7 +44,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies
status = SpanHelper.ResolveSpanStatusForGrpcStatusCode(statusCode);
}
return SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode);
return status;
}
public static bool TryParseRpcServiceAndRpcMethod(string grpcMethod, out string rpcService, out string rpcMethod)

View File

@ -80,7 +80,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
{
if (activity.IsAllDataRequested)
{
activity.AddTag(SpanAttributeConstants.StatusCodeKey, GrpcTagHelper.GetGrpcStatusCodeFromActivity(activity));
activity.SetStatus(GrpcTagHelper.GetGrpcStatusCodeFromActivity(activity));
}
this.activitySource.Stop(activity);

View File

@ -110,14 +110,12 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
{
if (requestTaskStatus == TaskStatus.Canceled)
{
Status status = Status.Cancelled;
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
activity.SetStatus(Status.Cancelled);
}
else if (requestTaskStatus != TaskStatus.Faulted)
{
// Faults are handled in OnException and should already have a span.Status of Unknown w/ Description.
Status status = Status.Unknown;
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
activity.SetStatus(Status.Unknown);
}
}
}
@ -127,9 +125,10 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
// response could be null for DNS issues, timeouts, etc...
activity.AddTag(SemanticConventions.AttributeHTTPStatusCode, response.StatusCode.ToString());
Status status = SpanHelper.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode);
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, response.ReasonPhrase);
activity.SetStatus(
SpanHelper
.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode)
.WithDescription(response.ReasonPhrase));
}
}
@ -153,18 +152,14 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
switch (exception.SocketErrorCode)
{
case SocketError.HostNotFound:
Status status = Status.InvalidArgument;
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, exc.Message);
activity.SetStatus(Status.InvalidArgument.WithDescription(exc.Message));
return;
}
}
if (exc.InnerException != null)
{
Status status = Status.Unknown;
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, exc.Message);
activity.SetStatus(Status.Unknown.WithDescription(exc.Message));
}
}
}

View File

@ -117,10 +117,10 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
{
activity.AddTag(SemanticConventions.AttributeHTTPStatusCode, HttpTagHelper.GetStatusCodeTagValueFromHttpStatusCode(response.StatusCode));
Status status = SpanHelper.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode);
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, response.StatusDescription);
activity.SetStatus(
SpanHelper
.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode)
.WithDescription(response.StatusDescription));
}
}
@ -179,8 +179,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
status = Status.Unknown.WithDescription(exception.Message);
}
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, status.Description);
activity.SetStatus(status);
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]

View File

@ -145,7 +145,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
{
if (activity.IsAllDataRequested)
{
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(StatusCanonicalCode.Ok));
activity.SetStatus(Status.Ok);
}
}
finally
@ -175,9 +175,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
{
if (this.exceptionFetcher.Fetch(payload) is Exception exception)
{
Status status = Status.Unknown;
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(status.CanonicalCode));
activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, exception.Message);
activity.SetStatus(Status.Unknown.WithDescription(exception.Message));
}
else
{

View File

@ -159,19 +159,15 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Implementation
int compositeState = (int)eventData.Payload[1];
if ((compositeState & 0b001) == 0b001)
{
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(StatusCanonicalCode.Ok));
activity.SetStatus(Status.Ok);
}
else if ((compositeState & 0b010) == 0b010)
{
activity.SetStatus(Status.Unknown.WithDescription($"SqlExceptionNumber {eventData.Payload[2]} thrown."));
}
else
{
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(StatusCanonicalCode.Unknown));
if ((compositeState & 0b010) == 0b010)
{
activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, $"SqlExceptionNumber {eventData.Payload[2]} thrown.");
}
else
{
activity.AddTag(SpanAttributeConstants.StatusDescriptionKey, $"Unknown Sql failure.");
}
activity.SetStatus(Status.Unknown.WithDescription("Unknown Sql failure."));
}
}
}

View File

@ -57,7 +57,8 @@ namespace OpenTelemetry.Instrumentation.StackExchangeRedis.Implementation
// Total:
// command.ElapsedTime; // 00:00:32.4988020
activity.AddTag(SpanAttributeConstants.StatusCodeKey, SpanHelper.GetCachedCanonicalCodeString(StatusCanonicalCode.Ok));
activity.SetStatus(Status.Ok);
activity.AddTag(SemanticConventions.AttributeDBSystem, "redis");
activity.AddTag(StackExchangeRedisCallsInstrumentation.RedisFlagsKeyName, command.Flags.ToString());

View File

@ -14,6 +14,7 @@
// limitations under the License.
// </copyright>
using System.Diagnostics;
using OpenTelemetry.Trace;
using Xunit;
namespace OpenTelemetry.Instrumentation.Dependencies.Tests
@ -55,7 +56,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
var statusCode = GrpcTagHelper.GetGrpcStatusCodeFromActivity(activity);
Assert.Equal("Ok", statusCode);
Assert.Equal(StatusCanonicalCode.Ok, statusCode.CanonicalCode);
}
}
}

View File

@ -633,7 +633,7 @@ namespace OpenTelemetry.Instrumentation.Dependencies.Tests
Exception exceptionException = (Exception)exceptionEvent.Value.GetCustomProperty("HttpWebRequest.Exception");
Assert.Contains(exceptionEvent.Value.Tags, i => i.Key == SpanAttributeConstants.StatusCodeKey);
Assert.Contains(exceptionEvent.Value.Tags, i => i.Key == SpanAttributeConstants.StatusDescriptionKey);
Assert.DoesNotContain(exceptionEvent.Value.Tags, i => i.Key == SpanAttributeConstants.StatusDescriptionKey);
}
/// <summary>

View File

@ -70,16 +70,6 @@ namespace OpenTelemetry.Trace.Test
Assert.True(activity.GetStatus().CanonicalCode.Equals(Status.Cancelled.CanonicalCode));
}
[Fact]
public void GetOrSetStatusThrowsExceptionOnNullActivity()
{
using var source = new ActivitySource(ActivitySourceName);
using var activity = source.StartActivity(ActivityName);
Assert.Throws<ArgumentNullException>(() => activity.SetStatus(Status.Ok));
Assert.Throws<ArgumentNullException>(() => activity.GetStatus());
activity?.Stop();
}
[Fact]
public void GetStatusWithNoStatusInActivity()
{