From 5be940eae71d37c356e35113e419bc48fab6e0c2 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 2 May 2024 23:03:43 -0700 Subject: [PATCH 1/4] Update ZipkinExporter to use TagWriter. --- .../Implementation/ZipkinSpan.cs | 28 +++--------- .../Implementation/ZipkinTagTransformer.cs | 37 --------------- .../Implementation/ZipkinTagWriter.cs | 45 +++++++++++++++++++ .../OpenTelemetry.Exporter.Zipkin.csproj | 3 +- .../ZipkinExporterTests.cs | 2 +- 5 files changed, 52 insertions(+), 63 deletions(-) delete mode 100644 src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagTransformer.cs create mode 100644 src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagWriter.cs diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs index e28fcda6a60..1e11eba503b 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs @@ -1,7 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -using System.Globalization; using System.Text.Json; using OpenTelemetry.Internal; @@ -153,31 +152,14 @@ public void Write(Utf8JsonWriter writer) writer.WritePropertyName(ZipkinSpanJsonHelper.TagsPropertyName); writer.WriteStartObject(); - // this will be used when we convert int, double, int[], double[] to string - var originalUICulture = Thread.CurrentThread.CurrentUICulture; - Thread.CurrentThread.CurrentUICulture = CultureInfo.InvariantCulture; - - try + foreach (var tag in this.LocalEndpoint.Tags ?? Enumerable.Empty>()) { - foreach (var tag in this.LocalEndpoint.Tags ?? Enumerable.Empty>()) - { - if (ZipkinTagTransformer.Instance.TryTransformTag(tag, out var result)) - { - writer.WriteString(tag.Key, result); - } - } - - foreach (var tag in this.Tags) - { - if (ZipkinTagTransformer.Instance.TryTransformTag(tag, out var result)) - { - writer.WriteString(tag.Key, result); - } - } + ZipkinTagWriter.Instance.TryWriteTag(ref writer, tag); } - finally + + foreach (var tag in this.Tags) { - Thread.CurrentThread.CurrentUICulture = originalUICulture; + ZipkinTagWriter.Instance.TryWriteTag(ref writer, tag); } writer.WriteEndObject(); diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagTransformer.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagTransformer.cs deleted file mode 100644 index ac8b686e5dc..00000000000 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagTransformer.cs +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#nullable enable - -using OpenTelemetry.Internal; - -namespace OpenTelemetry.Exporter.Zipkin.Implementation; - -internal sealed class ZipkinTagTransformer : TagTransformer -{ - private ZipkinTagTransformer() - { - } - - public static ZipkinTagTransformer Instance { get; } = new(); - - protected override string TransformIntegralTag(string key, long value) => value.ToString(); - - protected override string TransformFloatingPointTag(string key, double value) => value.ToString(); - - protected override string TransformBooleanTag(string key, bool value) => value ? "true" : "false"; - - protected override string TransformStringTag(string key, string value) => value; - - protected override string TransformArrayTag(string key, Array array) - => this.TransformStringTag(key, TagTransformerJsonHelper.JsonSerializeArrayTag(array)); - - protected override void OnUnsupportedTagDropped( - string tagKey, - string tagValueTypeFullName) - { - ZipkinExporterEventSource.Log.UnsupportedAttributeType( - tagValueTypeFullName, - tagKey); - } -} diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagWriter.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagWriter.cs new file mode 100644 index 00000000000..98dc3e0f246 --- /dev/null +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagWriter.cs @@ -0,0 +1,45 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#nullable enable + +using System.Text.Json; +using OpenTelemetry.Internal; + +namespace OpenTelemetry.Exporter.Zipkin.Implementation; + +internal sealed class ZipkinTagWriter : JsonStringArrayTagWriter +{ + private ZipkinTagWriter() + { + } + + public static ZipkinTagWriter Instance { get; } = new(); + + protected override void WriteIntegralTag(ref Utf8JsonWriter writer, string key, long value) + => writer.WriteNumber(key, value); + + protected override void WriteFloatingPointTag(ref Utf8JsonWriter writer, string key, double value) + => writer.WriteNumber(key, value); + + protected override void WriteBooleanTag(ref Utf8JsonWriter writer, string key, bool value) + => writer.WriteBoolean(key, value); + + protected override void WriteStringTag(ref Utf8JsonWriter writer, string key, string value) + => writer.WriteString(key, value); + + protected override void WriteArrayTag(ref Utf8JsonWriter writer, string key, ArraySegment arrayUtf8JsonBytes) + { + writer.WritePropertyName(key); + writer.WriteStringValue(arrayUtf8JsonBytes); + } + + protected override void OnUnsupportedTagDropped( + string tagKey, + string tagValueTypeFullName) + { + ZipkinExporterEventSource.Log.UnsupportedAttributeType( + tagValueTypeFullName, + tagKey); + } +} diff --git a/src/OpenTelemetry.Exporter.Zipkin/OpenTelemetry.Exporter.Zipkin.csproj b/src/OpenTelemetry.Exporter.Zipkin/OpenTelemetry.Exporter.Zipkin.csproj index d3418763073..3ca65ced4d0 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/OpenTelemetry.Exporter.Zipkin.csproj +++ b/src/OpenTelemetry.Exporter.Zipkin/OpenTelemetry.Exporter.Zipkin.csproj @@ -23,8 +23,7 @@ - - + diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index 308f2e6715a..28ce57f4d43 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -422,7 +422,7 @@ public void IntegrationTest( } 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"":{{{resourceTags}""stringKey"":""value"",""longKey"":""1"",""longKey2"":""1"",""doubleKey"":""1"",""doubleKey2"":""1"",""longArrayKey"":""[1,2]"",""boolKey"":""true"",""boolArrayKey"":""[true,false]"",""http.host"":""http://localhost:44312/"",{statusTag}{errorTag}""otel.scope.name"":""CreateTestActivity"",""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"":{{{resourceTags}""stringKey"":""value"",""longKey"":1,""longKey2"":1,""doubleKey"":1,""doubleKey2"":1,""longArrayKey"":""[1,2]"",""boolKey"":true,""boolArrayKey"":""[true,false]"",""http.host"":""http://localhost:44312/"",{statusTag}{errorTag}""otel.scope.name"":""CreateTestActivity"",""otel.library.name"":""CreateTestActivity"",""peer.service"":""http://localhost:44312/""}}}}]", Responses[requestId]); } From fb2959bcd248b552e1bc28b2d737dc8ab8ea97f9 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 2 May 2024 23:08:36 -0700 Subject: [PATCH 2/4] Update CHANGELOG. --- src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md b/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md index cf72bf0c33f..3898965cdf3 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +* Zipkin will now write numeric, boolean, and floating-point tag values as + primitive types in its emitted JSON instead of converting the values to + strings. + ([#5590](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5590)) + ## 1.8.1 Released 2024-Apr-17 From 7e455f0a2bf3ba63dd8b977a19d8407d2b525911 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 6 May 2024 15:08:03 -0700 Subject: [PATCH 3/4] Code review. --- .../CHANGELOG.md | 5 ---- .../Implementation/ZipkinSpan.cs | 24 +++++++++++---- .../Implementation/ZipkinTagWriter.cs | 30 +++++++++++++++++-- 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md b/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md index 3898965cdf3..cf72bf0c33f 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md @@ -2,11 +2,6 @@ ## Unreleased -* Zipkin will now write numeric, boolean, and floating-point tag values as - primitive types in its emitted JSON instead of converting the values to - strings. - ([#5590](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5590)) - ## 1.8.1 Released 2024-Apr-17 diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs index 1e11eba503b..8038492a0be 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs @@ -1,6 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +using System.Globalization; using System.Text.Json; using OpenTelemetry.Internal; @@ -152,14 +153,27 @@ public void Write(Utf8JsonWriter writer) writer.WritePropertyName(ZipkinSpanJsonHelper.TagsPropertyName); writer.WriteStartObject(); - foreach (var tag in this.LocalEndpoint.Tags ?? Enumerable.Empty>()) + // Note: The spec says "Primitive types MUST be converted to string using en-US culture settings" + // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk_exporters/zipkin.md#attribute + + var originalUICulture = Thread.CurrentThread.CurrentUICulture; + Thread.CurrentThread.CurrentUICulture = CultureInfo.InvariantCulture; + + try { - ZipkinTagWriter.Instance.TryWriteTag(ref writer, tag); + foreach (var tag in this.LocalEndpoint.Tags ?? Enumerable.Empty>()) + { + ZipkinTagWriter.Instance.TryWriteTag(ref writer, tag); + } + + foreach (var tag in this.Tags) + { + ZipkinTagWriter.Instance.TryWriteTag(ref writer, tag); + } } - - foreach (var tag in this.Tags) + finally { - ZipkinTagWriter.Instance.TryWriteTag(ref writer, tag); + Thread.CurrentThread.CurrentUICulture = originalUICulture; } writer.WriteEndObject(); diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagWriter.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagWriter.cs index 98dc3e0f246..c83b1f92003 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagWriter.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinTagWriter.cs @@ -3,6 +3,8 @@ #nullable enable +using System.Buffers.Text; +using System.Globalization; using System.Text.Json; using OpenTelemetry.Internal; @@ -10,6 +12,8 @@ namespace OpenTelemetry.Exporter.Zipkin.Implementation; internal sealed class ZipkinTagWriter : JsonStringArrayTagWriter { + public const int StackallocByteThreshold = 256; + private ZipkinTagWriter() { } @@ -17,13 +21,33 @@ private ZipkinTagWriter() public static ZipkinTagWriter Instance { get; } = new(); protected override void WriteIntegralTag(ref Utf8JsonWriter writer, string key, long value) - => writer.WriteNumber(key, value); + { + Span destination = stackalloc byte[StackallocByteThreshold]; + if (Utf8Formatter.TryFormat(value, destination, out int bytesWritten)) + { + writer.WriteString(key, destination.Slice(0, bytesWritten)); + } + else + { + writer.WriteString(key, value.ToString(CultureInfo.InvariantCulture)); + } + } protected override void WriteFloatingPointTag(ref Utf8JsonWriter writer, string key, double value) - => writer.WriteNumber(key, value); + { + Span destination = stackalloc byte[StackallocByteThreshold]; + if (Utf8Formatter.TryFormat(value, destination, out int bytesWritten)) + { + writer.WriteString(key, destination.Slice(0, bytesWritten)); + } + else + { + writer.WriteString(key, value.ToString(CultureInfo.InvariantCulture)); + } + } protected override void WriteBooleanTag(ref Utf8JsonWriter writer, string key, bool value) - => writer.WriteBoolean(key, value); + => writer.WriteString(key, value ? "true" : "false"); protected override void WriteStringTag(ref Utf8JsonWriter writer, string key, string value) => writer.WriteString(key, value); From 02bad894623945d88ed50e86ee73cc4058b4d798 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 6 May 2024 15:08:56 -0700 Subject: [PATCH 4/4] Code review. --- test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index 28ce57f4d43..308f2e6715a 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -422,7 +422,7 @@ public void IntegrationTest( } 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"":{{{resourceTags}""stringKey"":""value"",""longKey"":1,""longKey2"":1,""doubleKey"":1,""doubleKey2"":1,""longArrayKey"":""[1,2]"",""boolKey"":true,""boolArrayKey"":""[true,false]"",""http.host"":""http://localhost:44312/"",{statusTag}{errorTag}""otel.scope.name"":""CreateTestActivity"",""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"":{{{resourceTags}""stringKey"":""value"",""longKey"":""1"",""longKey2"":""1"",""doubleKey"":""1"",""doubleKey2"":""1"",""longArrayKey"":""[1,2]"",""boolKey"":""true"",""boolArrayKey"":""[true,false]"",""http.host"":""http://localhost:44312/"",{statusTag}{errorTag}""otel.scope.name"":""CreateTestActivity"",""otel.library.name"":""CreateTestActivity"",""peer.service"":""http://localhost:44312/""}}}}]", Responses[requestId]); }