From 53a579f507d2135767e80030d86d78218642f4ee Mon Sep 17 00:00:00 2001 From: Eddy Nakamura Date: Tue, 4 Aug 2020 21:10:29 -0300 Subject: [PATCH] Updating exporters to use TagObjects instead of Tags (#1000) * Update ZipkinExporter to use TagObjects instead of Tags * Updating JaegerExporter to use TagObjects * Updating OtlpExporter to use TagObjects * removing unused using * Removing duplicated logic Co-authored-by: Cijo Thomas --- .../JaegerActivityExtensions.cs | 39 +++++++---------- .../Implementation/ActivityExtensions.cs | 35 +-------------- .../ZipkinActivityConversionExtensions.cs | 43 ++++++++----------- .../Implementation/ZipkinSpan.cs | 27 ++++++++++-- .../OtlpExporterTest.cs | 2 +- 5 files changed, 61 insertions(+), 85 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs b/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs index 8fded0e64cb..849d606d53b 100644 --- a/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs +++ b/src/OpenTelemetry.Exporter.Jaeger/Implementation/JaegerActivityExtensions.cs @@ -48,11 +48,11 @@ internal static class JaegerActivityExtensions [SemanticConventions.AttributeNetPeerIp] = 2, ["peer.hostname"] = 2, ["peer.address"] = 2, - ["http.host"] = 3, // peer.service for Http. - ["db.instance"] = 3, // peer.service for Redis. + [SemanticConventions.AttributeHttpHost] = 3, // peer.service for Http. + [SemanticConventions.AttributeDbInstance] = 3, // peer.service for Redis. }; - private static readonly DictionaryEnumerator.ForEachDelegate ProcessActivityTagRef = ProcessActivityTag; + private static readonly DictionaryEnumerator.ForEachDelegate ProcessActivityTagRef = ProcessActivityTag; private static readonly ListEnumerator>.ForEachDelegate ProcessActivityLinkRef = ProcessActivityLink; private static readonly ListEnumerator>.ForEachDelegate ProcessActivityEventRef = ProcessActivityEvent; private static readonly DictionaryEnumerator>.ForEachDelegate ProcessTagRef = ProcessTag; @@ -64,8 +64,8 @@ public static JaegerSpan ToJaegerSpan(this Activity activity) Tags = PooledList.Create(), }; - DictionaryEnumerator.AllocationFreeForEach( - activity.Tags, + DictionaryEnumerator.AllocationFreeForEach( + activity.TagObjects, ref jaegerTags, ProcessActivityTagRef); @@ -217,23 +217,16 @@ public static JaegerSpanRef ToJaegerSpanRef(this in ActivityLink link) public static JaegerTag ToJaegerTag(this KeyValuePair attribute) { - switch (attribute.Value) + return attribute.Value switch { - case string s: - return new JaegerTag(attribute.Key, JaegerTagType.STRING, vStr: s); - case int i: - return new JaegerTag(attribute.Key, JaegerTagType.LONG, vLong: Convert.ToInt64(i)); - case long l: - return new JaegerTag(attribute.Key, JaegerTagType.LONG, vLong: l); - case float f: - return new JaegerTag(attribute.Key, JaegerTagType.DOUBLE, vDouble: Convert.ToDouble(f)); - case double d: - return new JaegerTag(attribute.Key, JaegerTagType.DOUBLE, vDouble: d); - case bool b: - return new JaegerTag(attribute.Key, JaegerTagType.BOOL, vBool: b); - } - - return new JaegerTag(attribute.Key, JaegerTagType.STRING, vStr: attribute.Value.ToString()); + string s => new JaegerTag(attribute.Key, JaegerTagType.STRING, vStr: s), + int i => new JaegerTag(attribute.Key, JaegerTagType.LONG, vLong: Convert.ToInt64(i)), + long l => new JaegerTag(attribute.Key, JaegerTagType.LONG, vLong: l), + float f => new JaegerTag(attribute.Key, JaegerTagType.DOUBLE, vDouble: Convert.ToDouble(f)), + double d => new JaegerTag(attribute.Key, JaegerTagType.DOUBLE, vDouble: d), + bool b => new JaegerTag(attribute.Key, JaegerTagType.BOOL, vBool: b), + _ => new JaegerTag(attribute.Key, JaegerTagType.STRING, vStr: attribute.Value.ToString()), + }; } public static long ToEpochMicroseconds(this DateTime utcDateTime) @@ -256,9 +249,9 @@ public static long ToEpochMicroseconds(this DateTimeOffset timestamp) return microseconds - UnixEpochMicroseconds; } - private static bool ProcessActivityTag(ref TagState state, KeyValuePair activityTag) + private static bool ProcessActivityTag(ref TagState state, KeyValuePair activityTag) { - var jaegerTag = new JaegerTag(activityTag.Key, JaegerTagType.STRING, activityTag.Value); + JaegerTag jaegerTag = activityTag.ToJaegerTag(); if (jaegerTag.VStr != null && PeerServiceKeyResolutionDictionary.TryGetValue(activityTag.Key, out int priority) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs index 0706fc66a32..190ff93fcb4 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs @@ -112,7 +112,7 @@ internal static OtlpTrace.Span ToOtlpSpan(this Activity activity) EndTimeUnixNano = (ulong)(startTimeUnixNano + activity.Duration.ToNanoseconds()), }; - foreach (var kvp in activity.Tags) + foreach (var kvp in activity.TagObjects) { var attribute = ToOtlpAttribute(kvp); if (attribute != null && attribute.Key != SpanAttributeConstants.StatusCodeKey && attribute.Key != SpanAttributeConstants.StatusDescriptionKey) @@ -219,39 +219,6 @@ private static OtlpTrace.Span.Types.Event ToOtlpEvent(ActivityEvent activityEven return otlpEvent; } - private static OtlpCommon.KeyValue ToOtlpAttribute(KeyValuePair kvp) - { - // TODO: enforce no duplicate keys? - // TODO: reverse? - // To maintain full fidelity to downstream receivers convert to the proper attribute types - - if (kvp.Value == null) - { - return null; - } - - var attrib = new OtlpCommon.KeyValue { Key = kvp.Key, Value = new OtlpCommon.AnyValue { } }; - - if (long.TryParse(kvp.Value, out var longValue)) - { - attrib.Value.IntValue = longValue; - } - else if (double.TryParse(kvp.Value, out var doubleValue)) - { - attrib.Value.DoubleValue = doubleValue; - } - else if (bool.TryParse(kvp.Value, out var boolValue)) - { - attrib.Value.BoolValue = boolValue; - } - else - { - attrib.Value.StringValue = kvp.Value; - } - - return attrib; - } - private static OtlpCommon.KeyValue ToOtlpAttribute(KeyValuePair kvp) { if (kvp.Value == null) diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs index 51f62c7a0fc..e8d95a7b6f8 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs @@ -37,8 +37,8 @@ internal static class ZipkinActivityConversionExtensions [SemanticConventions.AttributeNetPeerIp] = 2, ["peer.hostname"] = 2, ["peer.address"] = 2, - ["http.host"] = 3, // RemoteEndpoint.ServiceName for Http. - ["db.instance"] = 3, // RemoteEndpoint.ServiceName for Redis. + [SemanticConventions.AttributeHttpHost] = 3, // RemoteEndpoint.ServiceName for Http. + [SemanticConventions.AttributeDbInstance] = 3, // RemoteEndpoint.ServiceName for Redis. }; private static readonly string InvalidSpanId = default(ActivitySpanId).ToHexString(); @@ -46,7 +46,7 @@ internal static class ZipkinActivityConversionExtensions private static readonly ConcurrentDictionary LocalEndpointCache = new ConcurrentDictionary(); private static readonly ConcurrentDictionary RemoteEndpointCache = new ConcurrentDictionary(); - private static readonly DictionaryEnumerator.ForEachDelegate ProcessTagsRef = ProcessTags; + private static readonly DictionaryEnumerator.ForEachDelegate ProcessTagsRef = ProcessTags; private static readonly ListEnumerator>.ForEachDelegate ProcessActivityEventsRef = ProcessActivityEvents; internal static ZipkinSpan ToZipkinSpan(this Activity activity, ZipkinEndpoint defaultLocalEndpoint, bool useShortTraceIds = false) @@ -62,18 +62,18 @@ internal static ZipkinSpan ToZipkinSpan(this Activity activity, ZipkinEndpoint d var attributeEnumerationState = new AttributeEnumerationState { - Tags = PooledList>.Create(), + Tags = PooledList>.Create(), }; - DictionaryEnumerator.AllocationFreeForEach(activity.Tags, ref attributeEnumerationState, ProcessTagsRef); + DictionaryEnumerator.AllocationFreeForEach(activity.TagObjects, ref attributeEnumerationState, ProcessTagsRef); var activitySource = activity.Source; if (!string.IsNullOrEmpty(activitySource.Name)) { - PooledList>.Add(ref attributeEnumerationState.Tags, new KeyValuePair("library.name", activitySource.Name)); + PooledList>.Add(ref attributeEnumerationState.Tags, new KeyValuePair("library.name", activitySource.Name)); if (!string.IsNullOrEmpty(activitySource.Version)) { - PooledList>.Add(ref attributeEnumerationState.Tags, new KeyValuePair("library.version", activitySource.Version)); + PooledList>.Add(ref attributeEnumerationState.Tags, new KeyValuePair("library.version", activitySource.Version)); } } @@ -161,19 +161,14 @@ private static string EncodeTraceId(ActivityTraceId traceId, bool useShortTraceI private static string ToActivityKind(Activity activity) { - switch (activity.Kind) + return activity.Kind switch { - case ActivityKind.Server: - return "SERVER"; - case ActivityKind.Producer: - return "PRODUCER"; - case ActivityKind.Consumer: - return "CONSUMER"; - case ActivityKind.Client: - return "CLIENT"; - } - - return null; + ActivityKind.Server => "SERVER", + ActivityKind.Producer => "PRODUCER", + ActivityKind.Consumer => "CONSUMER", + ActivityKind.Client => "CLIENT", + _ => null, + }; } private static bool ProcessActivityEvents(ref PooledList annotations, ActivityEvent @event) @@ -182,10 +177,10 @@ private static bool ProcessActivityEvents(ref PooledList annot return true; } - private static bool ProcessTags(ref AttributeEnumerationState state, KeyValuePair attribute) + private static bool ProcessTags(ref AttributeEnumerationState state, KeyValuePair attribute) { string key = attribute.Key; - string strVal = attribute.Value; + string strVal = attribute.Value as string; if (strVal != null) { @@ -205,12 +200,12 @@ private static bool ProcessTags(ref AttributeEnumerationState state, KeyValuePai } else { - PooledList>.Add(ref state.Tags, new KeyValuePair(key, strVal)); + PooledList>.Add(ref state.Tags, new KeyValuePair(key, strVal)); } } else { - PooledList>.Add(ref state.Tags, new KeyValuePair(key, strVal)); + PooledList>.Add(ref state.Tags, new KeyValuePair(key, strVal)); } return true; @@ -218,7 +213,7 @@ private static bool ProcessTags(ref AttributeEnumerationState state, KeyValuePai private struct AttributeEnumerationState { - public PooledList> Tags; + public PooledList> Tags; public string RemoteEndpointServiceName; diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs index c36514e0acc..ae7a03b2346 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinSpan.cs @@ -37,7 +37,7 @@ public ZipkinSpan( ZipkinEndpoint localEndpoint, ZipkinEndpoint remoteEndpoint, in PooledList? annotations, - in PooledList>? tags, + in PooledList>? tags, bool? debug, bool? shared) { @@ -86,7 +86,7 @@ public ZipkinSpan( public PooledList? Annotations { get; } - public PooledList>? Tags { get; } + public PooledList>? Tags { get; } public bool? Debug { get; } @@ -282,7 +282,28 @@ public void Write(Utf8JsonWriter writer) foreach (var tag in this.Tags.Value) { - writer.WriteString(tag.Key, tag.Value); + if (tag.Value is int intValue) + { + writer.WriteNumber(tag.Key, intValue); + } + else if (tag.Value is bool boolVal) + { + writer.WriteBoolean(tag.Key, boolVal); + } + else if (tag.Value is double doubleVal) + { + writer.WriteNumber(tag.Key, doubleVal); + } + else if (tag.Value is string stringVal) + { + writer.WriteString(tag.Key, stringVal); + } + else + { + // Should we try to convert to string? Or + // just drop it? + writer.WriteString(tag.Key, tag.Value.ToString()); + } } writer.WriteEndObject(); diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterTest.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterTest.cs index fa1152c25c4..4fd2941e5d3 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterTest.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterTest.cs @@ -152,7 +152,7 @@ public void ToOtlpSpanTest() foreach (var kvp in attributes) { - rootActivity.SetTag(kvp.Key, kvp.Value.ToString()); + rootActivity.SetTag(kvp.Key, kvp.Value); } var startTime = new DateTime(2020, 02, 20, 20, 20, 20, DateTimeKind.Utc);