From d02307818f4f5e23d545500dd12275e65ba9e3e8 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Fri, 6 May 2022 18:35:41 -0700 Subject: [PATCH 01/16] Expand supported attribute types --- .../Implementation/ActivityExtensions.cs | 35 ---- .../Implementation/OtlpCommonExtensions.cs | 116 +++++++++++ .../OtlpAttributeTests.cs | 193 ++++++++++++++++++ 3 files changed, 309 insertions(+), 35 deletions(-) create mode 100644 src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs create mode 100644 test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs index fbc123db6b7..e6fe5027e7b 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs @@ -201,41 +201,6 @@ internal static OtlpTrace.Span ToOtlpSpan(this Activity activity) return otlpSpan; } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair kvp) - { - if (kvp.Value == null) - { - return null; - } - - var attrib = new OtlpCommon.KeyValue { Key = kvp.Key, Value = new OtlpCommon.AnyValue { } }; - - switch (kvp.Value) - { - case string s: - attrib.Value.StringValue = s; - break; - case bool b: - attrib.Value.BoolValue = b; - break; - case int i: - attrib.Value.IntValue = i; - break; - case long l: - attrib.Value.IntValue = l; - break; - case double d: - attrib.Value.DoubleValue = d; - break; - default: - attrib.Value.StringValue = kvp.Value.ToString(); - break; - } - - return attrib; - } - [MethodImpl(MethodImplOptions.AggressiveInlining)] private static OtlpTrace.Status ToOtlpStatus(this Activity activity, ref TagEnumerationState otlpTags) { diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs new file mode 100644 index 00000000000..4e28622c1bf --- /dev/null +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs @@ -0,0 +1,116 @@ +// +// 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. +// + +using System; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using Google.Protobuf.Collections; +using OtlpCommon = Opentelemetry.Proto.Common.V1; + +namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation +{ + internal static class OtlpCommonExtensions + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair kvp) + { + if (kvp.Value == null) + { + return null; + } + + var attrib = new OtlpCommon.KeyValue { Key = kvp.Key, Value = new OtlpCommon.AnyValue { } }; + + switch (kvp.Value) + { + case char: + case string: + attrib.Value.StringValue = Convert.ToString(kvp.Value); + break; + case bool b: + attrib.Value.BoolValue = b; + break; + case byte: + case sbyte: + case short: + case ushort: + case int: + case uint: + case long: + attrib.Value.IntValue = Convert.ToInt64(kvp.Value); + break; + case float: + case double: + attrib.Value.DoubleValue = Convert.ToDouble(kvp.Value); + break; + case Array array: + var arrayValue = attrib.Value.ArrayValue = new OtlpCommon.ArrayValue(); + switch (kvp.Value) + { + case char[]: + case string[]: + foreach (var item in array) + { + arrayValue.Values.Add(new OtlpCommon.AnyValue { StringValue = Convert.ToString(item) }); + } + + break; + case bool[]: + foreach (var item in array) + { + arrayValue.Values.Add(new OtlpCommon.AnyValue { BoolValue = Convert.ToBoolean(item) }); + } + + break; + case byte[]: + case sbyte[]: + case short[]: + case ushort[]: + case int[]: + case uint[]: + case long[]: + foreach (var item in array) + { + arrayValue.Values.Add(new OtlpCommon.AnyValue { IntValue = Convert.ToInt64(item) }); + } + + break; + case float[]: + case double[]: + foreach (var item in array) + { + arrayValue.Values.Add(new OtlpCommon.AnyValue { DoubleValue = Convert.ToDouble(item) }); + } + + break; + default: + return null; + } + + break; + + // case nint: Pointer type. + // case nuint: Pointer type. + // case ulong: May throw an exception on overflow. + // case decimal: Converting to double produces rounding errors. + default: + return null; + } + + return attrib; + } + } +} diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs new file mode 100644 index 00000000000..b8ca97724db --- /dev/null +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs @@ -0,0 +1,193 @@ +// +// 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. +// + +using System; +using System.Collections.Generic; +using System.Linq; +using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation; +using Xunit; +using OtlpCommon = Opentelemetry.Proto.Common.V1; + +namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests +{ + public class OtlpAttributeTests + { + [Theory] + [InlineData(sbyte.MaxValue)] + [InlineData(byte.MaxValue)] + [InlineData(short.MaxValue)] + [InlineData(ushort.MaxValue)] + [InlineData(int.MaxValue)] + [InlineData(uint.MaxValue)] + [InlineData(long.MaxValue)] + [InlineData(new sbyte[] { 1, 2, 3 })] + [InlineData(new byte[] { 1, 2, 3 })] + [InlineData(new short[] { 1, 2, 3 })] + [InlineData(new ushort[] { 1, 2, 3 })] + [InlineData(new int[] { 1, 2, 3 })] + [InlineData(new uint[] { 1, 2, 3 })] + [InlineData(new long[] { 1, 2, 3 })] + public void IntegralTypesSupported(object value) + { + var kvp = new KeyValuePair("key", value); + var attribute = kvp.ToOtlpAttribute(); + Assert.NotNull(attribute); + + switch (value) + { + case Array array: + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.ArrayValue, attribute.Value.ValueCase); + var expectedArray = new long[array.Length]; + for (var i = 0; i < array.Length; i++) + { + expectedArray[i] = Convert.ToInt64(array.GetValue(i)); + } + + Assert.Equal(expectedArray, attribute.Value.ArrayValue.Values.Select(x => x.IntValue)); + break; + default: + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.IntValue, attribute.Value.ValueCase); + Assert.Equal(Convert.ToInt64(value), attribute.Value.IntValue); + break; + } + } + + [Theory] + [InlineData(float.MaxValue)] + [InlineData(double.MaxValue)] + [InlineData(new float[] { 1, 2, 3 })] + [InlineData(new double[] { 1, 2, 3 })] + public void FloatingPointTypesSupported(object value) + { + var kvp = new KeyValuePair("key", value); + var attribute = kvp.ToOtlpAttribute(); + Assert.NotNull(attribute); + + switch (value) + { + case Array array: + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.ArrayValue, attribute.Value.ValueCase); + var expectedArray = new double[array.Length]; + for (var i = 0; i < array.Length; i++) + { + expectedArray[i] = Convert.ToDouble(array.GetValue(i)); + } + + Assert.Equal(expectedArray, attribute.Value.ArrayValue.Values.Select(x => x.DoubleValue)); + break; + default: + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.DoubleValue, attribute.Value.ValueCase); + Assert.Equal(Convert.ToDouble(value), attribute.Value.DoubleValue); + break; + } + } + + [Theory] + [InlineData(true)] + [InlineData(new bool[] { true, false, true })] + public void BooleanTypeSupported(object value) + { + var kvp = new KeyValuePair("key", value); + var attribute = kvp.ToOtlpAttribute(); + Assert.NotNull(attribute); + + switch (value) + { + case Array array: + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.ArrayValue, attribute.Value.ValueCase); + var expectedArray = new bool[array.Length]; + for (var i = 0; i < array.Length; i++) + { + expectedArray[i] = Convert.ToBoolean(array.GetValue(i)); + } + + Assert.Equal(expectedArray, attribute.Value.ArrayValue.Values.Select(x => x.BoolValue)); + break; + default: + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.BoolValue, attribute.Value.ValueCase); + Assert.Equal(Convert.ToBoolean(value), attribute.Value.BoolValue); + break; + } + } + + [Theory] + [InlineData(char.MaxValue)] + [InlineData("string")] + public void StringTypesSupported(object value) + { + var kvp = new KeyValuePair("key", value); + var attribute = kvp.ToOtlpAttribute(); + Assert.NotNull(attribute); + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.StringValue, attribute.Value.ValueCase); + Assert.Equal(Convert.ToString(value), attribute.Value.StringValue); + } + + [Fact] + public void StringArrayTypesSupported() + { + var charArray = new char[] { 'a', 'b', 'c' }; + var stringArray = new string[] { "a", "b", "c" }; + + var kvp = new KeyValuePair("key", charArray); + var attribute = kvp.ToOtlpAttribute(); + Assert.NotNull(attribute); + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.ArrayValue, attribute.Value.ValueCase); + Assert.Equal(stringArray, attribute.Value.ArrayValue.Values.Select(x => x.StringValue)); + + kvp = new KeyValuePair("key", stringArray); + attribute = kvp.ToOtlpAttribute(); + Assert.NotNull(attribute); + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.ArrayValue, attribute.Value.ValueCase); + Assert.Equal(stringArray, attribute.Value.ArrayValue.Values.Select(x => x.StringValue)); + } + + [Fact] + public void UnsupportedTypes() + { + var kvp = new KeyValuePair("key", (nint)int.MaxValue); + var attribute = kvp.ToOtlpAttribute(); + Assert.Null(attribute); + + kvp = new KeyValuePair("key", (nuint)uint.MaxValue); + attribute = kvp.ToOtlpAttribute(); + Assert.Null(attribute); + + kvp = new KeyValuePair("key", decimal.MaxValue); + attribute = kvp.ToOtlpAttribute(); + Assert.Null(attribute); + + kvp = new KeyValuePair("key", new object()); + attribute = kvp.ToOtlpAttribute(); + Assert.Null(attribute); + + kvp = new KeyValuePair("key", new nint[] { 1, 2, 3 }); + attribute = kvp.ToOtlpAttribute(); + Assert.Null(attribute); + + kvp = new KeyValuePair("key", new nuint[] { 1, 2, 3 }); + attribute = kvp.ToOtlpAttribute(); + Assert.Null(attribute); + + kvp = new KeyValuePair("key", new decimal[] { 1, 2, 3 }); + attribute = kvp.ToOtlpAttribute(); + Assert.Null(attribute); + + kvp = new KeyValuePair("key", new object[] { new object(), new object(), new object() }); + attribute = kvp.ToOtlpAttribute(); + Assert.Null(attribute); + } + } +} From 73565d25c5cf9ec8ab36146ae3598d9a778be2be Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Mon, 9 May 2022 13:28:25 -0700 Subject: [PATCH 02/16] Suppress code style warning --- .../Implementation/OtlpCommonExtensions.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs index 4e28622c1bf..90818871f9b 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs @@ -58,6 +58,7 @@ public static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair Date: Mon, 9 May 2022 13:57:38 -0700 Subject: [PATCH 03/16] Update changelog --- .../CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md index e88e696600e..43afe043857 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md @@ -18,6 +18,22 @@ option * Fix handling of array-valued attributes for the OTLP trace exporter. ([#3238](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3238)) +* Standardize on the set of data types supported for attribute values on + resources, metrics, and logs. The list of data types that must be supported + per the + [OpenTelemetry specification](https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/common#attribute) + is more narrow than what the .NET OpenTelemetry SDK supports. Supported data + types includes strings and most + [built-in value types](https://docs.microsoft.com/dotnet/csharp/language-reference/builtin-types/built-in-types) + with a few exceptions where overflow or rounding may occur (`ulong` and + `decimal`). Additionally, arrays of these data types are supported. + *Breaking Change*: Previously, the OTLP exporter would `ToString()` any + `object`-valued attribute. This was a bug. In many circumstances `ToString()` + on an arbitrary type is not meaningful or worse could cause unintended, hard + to diagnose problems. Any type that is not explicitly supported is now + dropped. + ([#3262](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3262)) + ## 1.3.0-beta.1 Released 2022-Apr-15 From 6d556e6fe32331b7571c72c7f0ab574ba3ac5e2d Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Mon, 9 May 2022 14:05:27 -0700 Subject: [PATCH 04/16] Unused using --- .../Implementation/OtlpCommonExtensions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs index 90818871f9b..216a0d0e9bd 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs @@ -17,7 +17,6 @@ using System; using System.Collections.Generic; using System.Runtime.CompilerServices; -using Google.Protobuf.Collections; using OtlpCommon = Opentelemetry.Proto.Common.V1; namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation From 22d82cdac2341e0048fc61d8a11d29a179e9f0f9 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Mon, 9 May 2022 14:09:41 -0700 Subject: [PATCH 05/16] Update changelog --- src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md index 43afe043857..7503b5b13c9 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md @@ -27,7 +27,9 @@ option [built-in value types](https://docs.microsoft.com/dotnet/csharp/language-reference/builtin-types/built-in-types) with a few exceptions where overflow or rounding may occur (`ulong` and `decimal`). Additionally, arrays of these data types are supported. - *Breaking Change*: Previously, the OTLP exporter would `ToString()` any + ([#3262](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3262)) + +* **Breaking Change**: Previously, the OTLP exporter would `ToString()` any `object`-valued attribute. This was a bug. In many circumstances `ToString()` on an arbitrary type is not meaningful or worse could cause unintended, hard to diagnose problems. Any type that is not explicitly supported is now From ca49c93125f2b5f4f71489499b7da159ded80784 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Mon, 9 May 2022 14:50:46 -0700 Subject: [PATCH 06/16] Fix handling of nint for netframework and netstandard --- .../Implementation/OtlpCommonExtensions.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs index 216a0d0e9bd..4275f765cc2 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs @@ -82,6 +82,12 @@ public static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair Date: Tue, 10 May 2022 13:05:40 -0700 Subject: [PATCH 07/16] Fix nint/nuint issues --- .../Implementation/OtlpCommonExtensions.cs | 118 ++++++++---------- 1 file changed, 54 insertions(+), 64 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs index 4275f765cc2..50b76b79131 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs @@ -31,17 +31,25 @@ public static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair Date: Tue, 10 May 2022 13:29:58 -0700 Subject: [PATCH 08/16] Null check only necessary for netframework and netstandard builds --- .../Implementation/OtlpCommonExtensions.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs index 50b76b79131..c0ca2de81fc 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs @@ -94,11 +94,13 @@ private static OtlpCommon.AnyValue ToOtlpArrayValue(Array array) { var value = ToOtlpValue(item); +#if NETFRAMEWORK || NETSTANDARD // nint[] and nuint[] falls through to this case and ToOtlpValue will return null if (value == null) { return null; } +#endif arrayValue.Values.Add(value); } From 3bafc05c151c74f2684b5f05b5ba9e745e5b58dc Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 10 May 2022 13:51:16 -0700 Subject: [PATCH 09/16] Small refactor --- .../Implementation/OtlpCommonExtensions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs index c0ca2de81fc..a047cbbfe49 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs @@ -31,9 +31,7 @@ public static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair Date: Tue, 10 May 2022 14:18:11 -0700 Subject: [PATCH 10/16] Log when attribute type is not supported --- .../OpenTelemetryProtocolExporterEventSource.cs | 6 ++++++ .../Implementation/OtlpCommonExtensions.cs | 10 +++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OpenTelemetryProtocolExporterEventSource.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OpenTelemetryProtocolExporterEventSource.cs index 17db6324a1b..fb71c5cde8b 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OpenTelemetryProtocolExporterEventSource.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OpenTelemetryProtocolExporterEventSource.cs @@ -79,5 +79,11 @@ public void CouldNotTranslateLogRecord(string exceptionMessage) { this.WriteEvent(9, exceptionMessage); } + + [Event(10, Message = "Unsupported attribute type '{0}' for '{1}'. Attribute will not be exported.", Level = EventLevel.Warning)] + public void UnsupportedAttributeType(Type type, string key) + { + this.WriteEvent(10, type.ToString(), key); + } } } diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs index a047cbbfe49..f8f2d9aac5a 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs @@ -33,9 +33,13 @@ public static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair Date: Tue, 10 May 2022 14:31:45 -0700 Subject: [PATCH 11/16] Add tests for null value and empty array attributes --- .../OtlpAttributeTests.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs index b8ca97724db..b7d7ae7e910 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs @@ -25,6 +25,20 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests { public class OtlpAttributeTests { + [Fact] + public void EmptyArrays() + { + var kvp = new KeyValuePair("key", new int[] { }); + var attribute = kvp.ToOtlpAttribute(); + Assert.NotNull(attribute); + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.ArrayValue, attribute.Value.ValueCase); + Assert.Empty(attribute.Value.ArrayValue.Values); + + kvp = new KeyValuePair("key", new object[] { }); + attribute = kvp.ToOtlpAttribute(); + Assert.Null(attribute); + } + [Theory] [InlineData(sbyte.MaxValue)] [InlineData(byte.MaxValue)] From 8e68dd749b0474b7645112c893ab2f2e0c85875a Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 10 May 2022 16:09:44 -0700 Subject: [PATCH 12/16] Fix log test --- .../Implementation/OpenTelemetryProtocolExporterEventSource.cs | 2 +- .../Implementation/OtlpCommonExtensions.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OpenTelemetryProtocolExporterEventSource.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OpenTelemetryProtocolExporterEventSource.cs index fb71c5cde8b..9e3c905d748 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OpenTelemetryProtocolExporterEventSource.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OpenTelemetryProtocolExporterEventSource.cs @@ -81,7 +81,7 @@ public void CouldNotTranslateLogRecord(string exceptionMessage) } [Event(10, Message = "Unsupported attribute type '{0}' for '{1}'. Attribute will not be exported.", Level = EventLevel.Warning)] - public void UnsupportedAttributeType(Type type, string key) + public void UnsupportedAttributeType(string type, string key) { this.WriteEvent(10, type.ToString(), key); } diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs index f8f2d9aac5a..eb718fb3d65 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs @@ -35,7 +35,7 @@ public static OtlpCommon.KeyValue ToOtlpAttribute(this KeyValuePair Date: Tue, 10 May 2022 16:36:25 -0700 Subject: [PATCH 13/16] Null attribute value test --- .../OtlpAttributeTests.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs index b7d7ae7e910..bf777da311c 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs @@ -25,6 +25,14 @@ namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests { public class OtlpAttributeTests { + [Fact] + public void NullValueAttribute() + { + var kvp = new KeyValuePair("key", null); + var attribute = kvp.ToOtlpAttribute(); + Assert.Null(attribute); + } + [Fact] public void EmptyArrays() { From 330ab738cc0c09e0d51641231c636808baaaf1d3 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 10 May 2022 17:31:06 -0700 Subject: [PATCH 14/16] ToString all other types --- .../Implementation/OtlpCommonExtensions.cs | 32 +++++--- .../OtlpAttributeTests.cs | 78 +++++++++++++------ 2 files changed, 76 insertions(+), 34 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs index eb718fb3d65..b06b66d38c8 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs @@ -66,12 +66,21 @@ private static OtlpCommon.AnyValue ToOtlpValue(object value) case Array array: return ToOtlpArrayValue(array); + // All other types are converted to strings including the following + // built-in value types: // case nint: Pointer type. // case nuint: Pointer type. // case ulong: May throw an exception on overflow. // case decimal: Converting to double produces rounding errors. default: - return null; + try + { + return new OtlpCommon.AnyValue { StringValue = value.ToString() }; + } + catch + { + return null; + } } } @@ -79,6 +88,7 @@ private static OtlpCommon.AnyValue ToOtlpValue(object value) private static OtlpCommon.AnyValue ToOtlpArrayValue(Array array) { #pragma warning disable SA1011 // Closing square brackets should be spaced correctly + var arrayValue = new OtlpCommon.ArrayValue(); switch (array) { case char[]: @@ -93,25 +103,27 @@ private static OtlpCommon.AnyValue ToOtlpArrayValue(Array array) case long[]: case float[]: case double[]: - var arrayValue = new OtlpCommon.ArrayValue(); foreach (var item in array) { var value = ToOtlpValue(item); + arrayValue.Values.Add(value); + } -#if NETFRAMEWORK || NETSTANDARD - // nint[] and nuint[] falls through to this case and ToOtlpValue will return null - if (value == null) + return new OtlpCommon.AnyValue { ArrayValue = arrayValue }; + default: + foreach (var item in array) + { + try + { + arrayValue.Values.Add(ToOtlpValue(item.ToString())); + } + catch { return null; } -#endif - - arrayValue.Values.Add(value); } return new OtlpCommon.AnyValue { ArrayValue = arrayValue }; - default: - return null; } #pragma warning restore SA1011 // Closing square brackets should be spaced correctly } diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs index bf777da311c..dac7054690d 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpAttributeTests.cs @@ -44,7 +44,9 @@ public void EmptyArrays() kvp = new KeyValuePair("key", new object[] { }); attribute = kvp.ToOtlpAttribute(); - Assert.Null(attribute); + Assert.NotNull(attribute); + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.ArrayValue, attribute.Value.ValueCase); + Assert.Empty(attribute.Value.ArrayValue.Values); } [Theory] @@ -177,39 +179,67 @@ public void StringArrayTypesSupported() } [Fact] - public void UnsupportedTypes() + public void ToStringIsCalledForAllOtherTypes() { - var kvp = new KeyValuePair("key", (nint)int.MaxValue); - var attribute = kvp.ToOtlpAttribute(); - Assert.Null(attribute); - - kvp = new KeyValuePair("key", (nuint)uint.MaxValue); - attribute = kvp.ToOtlpAttribute(); - Assert.Null(attribute); + var testValues = new object[] + { + (nint)int.MaxValue, + (nuint)uint.MaxValue, + decimal.MaxValue, + new object(), + }; - kvp = new KeyValuePair("key", decimal.MaxValue); - attribute = kvp.ToOtlpAttribute(); - Assert.Null(attribute); + var testArrayValues = new object[] + { + new nint[] { 1, 2, 3 }, + new nuint[] { 1, 2, 3 }, + new decimal[] { 1, 2, 3 }, + new object[] { 1, new object(), false }, + }; - kvp = new KeyValuePair("key", new object()); - attribute = kvp.ToOtlpAttribute(); - Assert.Null(attribute); + foreach (var value in testValues) + { + var kvp = new KeyValuePair("key", value); + var attribute = kvp.ToOtlpAttribute(); + Assert.NotNull(attribute); + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.StringValue, attribute.Value.ValueCase); + Assert.Equal(value.ToString(), attribute.Value.StringValue); + } - kvp = new KeyValuePair("key", new nint[] { 1, 2, 3 }); - attribute = kvp.ToOtlpAttribute(); - Assert.Null(attribute); + foreach (var value in testArrayValues) + { + var kvp = new KeyValuePair("key", value); + var attribute = kvp.ToOtlpAttribute(); + Assert.NotNull(attribute); + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.ArrayValue, attribute.Value.ValueCase); + + var array = value as Array; + for (var i = 0; i < attribute.Value.ArrayValue.Values.Count; ++i) + { + Assert.Equal(OtlpCommon.AnyValue.ValueOneofCase.StringValue, attribute.Value.ArrayValue.Values[i].ValueCase); + Assert.Equal(array.GetValue(i).ToString(), attribute.Value.ArrayValue.Values[i].StringValue); + } + } + } - kvp = new KeyValuePair("key", new nuint[] { 1, 2, 3 }); - attribute = kvp.ToOtlpAttribute(); + [Fact] + public void ExceptionInToStringIsCaught() + { + var kvp = new KeyValuePair("key", new MyToStringMethodThrowsAnException()); + var attribute = kvp.ToOtlpAttribute(); Assert.Null(attribute); - kvp = new KeyValuePair("key", new decimal[] { 1, 2, 3 }); + kvp = new KeyValuePair("key", new object[] { 1, false, new MyToStringMethodThrowsAnException() }); attribute = kvp.ToOtlpAttribute(); Assert.Null(attribute); + } - kvp = new KeyValuePair("key", new object[] { new object(), new object(), new object() }); - attribute = kvp.ToOtlpAttribute(); - Assert.Null(attribute); + private class MyToStringMethodThrowsAnException + { + public override string ToString() + { + throw new Exception("Nope."); + } } } } From 1b18cffad544c85e62b2ecc696fb1cc953d7b505 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 10 May 2022 17:43:17 -0700 Subject: [PATCH 15/16] Update changelog --- .../CHANGELOG.md | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md index 7503b5b13c9..a83fc015f9b 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md @@ -18,22 +18,17 @@ option * Fix handling of array-valued attributes for the OTLP trace exporter. ([#3238](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3238)) -* Standardize on the set of data types supported for attribute values on - resources, metrics, and logs. The list of data types that must be supported - per the +* Improve the conversion and formatting of attribute values to the OTLP format + for resources, metrics, and logs. The list of data types that must be + supported per the [OpenTelemetry specification](https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/common#attribute) - is more narrow than what the .NET OpenTelemetry SDK supports. Supported data - types includes strings and most + is more narrow than what the .NET OpenTelemetry SDK supports. Numeric [built-in value types](https://docs.microsoft.com/dotnet/csharp/language-reference/builtin-types/built-in-types) - with a few exceptions where overflow or rounding may occur (`ulong` and - `decimal`). Additionally, arrays of these data types are supported. - ([#3262](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3262)) - -* **Breaking Change**: Previously, the OTLP exporter would `ToString()` any - `object`-valued attribute. This was a bug. In many circumstances `ToString()` - on an arbitrary type is not meaningful or worse could cause unintended, hard - to diagnose problems. Any type that is not explicitly supported is now - dropped. + are supported by converting to a `long` or `double` as appropriate except for + numeric types that could cause overflow (`ulong`) or rounding (`decimal`) + which are converted to strings. Non-numeric built-in types - `string`, + `char`, `bool` are supported. All other types are converted to a `string`. + Array values are also supported. ([#3262](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3262)) ## 1.3.0-beta.1 From c7893deca6865fa02810b4bc47d96dd164a94cf8 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 10 May 2022 17:50:05 -0700 Subject: [PATCH 16/16] Minor refactor --- .../Implementation/OtlpCommonExtensions.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs index b06b66d38c8..4185969a483 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCommonExtensions.cs @@ -105,8 +105,7 @@ private static OtlpCommon.AnyValue ToOtlpArrayValue(Array array) case double[]: foreach (var item in array) { - var value = ToOtlpValue(item); - arrayValue.Values.Add(value); + arrayValue.Values.Add(ToOtlpValue(item)); } return new OtlpCommon.AnyValue { ArrayValue = arrayValue };