diff --git a/src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md b/src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md index f19c9af2f9e..8021f670b29 100644 --- a/src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Jaeger/CHANGELOG.md @@ -8,6 +8,13 @@ * 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)) +* Jaeger will no longer send the `otel.status_code` tag if the value is `Unset`. + ([#1609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1609)) + +* Span Event.Name will now be populated as the `event` field on Jaeger Logs + instead of `message`. + ([#1609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1609)) + ## 1.0.0-rc1.1 Released 2020-Nov-17 diff --git a/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs b/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs index e78960f6a6b..cefb7355b01 100644 --- a/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs +++ b/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs @@ -162,10 +162,11 @@ public static JaegerLog ToJaegerLog(this ActivityEvent timedEvent) timedEvent.EnumerateTags(ref jaegerTags); - // Matches what OpenTracing and OpenTelemetry defines as the event name. - // https://github.com/opentracing/specification/blob/master/semantic_conventions.md#log-fields-table - // https://github.com/open-telemetry/opentelemetry-specification/pull/397/files - PooledList.Add(ref jaegerTags.Tags, new JaegerTag("message", JaegerTagType.STRING, vStr: timedEvent.Name)); + if (!jaegerTags.HasEvent) + { + // https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/jaeger.md#events + PooledList.Add(ref jaegerTags.Tags, new JaegerTag("event", JaegerTagType.STRING, vStr: timedEvent.Name)); + } // TODO: Use the same function as JaegerConversionExtensions or check that the perf here is acceptable. return new JaegerLog(timedEvent.Timestamp.ToEpochMicroseconds(), jaegerTags.Tags); @@ -256,9 +257,16 @@ private static void ProcessJaegerTag(ref TagEnumerationState state, string key, { PeerServiceResolver.InspectTag(ref state, key, jaegerTag.VStr); - if (key == SpanAttributeConstants.StatusCodeKey && jaegerTag.VStr == "Error") + if (key == SpanAttributeConstants.StatusCodeKey) { - PooledList.Add(ref state.Tags, new JaegerTag("error", JaegerTagType.BOOL, vBool: true)); + if (jaegerTag.VStr == "Error") + { + PooledList.Add(ref state.Tags, new JaegerTag("error", JaegerTagType.BOOL, vBool: true)); + } + else if (jaegerTag.VStr == "Unset") + { + return; + } } } else if (jaegerTag.VLong.HasValue) @@ -342,6 +350,8 @@ private struct EventTagsEnumerationState : IActivityEnumerator Tags; + public bool HasEvent; + public bool ForEach(KeyValuePair tag) { if (tag.Value is Array) @@ -353,6 +363,11 @@ public bool ForEach(KeyValuePair tag) PooledList.Add(ref this.Tags, tag.ToJaegerTag()); } + if (tag.Key == "event") + { + this.HasEvent = true; + } + return true; } } diff --git a/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md b/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md index 1c53505034e..2feca47b42c 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md @@ -5,6 +5,13 @@ * 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)) +* 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)) + +* Zipkin bool tag values will now be sent as `true`/`false` instead of + `True`/`False`. + ([#1609](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1609)) + ## 1.0.0-rc1.1 Released 2020-Nov-17 diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs index 3851418d1cd..f2173df08fe 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs @@ -175,9 +175,17 @@ public bool ForEach(KeyValuePair activityTag) { PeerServiceResolver.InspectTag(ref this, key, strVal); - if (key == SpanAttributeConstants.StatusCodeKey && strVal == "Error") + if (key == SpanAttributeConstants.StatusCodeKey) { - PooledList>.Add(ref this.Tags, new KeyValuePair("error", "true")); + if (strVal == "Error") + { + PooledList>.Add(ref this.Tags, new KeyValuePair("error", "true")); + } + else if (strVal == "Unset") + { + // Unset Status is not sent: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk_exporters/zipkin.md#status + return true; + } } } else if (activityTag.Value is int intVal && activityTag.Key == SemanticConventions.AttributeNetPeerPort) diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs index b7f0465edfe..a966e44e058 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs @@ -41,8 +41,8 @@ public ZipkinSpan( long? duration, ZipkinEndpoint localEndpoint, ZipkinEndpoint remoteEndpoint, - in PooledList? annotations, - in PooledList>? tags, + in PooledList annotations, + in PooledList> tags, bool? debug, bool? shared) { @@ -89,9 +89,9 @@ public ZipkinSpan( public ZipkinEndpoint RemoteEndpoint { get; } - public PooledList? Annotations { get; } + public PooledList Annotations { get; } - public PooledList>? Tags { get; } + public PooledList> Tags { get; } public bool? Debug { get; } @@ -99,8 +99,8 @@ public ZipkinSpan( public void Return() { - this.Annotations?.Return(); - this.Tags?.Return(); + this.Annotations.Return(); + this.Tags.Return(); } #if NET452 @@ -168,12 +168,12 @@ public void Write(JsonWriter writer) this.RemoteEndpoint.Write(writer); } - if (this.Annotations.HasValue) + if (!this.Annotations.IsEmpty) { writer.WritePropertyName("annotations"); writer.WriteStartArray(); - foreach (var annotation in this.Annotations.Value) + foreach (var annotation in this.Annotations) { writer.WriteStartObject(); @@ -189,7 +189,7 @@ public void Write(JsonWriter writer) writer.WriteEndArray(); } - if (this.Tags.HasValue || this.LocalEndpoint.Tags != null) + if (!this.Tags.IsEmpty || this.LocalEndpoint.Tags != null) { writer.WritePropertyName("tags"); writer.WriteStartObject(); @@ -203,13 +203,13 @@ public void Write(JsonWriter writer) foreach (var tag in this.LocalEndpoint.Tags ?? Enumerable.Empty>()) { writer.WritePropertyName(tag.Key); - writer.WriteValue(this.ConvertObjectToString(tag.Value)); + writer.WriteValue(ConvertObjectToString(tag.Value)); } - foreach (var tag in this.Tags ?? Enumerable.Empty>()) + foreach (var tag in this.Tags) { writer.WritePropertyName(tag.Key); - writer.WriteValue(this.ConvertObjectToString(tag.Value)); + writer.WriteValue(ConvertObjectToString(tag.Value)); } } finally @@ -278,12 +278,12 @@ public void Write(Utf8JsonWriter writer) this.RemoteEndpoint.Write(writer); } - if (this.Annotations.HasValue) + if (!this.Annotations.IsEmpty) { writer.WritePropertyName("annotations"); writer.WriteStartArray(); - foreach (var annotation in this.Annotations.Value) + foreach (var annotation in this.Annotations) { writer.WriteStartObject(); @@ -297,7 +297,7 @@ public void Write(Utf8JsonWriter writer) writer.WriteEndArray(); } - if (this.Tags.HasValue || this.LocalEndpoint.Tags != null) + if (!this.Tags.IsEmpty || this.LocalEndpoint.Tags != null) { writer.WritePropertyName("tags"); writer.WriteStartObject(); @@ -310,12 +310,12 @@ public void Write(Utf8JsonWriter writer) { foreach (var tag in this.LocalEndpoint.Tags ?? Enumerable.Empty>()) { - writer.WriteString(tag.Key, this.ConvertObjectToString(tag.Value)); + writer.WriteString(tag.Key, ConvertObjectToString(tag.Value)); } - foreach (var tag in this.Tags ?? Enumerable.Empty>()) + foreach (var tag in this.Tags) { - writer.WriteString(tag.Key, this.ConvertObjectToString(tag.Value)); + writer.WriteString(tag.Key, ConvertObjectToString(tag.Value)); } } finally @@ -332,17 +332,24 @@ public void Write(Utf8JsonWriter writer) #endif [MethodImpl(MethodImplOptions.AggressiveInlining)] - private string ConvertObjectToString(object obj) + private static string ConvertObjectToString(object obj) { return obj switch { string stringVal => stringVal, + bool boolVal => GetBoolString(boolVal), int[] arrayValue => string.Join(",", arrayValue), long[] arrayValue => string.Join(",", arrayValue), double[] arrayValue => string.Join(",", arrayValue), - bool[] arrayValue => string.Join(",", arrayValue), + bool[] arrayValue => string.Join(",", arrayValue.Select(GetBoolString)), _ => obj.ToString(), }; } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static string GetBoolString(bool value) + { + return value ? "true" : "false"; + } } } diff --git a/test/OpenTelemetry.Exporter.Jaeger.Tests/Implementation/JaegerActivityConversionTest.cs b/test/OpenTelemetry.Exporter.Jaeger.Tests/Implementation/JaegerActivityConversionTest.cs index 39f7b148a7f..52a87f0b217 100644 --- a/test/OpenTelemetry.Exporter.Jaeger.Tests/Implementation/JaegerActivityConversionTest.cs +++ b/test/OpenTelemetry.Exporter.Jaeger.Tests/Implementation/JaegerActivityConversionTest.cs @@ -116,7 +116,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_AllPropertie Assert.Equal("string_array", eventField.Key); Assert.Equal("b", eventField.VStr); eventField = eventFields[3]; - Assert.Equal("message", eventField.Key); + Assert.Equal("event", eventField.Key); Assert.Equal("Event1", eventField.VStr); Assert.Equal(activity.Events.First().Timestamp.ToEpochMicroseconds(), jaegerLog.Timestamp); @@ -128,7 +128,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_AllPropertie Assert.Equal("key", eventField.Key); Assert.Equal("value", eventField.VStr); eventField = eventFields[1]; - Assert.Equal("message", eventField.Key); + Assert.Equal("event", eventField.Key); Assert.Equal("Event2", eventField.VStr); } @@ -175,7 +175,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_NoAttributes Assert.Equal("key", eventField.Key); Assert.Equal("value", eventField.VStr); eventField = eventFields[3]; - Assert.Equal("message", eventField.Key); + Assert.Equal("event", eventField.Key); Assert.Equal("Event1", eventField.VStr); Assert.Equal(activity.Events.First().Timestamp.ToEpochMicroseconds(), jaegerLog.Timestamp); @@ -187,7 +187,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_NoAttributes Assert.Equal("key", eventField.Key); Assert.Equal("value", eventField.VStr); eventField = eventFields[1]; - Assert.Equal("message", eventField.Key); + Assert.Equal("event", eventField.Key); Assert.Equal("Event2", eventField.VStr); } @@ -340,7 +340,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_NoLinks() Assert.Equal("key", eventField.Key); Assert.Equal("value", eventField.VStr); eventField = eventFields[3]; - Assert.Equal("message", eventField.Key); + Assert.Equal("event", eventField.Key); Assert.Equal("Event1", eventField.VStr); Assert.Equal(activity.Events.First().Timestamp.ToEpochMicroseconds(), jaegerLog.Timestamp); @@ -351,7 +351,7 @@ public void JaegerActivityConverterTest_ConvertActivityToJaegerSpan_NoLinks() Assert.Equal("key", eventField.Key); Assert.Equal("value", eventField.VStr); eventField = eventFields[1]; - Assert.Equal("message", eventField.Key); + Assert.Equal("event", eventField.Key); Assert.Equal("Event2", eventField.VStr); } @@ -456,6 +456,18 @@ public void JaegerActivityConverterTest_Status_ErrorFlagTest(StatusCode statusCo var jaegerSpan = activity.ToJaegerSpan(); // Assert + + if (statusCode == StatusCode.Unset) + { + Assert.DoesNotContain(jaegerSpan.Tags, t => t.Key == SpanAttributeConstants.StatusCodeKey); + } + else + { + Assert.Equal( + StatusHelper.GetStringNameForStatusCode(statusCode), + jaegerSpan.Tags.FirstOrDefault(t => t.Key == SpanAttributeConstants.StatusCodeKey).VStr); + } + if (hasErrorFlag) { Assert.Contains(jaegerSpan.Tags, t => t.Key == "error" && t.VType == JaegerTagType.BOOL && (t.VBool ?? false)); diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs index 49fe736264c..fd2f20e01f3 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityConversionTest.cs @@ -17,6 +17,7 @@ using System; using System.Linq; using OpenTelemetry.Exporter.Zipkin.Implementation; +using OpenTelemetry.Internal; using OpenTelemetry.Trace; using Xunit; @@ -45,7 +46,7 @@ public void ToZipkinSpan_AllPropertiesSet() Assert.Equal((long)(activity.Duration.TotalMilliseconds * 1000), zipkinSpan.Duration); int counter = 0; - var tagsArray = zipkinSpan.Tags.Value.ToArray(); + var tagsArray = zipkinSpan.Tags.ToArray(); foreach (var tags in activity.TagObjects) { @@ -70,12 +71,12 @@ public void ToZipkinSpan_NoEvents() var zipkinSpan = activity.ToZipkinSpan(DefaultZipkinEndpoint); Assert.Equal(ZipkinSpanName, zipkinSpan.Name); - Assert.Empty(zipkinSpan.Annotations.Value); + Assert.Empty(zipkinSpan.Annotations); Assert.Equal(activity.TraceId.ToHexString(), zipkinSpan.TraceId); Assert.Equal(activity.SpanId.ToHexString(), zipkinSpan.Id); int counter = 0; - var tagsArray = zipkinSpan.Tags.Value.ToArray(); + var tagsArray = zipkinSpan.Tags.ToArray(); foreach (var tags in activity.TagObjects) { @@ -108,13 +109,25 @@ public void ToZipkinSpan_Status_ErrorFlagTest(StatusCode statusCode, bool hasErr var zipkinSpan = activity.ToZipkinSpan(DefaultZipkinEndpoint); // Assert + + if (statusCode == StatusCode.Unset) + { + Assert.DoesNotContain(zipkinSpan.Tags, t => t.Key == SpanAttributeConstants.StatusCodeKey); + } + else + { + Assert.Equal( + StatusHelper.GetStringNameForStatusCode(statusCode), + zipkinSpan.Tags.FirstOrDefault(t => t.Key == SpanAttributeConstants.StatusCodeKey).Value); + } + if (hasErrorFlag) { - Assert.Contains(zipkinSpan.Tags.Value, t => t.Key == "error" && (string)t.Value == "true"); + Assert.Contains(zipkinSpan.Tags, t => t.Key == "error" && (string)t.Value == "true"); } else { - Assert.DoesNotContain(zipkinSpan.Tags.Value, t => t.Key == "error"); + Assert.DoesNotContain(zipkinSpan.Tags, t => t.Key == "error"); } } } diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index 85b28276524..abd1c67bf6f 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -193,7 +193,7 @@ public void IntegrationTest(bool useShortTraceIds, bool useTestResource, bool is var traceId = useShortTraceIds ? TraceId.Substring(TraceId.Length - 16, 16) : TraceId; 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"",""http.host"":""http://localhost:44312/"",""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/"",""otel.library.name"":""CreateTestActivity"",""peer.service"":""http://localhost:44312/""}}}}]", Responses[requestId]); } @@ -223,6 +223,7 @@ internal static Activity CreateTestActivity( { "doubleKey2", 1F }, { "longArrayKey", new long[] { 1, 2 } }, { "boolKey", true }, + { "boolArrayKey", new bool[] { true, false } }, { "http.host", "http://localhost:44312/" }, // simulating instrumentation tag adding http.host }; if (additionalAttributes != null)