Status & error flag updates to match spec (#1655)

* Status & error flag updates to match spec.

* CHANGELOG updates.
This commit is contained in:
Mikel Blanchard 2020-12-14 15:39:25 -08:00 committed by GitHub
parent 01dc04710a
commit 8cda9ef394
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 110 additions and 22 deletions

View File

@ -15,6 +15,9 @@
[#1501](https://github.com/open-telemetry/opentelemetry-dotnet/issues/1501)
for more information.
([#1611](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1611))
* `Status.WithDescription` will now ignore the provided description if the
`Status.StatusCode` is anything other than `ERROR`.
([#1655](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1655))
## 1.0.0-rc1.1

View File

@ -69,11 +69,18 @@ namespace OpenTelemetry.Trace
/// <summary>
/// Returns a new instance of a status with the description populated.
/// </summary>
/// <remarks>
/// Note: Status Description is only valid for <see
/// cref="StatusCode.Error"/> Status and will be ignored for all other
/// <see cref="Trace.StatusCode"/> values. See the <a
/// href="https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#set-status">Status
/// API</a> for details.
/// </remarks>
/// <param name="description">Description of the status.</param>
/// <returns>New instance of the status class with the description populated.</returns>
public Status WithDescription(string description)
{
if (this.Description == description)
if (this.StatusCode != StatusCode.Error || this.Description == description)
{
return this;
}

View File

@ -25,6 +25,8 @@ namespace OpenTelemetry.Exporter.Jaeger.Implementation
{
internal static class JaegerActivityExtensions
{
internal const string JaegerErrorFlagTagName = "error";
private const int DaysPerYear = 365;
// Number of days in 4 years
@ -263,7 +265,7 @@ namespace OpenTelemetry.Exporter.Jaeger.Implementation
if (statusCode == StatusCode.Error)
{
// Error flag: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/jaeger.md#error-flag
PooledList<JaegerTag>.Add(ref state.Tags, new JaegerTag("error", JaegerTagType.BOOL, vBool: true));
PooledList<JaegerTag>.Add(ref state.Tags, new JaegerTag(JaegerErrorFlagTagName, JaegerTagType.BOOL, vBool: true));
}
else if (!statusCode.HasValue || statusCode == StatusCode.Unset)
{
@ -274,6 +276,11 @@ namespace OpenTelemetry.Exporter.Jaeger.Implementation
// Normalize status since it is user-driven.
jaegerTag = new JaegerTag(key, JaegerTagType.STRING, vStr: StatusHelper.GetTagValueForStatusCode(statusCode.Value));
}
else if (key == JaegerErrorFlagTagName)
{
// Ignore `error` tag if it exists, it will be added based on StatusCode + StatusDescription.
return;
}
}
else if (jaegerTag.VLong.HasValue)
{

View File

@ -5,9 +5,12 @@
* Changed `ZipkinExporter` class and constructor from internal to public.
([#1612](https://github.com/open-telemetry/opentelemetry-dotnet/issues/1612))
* 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) &
[#1620](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1620))
* Zipkin will now set the `error` tag to the `Status.Description` value or an
empty string when `Status.StatusCode` (`otel.status_code` tag) is set to
`ERROR`.
([#1579](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1579),
[#1620](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1620), &
[#1655](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1655))
* Zipkin will no longer send the `otel.status_code` tag if the value is `UNSET`.
([#1609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1609) &

View File

@ -25,6 +25,7 @@ namespace OpenTelemetry.Exporter.Zipkin.Implementation
{
internal static class ZipkinActivityConversionExtensions
{
internal const string ZipkinErrorFlagTagName = "error";
private const long TicksPerMicrosecond = TimeSpan.TicksPerMillisecond / 1000;
private const long UnixEpochTicks = 621355968000000000L; // = DateTimeOffset.FromUnixTimeMilliseconds(0).Ticks
private const long UnixEpochMicroseconds = UnixEpochTicks / TicksPerMicrosecond;
@ -79,6 +80,16 @@ namespace OpenTelemetry.Exporter.Zipkin.Implementation
}
}
if (tagState.StatusCode == StatusCode.Error)
{
// Error flag rule from https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/zipkin.md#status
PooledList<KeyValuePair<string, object>>.Add(
ref tagState.Tags,
new KeyValuePair<string, object>(
ZipkinErrorFlagTagName,
tagState.StatusDescription ?? string.Empty));
}
EventEnumerationState eventState = default;
activity.EnumerateEvents(ref eventState);
@ -162,6 +173,10 @@ namespace OpenTelemetry.Exporter.Zipkin.Implementation
public long Port { get; set; }
public StatusCode? StatusCode { get; set; }
public string StatusDescription { get; set; }
public bool ForEach(KeyValuePair<string, object> activityTag)
{
if (activityTag.Value == null)
@ -177,20 +192,27 @@ namespace OpenTelemetry.Exporter.Zipkin.Implementation
if (key == SpanAttributeConstants.StatusCodeKey)
{
StatusCode? statusCode = StatusHelper.GetStatusCodeForTagValue(strVal);
if (statusCode == StatusCode.Error)
{
// Error flag: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/zipkin.md#error-flag
PooledList<KeyValuePair<string, object>>.Add(ref this.Tags, new KeyValuePair<string, object>("error", string.Empty));
}
else if (!statusCode.HasValue || statusCode == StatusCode.Unset)
this.StatusCode = StatusHelper.GetStatusCodeForTagValue(strVal);
if (!this.StatusCode.HasValue || this.StatusCode == Trace.StatusCode.Unset)
{
// Unset Status is not sent: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/zipkin.md#status
return true;
}
// Normalize status since it is user-driven.
activityTag = new KeyValuePair<string, object>(key, StatusHelper.GetTagValueForStatusCode(statusCode.Value));
activityTag = new KeyValuePair<string, object>(key, StatusHelper.GetTagValueForStatusCode(this.StatusCode.Value));
}
else if (key == SpanAttributeConstants.StatusDescriptionKey)
{
// Description is sent as `error` but only if StatusCode is Error. See: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/zipkin.md#status
this.StatusDescription = strVal;
return true;
}
else if (key == ZipkinErrorFlagTagName)
{
// Ignore `error` tag if it exists, it will be added based on StatusCode + StatusDescription.
return true;
}
}
else if (activityTag.Value is int intVal && activityTag.Key == SemanticConventions.AttributeNetPeerPort)

View File

@ -466,11 +466,11 @@ namespace OpenTelemetry.Exporter.Jaeger.Tests.Implementation
if (expectedStatusCode == StatusCode.Error)
{
Assert.Contains(jaegerSpan.Tags, t => t.Key == "error" && t.VType == JaegerTagType.BOOL && (t.VBool ?? false));
Assert.Contains(jaegerSpan.Tags, t => t.Key == JaegerActivityExtensions.JaegerErrorFlagTagName && t.VType == JaegerTagType.BOOL && (t.VBool ?? false));
}
else
{
Assert.DoesNotContain(jaegerSpan.Tags, t => t.Key == "error");
Assert.DoesNotContain(jaegerSpan.Tags, t => t.Key == JaegerActivityExtensions.JaegerErrorFlagTagName);
}
}

View File

@ -138,8 +138,16 @@ namespace OpenTelemetry.Exporter.Zipkin.Tests
[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)]
public void IntegrationTest(bool useShortTraceIds, bool useTestResource, bool isRootSpan, StatusCode statusCode = StatusCode.Unset)
[InlineData(false, false, false, StatusCode.Error, "Error description")]
public void IntegrationTest(
bool useShortTraceIds,
bool useTestResource,
bool isRootSpan,
StatusCode statusCode = StatusCode.Unset,
string statusDescription = null,
bool addErrorTag = false)
{
var status = statusCode switch
{
@ -149,6 +157,11 @@ namespace OpenTelemetry.Exporter.Zipkin.Tests
_ => throw new InvalidOperationException(),
};
if (!string.IsNullOrEmpty(statusDescription))
{
status = status.WithDescription(statusDescription);
}
Guid requestId = Guid.NewGuid();
ZipkinExporter exporter = new ZipkinExporter(
@ -178,6 +191,11 @@ namespace OpenTelemetry.Exporter.Zipkin.Tests
exporter.SetLocalEndpointFromResource(Resource.Empty);
}
if (addErrorTag)
{
activity.SetTag(ZipkinActivityConversionExtensions.ZipkinErrorFlagTagName, "This should be removed.");
}
var processor = new SimpleExportProcessor<Activity>(exporter);
processor.OnEnd(activity);
@ -202,15 +220,26 @@ namespace OpenTelemetry.Exporter.Zipkin.Tests
var traceId = useShortTraceIds ? TraceId.Substring(TraceId.Length - 16, 16) : TraceId;
var statusTag = statusCode switch
string statusTag;
string errorTag = string.Empty;
switch (statusCode)
{
StatusCode.Ok => $@"""{SpanAttributeConstants.StatusCodeKey}"":""OK"",",
StatusCode.Error => $@"""error"":"""",""{SpanAttributeConstants.StatusCodeKey}"":""ERROR"",",
_ => string.Empty,
};
case StatusCode.Ok:
statusTag = $@"""{SpanAttributeConstants.StatusCodeKey}"":""OK"",";
break;
case StatusCode.Unset:
statusTag = string.Empty;
break;
case StatusCode.Error:
statusTag = $@"""{SpanAttributeConstants.StatusCodeKey}"":""ERROR"",";
errorTag = $@",""{ZipkinActivityConversionExtensions.ZipkinErrorFlagTagName}"":""{statusDescription}""";
break;
default:
throw new NotSupportedException();
}
Assert.Equal(
$@"[{{""traceId"":""{traceId}"",""name"":""Name"",{parentId}""id"":""{ZipkinActivityConversionExtensions.EncodeSpanId(context.SpanId)}"",""kind"":""CLIENT"",""timestamp"":{timestamp},""duration"":60000000,""localEndpoint"":{{""serviceName"":""{serviceName}""{ipInformation}}},""remoteEndpoint"":{{""serviceName"":""http://localhost:44312/""}},""annotations"":[{{""timestamp"":{eventTimestamp},""value"":""Event1""}},{{""timestamp"":{eventTimestamp},""value"":""Event2""}}],""tags"":{{{resoureTags}""stringKey"":""value"",""longKey"":""1"",""longKey2"":""1"",""doubleKey"":""1"",""doubleKey2"":""1"",""longArrayKey"":""1,2"",""boolKey"":""true"",""boolArrayKey"":""true,false"",""http.host"":""http://localhost:44312/"",{statusTag}""otel.library.name"":""CreateTestActivity"",""peer.service"":""http://localhost:44312/""}}}}]",
$@"[{{""traceId"":""{traceId}"",""name"":""Name"",{parentId}""id"":""{ZipkinActivityConversionExtensions.EncodeSpanId(context.SpanId)}"",""kind"":""CLIENT"",""timestamp"":{timestamp},""duration"":60000000,""localEndpoint"":{{""serviceName"":""{serviceName}""{ipInformation}}},""remoteEndpoint"":{{""serviceName"":""http://localhost:44312/""}},""annotations"":[{{""timestamp"":{eventTimestamp},""value"":""Event1""}},{{""timestamp"":{eventTimestamp},""value"":""Event2""}}],""tags"":{{{resoureTags}""stringKey"":""value"",""longKey"":""1"",""longKey2"":""1"",""doubleKey"":""1"",""doubleKey2"":""1"",""longArrayKey"":""1,2"",""boolKey"":""true"",""boolArrayKey"":""true,false"",""http.host"":""http://localhost:44312/"",{statusTag}""otel.library.name"":""CreateTestActivity"",""peer.service"":""http://localhost:44312/""{errorTag}}}}}]",
Responses[requestId]);
}

View File

@ -77,6 +77,23 @@ namespace OpenTelemetry.Trace.Tests
Assert.Null(status.Description);
}
[Fact]
public void SetStatusWithIgnoredDescription()
{
using var openTelemetrySdk = Sdk.CreateTracerProviderBuilder()
.AddSource(ActivitySourceName)
.Build();
using var source = new ActivitySource(ActivitySourceName);
using var activity = source.StartActivity(ActivityName);
activity.SetStatus(Status.Ok.WithDescription("This should be ignored."));
activity?.Stop();
var status = activity.GetStatus();
Assert.Equal(StatusCode.Ok, status.StatusCode);
Assert.Null(status.Description);
}
[Fact]
public void SetCancelledStatus()
{