From 76136b5224080da833f3d91c69ef64f529cf61d6 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 25 Aug 2020 16:52:04 -0700 Subject: [PATCH 1/7] Include net.peer.port in ZipkinSpan.RemoteEndpoint.ServiceName when applicable --- .../ZipkinActivityConversionExtensions.cs | 51 ++++++- ...pkinActivityExporterRemoteEndpointTests.cs | 142 ++++++++++++++++-- .../ZipkinExporterTests.cs | 4 +- 3 files changed, 177 insertions(+), 20 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs index 6dec3619143..31912bda0fb 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs @@ -33,12 +33,10 @@ internal static class ZipkinActivityConversionExtensions private static readonly Dictionary RemoteEndpointServiceNameKeyResolutionDictionary = new Dictionary(StringComparer.OrdinalIgnoreCase) { [SemanticConventions.AttributePeerService] = 0, // priority 0 (highest). - [SemanticConventions.AttributeNetPeerName] = 1, - [SemanticConventions.AttributeNetPeerIp] = 2, - ["peer.hostname"] = 2, - ["peer.address"] = 2, - [SemanticConventions.AttributeHttpHost] = 3, // RemoteEndpoint.ServiceName for Http. - [SemanticConventions.AttributeDbInstance] = 3, // RemoteEndpoint.ServiceName for Redis. + ["peer.hostname"] = 1, + ["peer.address"] = 1, + [SemanticConventions.AttributeHttpHost] = 2, // RemoteEndpoint.ServiceName for Http. + [SemanticConventions.AttributeDbInstance] = 2, // RemoteEndpoint.ServiceName for Redis. }; private static readonly string InvalidSpanId = default(ActivitySpanId).ToHexString(); @@ -96,9 +94,24 @@ internal static ZipkinSpan ToZipkinSpan(this Activity activity, ZipkinEndpoint d } ZipkinEndpoint remoteEndpoint = null; - if ((activity.Kind == ActivityKind.Client || activity.Kind == ActivityKind.Producer) && attributeEnumerationState.RemoteEndpointServiceName != null) + if (activity.Kind == ActivityKind.Client || activity.Kind == ActivityKind.Producer) { - remoteEndpoint = RemoteEndpointCache.GetOrAdd(attributeEnumerationState.RemoteEndpointServiceName, ZipkinEndpoint.Create); + var hostNameOrIpAddress = attributeEnumerationState.HostName ?? attributeEnumerationState.IpAddress; + + if ((attributeEnumerationState.RemoteEndpointServiceName == null || attributeEnumerationState.RemoteEndpointServiceNamePriority > 0) + && hostNameOrIpAddress != null) + { + var remoteEndpointStr = attributeEnumerationState.Port != default + ? $"{hostNameOrIpAddress}:{attributeEnumerationState.Port}" + : hostNameOrIpAddress; + + remoteEndpoint = RemoteEndpointCache.GetOrAdd(remoteEndpointStr, ZipkinEndpoint.Create); + } + + if (remoteEndpoint == null && attributeEnumerationState.RemoteEndpointServiceName != null) + { + remoteEndpoint = RemoteEndpointCache.GetOrAdd(attributeEnumerationState.RemoteEndpointServiceName, ZipkinEndpoint.Create); + } } var annotations = PooledList.Create(); @@ -162,6 +175,18 @@ internal static bool ProcessTags(ref AttributeEnumerationState state, KeyValuePa state.RemoteEndpointServiceName = strVal; state.RemoteEndpointServiceNamePriority = priority; } + else if (key == SemanticConventions.AttributeNetPeerName) + { + state.HostName = strVal; + } + else if (key == SemanticConventions.AttributeNetPeerIp) + { + state.IpAddress = strVal; + } + else if (key == SemanticConventions.AttributeNetPeerPort && int.TryParse(strVal, out var port)) + { + state.Port = port; + } else if (key == Resource.ServiceNameKey) { state.ServiceName = strVal; @@ -175,6 +200,10 @@ internal static bool ProcessTags(ref AttributeEnumerationState state, KeyValuePa PooledList>.Add(ref state.Tags, new KeyValuePair(key, strVal)); } } + else if (attribute.Value is int intVal && attribute.Key == SemanticConventions.AttributeNetPeerPort) + { + state.Port = intVal; + } else { PooledList>.Add(ref state.Tags, attribute); @@ -224,6 +253,12 @@ internal struct AttributeEnumerationState public string ServiceName; public string ServiceNamespace; + + public string HostName; + + public string IpAddress; + + public int Port; } } } diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs index 37a62ed78f4..973eb15aa01 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs @@ -16,6 +16,7 @@ using System.Collections.Generic; using OpenTelemetry.Exporter.Zipkin.Implementation; +using OpenTelemetry.Trace; using Xunit; namespace OpenTelemetry.Exporter.Zipkin.Tests.Implementation @@ -53,23 +54,144 @@ public void ZipkinSpanConverterTest_GenerateActivity_RemoteEndpointResolution() Assert.Equal("RemoteServiceName", zipkinSpan.RemoteEndpoint.ServiceName); } - [Fact] - public void ZipkinSpanConverterTest_GenerateActivity_RemoteEndpointResolutionPriority() + [Theory] + [MemberData(nameof(RemoteEndpointPriorityTestCase.GetTestCases), MemberType = typeof(RemoteEndpointPriorityTestCase))] + public void ZipkinSpanConverterTest_GenerateActivity_RemoteEndpointResolutionPriority(RemoteEndpointPriorityTestCase testCase) { // Arrange - var activity = ZipkinExporterTests.CreateTestActivity( - additionalAttributes: new Dictionary - { - ["http.host"] = "DiscardedRemoteServiceName", - ["net.peer.name"] = "RemoteServiceName", - ["peer.hostname"] = "DiscardedRemoteServiceName", - }); + var activity = ZipkinExporterTests.CreateTestActivity(additionalAttributes: testCase.RemoteEndpointAttributes); // Act & Assert var zipkinSpan = ZipkinActivityConversionExtensions.ToZipkinSpan(activity, DefaultZipkinEndpoint); Assert.NotNull(zipkinSpan.RemoteEndpoint); - Assert.Equal("RemoteServiceName", zipkinSpan.RemoteEndpoint.ServiceName); + Assert.Equal(testCase.ExpectedResult, zipkinSpan.RemoteEndpoint.ServiceName); + } + + public class RemoteEndpointPriorityTestCase + { + public string Name { get; set; } + + public string ExpectedResult { get; set; } + + public Dictionary RemoteEndpointAttributes { get; set; } + + public static IEnumerable GetTestCases() + { + yield return new object[] + { + new RemoteEndpointPriorityTestCase + { + Name = "Highest priority name = net.peer.name", + ExpectedResult = "RemoteServiceName", + RemoteEndpointAttributes = new Dictionary + { + ["http.host"] = "DiscardedRemoteServiceName", + ["net.peer.name"] = "RemoteServiceName", + ["peer.hostname"] = "DiscardedRemoteServiceName", + }, + }, + }; + + yield return new object[] + { + new RemoteEndpointPriorityTestCase + { + Name = "Highest priority name = SemanticConventions.AttributePeerService", + ExpectedResult = "RemoteServiceName", + RemoteEndpointAttributes = new Dictionary + { + [SemanticConventions.AttributePeerService] = "RemoteServiceName", + ["http.host"] = "DiscardedRemoteServiceName", + ["net.peer.name"] = "DiscardedRemoteServiceName", + ["net.peer.port"] = "1234", + ["peer.hostname"] = "DiscardedRemoteServiceName", + }, + }, + }; + + yield return new object[] + { + new RemoteEndpointPriorityTestCase + { + Name = "Only has net.peer.name and net.peer.port", + ExpectedResult = "RemoteServiceName:1234", + RemoteEndpointAttributes = new Dictionary + { + ["net.peer.name"] = "RemoteServiceName", + ["net.peer.port"] = "1234", + }, + }, + }; + + yield return new object[] + { + new RemoteEndpointPriorityTestCase + { + Name = "net.peer.port is an int", + ExpectedResult = "RemoteServiceName:1234", + RemoteEndpointAttributes = new Dictionary + { + ["net.peer.name"] = "RemoteServiceName", + ["net.peer.port"] = 1234, + }, + }, + }; + + yield return new object[] + { + new RemoteEndpointPriorityTestCase + { + Name = "Has net.peer.name and net.peer.port", + ExpectedResult = "RemoteServiceName:1234", + RemoteEndpointAttributes = new Dictionary + { + ["http.host"] = "DiscardedRemoteServiceName", + ["net.peer.name"] = "RemoteServiceName", + ["net.peer.port"] = "1234", + ["peer.hostname"] = "DiscardedRemoteServiceName", + }, + }, + }; + + yield return new object[] + { + new RemoteEndpointPriorityTestCase + { + Name = "Has net.peer.ip and net.peer.port", + ExpectedResult = "1.2.3.4:1234", + RemoteEndpointAttributes = new Dictionary + { + ["http.host"] = "DiscardedRemoteServiceName", + ["net.peer.ip"] = "1.2.3.4", + ["net.peer.port"] = "1234", + ["peer.hostname"] = "DiscardedRemoteServiceName", + }, + }, + }; + + yield return new object[] + { + new RemoteEndpointPriorityTestCase + { + Name = "Has net.peer.name, net.peer.ip, and net.peer.port", + ExpectedResult = "RemoteServiceName:1234", + RemoteEndpointAttributes = new Dictionary + { + ["http.host"] = "DiscardedRemoteServiceName", + ["net.peer.name"] = "RemoteServiceName", + ["net.peer.ip"] = "1.2.3.4", + ["net.peer.port"] = "1234", + ["peer.hostname"] = "DiscardedRemoteServiceName", + }, + }, + }; + } + + public override string ToString() + { + return this.Name; + } } } } diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index 6c098b4516c..ff476339f67 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -170,7 +170,7 @@ public void ZipkinExporterIntegrationTest(bool useShortTraceIds) var traceId = useShortTraceIds ? TraceId.Substring(TraceId.Length - 16, 16) : TraceId; Assert.Equal( - $@"[{{""traceId"":""{traceId}"",""name"":""Name"",""parentId"":""{ZipkinActivityConversionExtensions.EncodeSpanId(activity.ParentSpanId)}"",""id"":""{ZipkinActivityConversionExtensions.EncodeSpanId(context.SpanId)}"",""kind"":""CLIENT"",""timestamp"":{timestamp},""duration"":60000000,""localEndpoint"":{{""serviceName"":""OpenTelemetry Exporter""{ipInformation}}},""annotations"":[{{""timestamp"":{eventTimestamp},""value"":""Event1""}},{{""timestamp"":{eventTimestamp},""value"":""Event2""}}],""tags"":{{""stringKey"":""value"",""longKey"":""1"",""longKey2"":""1"",""doubleKey"":""1"",""doubleKey2"":""1"",""boolKey"":""True"",""library.name"":""CreateTestActivity""}}}}]", + $@"[{{""traceId"":""{traceId}"",""name"":""Name"",""parentId"":""{ZipkinActivityConversionExtensions.EncodeSpanId(activity.ParentSpanId)}"",""id"":""{ZipkinActivityConversionExtensions.EncodeSpanId(context.SpanId)}"",""kind"":""CLIENT"",""timestamp"":{timestamp},""duration"":60000000,""localEndpoint"":{{""serviceName"":""OpenTelemetry Exporter""{ipInformation}}},""annotations"":[{{""timestamp"":{eventTimestamp},""value"":""Event1""}},{{""timestamp"":{eventTimestamp},""value"":""Event2""}}],""tags"":{{""stringKey"":""value"",""longKey"":1,""longKey2"":1,""doubleKey"":1.0,""doubleKey2"":1.0,""boolKey"":true,""library.name"":""CreateTestActivity""}}}}]", Responses[requestId]); } @@ -229,7 +229,7 @@ internal static Activity CreateTestActivity( var activitySource = new ActivitySource(nameof(CreateTestActivity)); var tags = setAttributes ? - attributes.Select(kvp => new KeyValuePair(kvp.Key, kvp.Value.ToString())) + attributes.Select(kvp => new KeyValuePair(kvp.Key, kvp.Value)) : null; var links = addLinks ? new[] From c48b01f3f7de4624e7b3e1215918467bf66af5bc Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 25 Aug 2020 20:19:04 -0700 Subject: [PATCH 2/7] Revert change to CreateTestActivity --- .../ZipkinExporterTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index ff476339f67..6c098b4516c 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -170,7 +170,7 @@ public void ZipkinExporterIntegrationTest(bool useShortTraceIds) var traceId = useShortTraceIds ? TraceId.Substring(TraceId.Length - 16, 16) : TraceId; Assert.Equal( - $@"[{{""traceId"":""{traceId}"",""name"":""Name"",""parentId"":""{ZipkinActivityConversionExtensions.EncodeSpanId(activity.ParentSpanId)}"",""id"":""{ZipkinActivityConversionExtensions.EncodeSpanId(context.SpanId)}"",""kind"":""CLIENT"",""timestamp"":{timestamp},""duration"":60000000,""localEndpoint"":{{""serviceName"":""OpenTelemetry Exporter""{ipInformation}}},""annotations"":[{{""timestamp"":{eventTimestamp},""value"":""Event1""}},{{""timestamp"":{eventTimestamp},""value"":""Event2""}}],""tags"":{{""stringKey"":""value"",""longKey"":1,""longKey2"":1,""doubleKey"":1.0,""doubleKey2"":1.0,""boolKey"":true,""library.name"":""CreateTestActivity""}}}}]", + $@"[{{""traceId"":""{traceId}"",""name"":""Name"",""parentId"":""{ZipkinActivityConversionExtensions.EncodeSpanId(activity.ParentSpanId)}"",""id"":""{ZipkinActivityConversionExtensions.EncodeSpanId(context.SpanId)}"",""kind"":""CLIENT"",""timestamp"":{timestamp},""duration"":60000000,""localEndpoint"":{{""serviceName"":""OpenTelemetry Exporter""{ipInformation}}},""annotations"":[{{""timestamp"":{eventTimestamp},""value"":""Event1""}},{{""timestamp"":{eventTimestamp},""value"":""Event2""}}],""tags"":{{""stringKey"":""value"",""longKey"":""1"",""longKey2"":""1"",""doubleKey"":""1"",""doubleKey2"":""1"",""boolKey"":""True"",""library.name"":""CreateTestActivity""}}}}]", Responses[requestId]); } @@ -229,7 +229,7 @@ internal static Activity CreateTestActivity( var activitySource = new ActivitySource(nameof(CreateTestActivity)); var tags = setAttributes ? - attributes.Select(kvp => new KeyValuePair(kvp.Key, kvp.Value)) + attributes.Select(kvp => new KeyValuePair(kvp.Key, kvp.Value.ToString())) : null; var links = addLinks ? new[] From c208e312cb14782bf190244c394d4da3af768c89 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 25 Aug 2020 20:30:39 -0700 Subject: [PATCH 3/7] Delete duplicate test suite --- .../ZipkinTraceExporterRemoteEndpointTests.cs | 74 ------------------- 1 file changed, 74 deletions(-) delete mode 100644 test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinTraceExporterRemoteEndpointTests.cs diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinTraceExporterRemoteEndpointTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinTraceExporterRemoteEndpointTests.cs deleted file mode 100644 index 108c9d303fa..00000000000 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinTraceExporterRemoteEndpointTests.cs +++ /dev/null @@ -1,74 +0,0 @@ -// -// 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.Collections.Generic; -using OpenTelemetry.Exporter.Zipkin.Implementation; -using Xunit; - -namespace OpenTelemetry.Exporter.Zipkin.Tests.Implementation -{ - public class ZipkinTraceExporterRemoteEndpointTests - { - private static readonly ZipkinEndpoint DefaultZipkinEndpoint = new ZipkinEndpoint("TestService"); - - [Fact] - public void ZipkinSpanConverterTest_GenerateSpan_RemoteEndpointOmittedByDefault() - { - // Arrange - var activity = ZipkinExporterTests.CreateTestActivity(); - - // Act & Assert - var zipkinSpan = ZipkinActivityConversionExtensions.ToZipkinSpan(activity, DefaultZipkinEndpoint); - - Assert.Null(zipkinSpan.RemoteEndpoint); - } - - [Fact] - public void ZipkinSpanConverterTest_GenerateSpan_RemoteEndpointResolution() - { - // Arrange - var activity = ZipkinExporterTests.CreateTestActivity( - additionalAttributes: new Dictionary - { - ["net.peer.name"] = "RemoteServiceName", - }); - - // Act & Assert - var zipkinSpan = ZipkinActivityConversionExtensions.ToZipkinSpan(activity, DefaultZipkinEndpoint); - - Assert.NotNull(zipkinSpan.RemoteEndpoint); - Assert.Equal("RemoteServiceName", zipkinSpan.RemoteEndpoint.ServiceName); - } - - [Fact] - public void ZipkinSpanConverterTest_GenerateSpan_RemoteEndpointResolutionPriority() - { - // Arrange - var activity = ZipkinExporterTests.CreateTestActivity( - additionalAttributes: new Dictionary - { - ["http.host"] = "DiscardedRemoteServiceName", - ["net.peer.name"] = "RemoteServiceName", - ["peer.hostname"] = "DiscardedRemoteServiceName", - }); - - // Act & Assert - var zipkinSpan = ZipkinActivityConversionExtensions.ToZipkinSpan(activity, DefaultZipkinEndpoint); - - Assert.NotNull(zipkinSpan.RemoteEndpoint); - Assert.Equal("RemoteServiceName", zipkinSpan.RemoteEndpoint.ServiceName); - } - } -} From 891051c9dd3f9f3cbb1b6fc5ad33a595d30163e8 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 25 Aug 2020 20:35:21 -0700 Subject: [PATCH 4/7] Update changelog --- src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md b/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md index a1c959756db..bf2592860f9 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.Zipkin/CHANGELOG.md @@ -6,6 +6,9 @@ ([#1066](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1066)) * Changed `ZipkinExporter` to use `BatchExportActivityProcessor` by default ([#1103](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1103)) +* Fixed issue when span has both the `net.peer.name` and `net.peer.port` + attributes but did not include `net.peer.port` in the service address field + ([#1168](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1168)). ## 0.4.0-beta.2 From c88120dc58b5b3647cf7aea42ffca157b754bfcc Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 25 Aug 2020 21:22:41 -0700 Subject: [PATCH 5/7] Enable handling of non-string attributes in Zipkin tests --- .../ZipkinActivityExporterRemoteEndpointTests.cs | 4 +++- .../ZipkinExporterTests.cs | 12 +++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs index 973eb15aa01..ed33110daee 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs @@ -59,7 +59,9 @@ public void ZipkinSpanConverterTest_GenerateActivity_RemoteEndpointResolution() public void ZipkinSpanConverterTest_GenerateActivity_RemoteEndpointResolutionPriority(RemoteEndpointPriorityTestCase testCase) { // Arrange - var activity = ZipkinExporterTests.CreateTestActivity(additionalAttributes: testCase.RemoteEndpointAttributes); + var activity = ZipkinExporterTests.CreateTestActivity( + additionalAttributes: testCase.RemoteEndpointAttributes, + convertAttributeValuesToString: false); // Act & Assert var zipkinSpan = ZipkinActivityConversionExtensions.ToZipkinSpan(activity, DefaultZipkinEndpoint); diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index 6c098b4516c..3c52df25665 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -177,6 +177,7 @@ public void ZipkinExporterIntegrationTest(bool useShortTraceIds) internal static Activity CreateTestActivity( bool setAttributes = true, Dictionary additionalAttributes = null, + bool convertAttributeValuesToString = true, bool addEvents = true, bool addLinks = true, Resource resource = null, @@ -228,9 +229,14 @@ internal static Activity CreateTestActivity( var activitySource = new ActivitySource(nameof(CreateTestActivity)); - var tags = setAttributes ? - attributes.Select(kvp => new KeyValuePair(kvp.Key, kvp.Value.ToString())) - : null; + IEnumerable> tags = null; + if (setAttributes) + { + tags = convertAttributeValuesToString + ? attributes.Select(kvp => new KeyValuePair(kvp.Key, kvp.Value.ToString())) + : attributes.Select(kvp => new KeyValuePair(kvp.Key, kvp.Value)); + } + var links = addLinks ? new[] { From b4dd84dd4df12b54d68f648faf2e69956eea1d33 Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Wed, 26 Aug 2020 10:22:20 -0700 Subject: [PATCH 6/7] Revert "Enable handling of non-string attributes in Zipkin tests" This reverts commit c88120dc58b5b3647cf7aea42ffca157b754bfcc. --- .../ZipkinActivityExporterRemoteEndpointTests.cs | 4 +--- .../ZipkinExporterTests.cs | 12 +++--------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs index ed33110daee..973eb15aa01 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/Implementation/ZipkinActivityExporterRemoteEndpointTests.cs @@ -59,9 +59,7 @@ public void ZipkinSpanConverterTest_GenerateActivity_RemoteEndpointResolution() public void ZipkinSpanConverterTest_GenerateActivity_RemoteEndpointResolutionPriority(RemoteEndpointPriorityTestCase testCase) { // Arrange - var activity = ZipkinExporterTests.CreateTestActivity( - additionalAttributes: testCase.RemoteEndpointAttributes, - convertAttributeValuesToString: false); + var activity = ZipkinExporterTests.CreateTestActivity(additionalAttributes: testCase.RemoteEndpointAttributes); // Act & Assert var zipkinSpan = ZipkinActivityConversionExtensions.ToZipkinSpan(activity, DefaultZipkinEndpoint); diff --git a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs index 3c52df25665..6c098b4516c 100644 --- a/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs +++ b/test/OpenTelemetry.Exporter.Zipkin.Tests/ZipkinExporterTests.cs @@ -177,7 +177,6 @@ public void ZipkinExporterIntegrationTest(bool useShortTraceIds) internal static Activity CreateTestActivity( bool setAttributes = true, Dictionary additionalAttributes = null, - bool convertAttributeValuesToString = true, bool addEvents = true, bool addLinks = true, Resource resource = null, @@ -229,14 +228,9 @@ internal static Activity CreateTestActivity( var activitySource = new ActivitySource(nameof(CreateTestActivity)); - IEnumerable> tags = null; - if (setAttributes) - { - tags = convertAttributeValuesToString - ? attributes.Select(kvp => new KeyValuePair(kvp.Key, kvp.Value.ToString())) - : attributes.Select(kvp => new KeyValuePair(kvp.Key, kvp.Value)); - } - + var tags = setAttributes ? + attributes.Select(kvp => new KeyValuePair(kvp.Key, kvp.Value.ToString())) + : null; var links = addLinks ? new[] { From b3d6f02a2d64f2f8f7318a93ab6857d9e99601ca Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Wed, 26 Aug 2020 14:20:51 -0700 Subject: [PATCH 7/7] Use ValueTuple for RemoteEndpointCache key --- .../ZipkinActivityConversionExtensions.cs | 13 +++++++++++++ .../Implementation/ZipkinEndpoint.cs | 11 +++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs index 31912bda0fb..ccabc070210 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinActivityConversionExtensions.cs @@ -42,7 +42,12 @@ internal static class ZipkinActivityConversionExtensions private static readonly string InvalidSpanId = default(ActivitySpanId).ToHexString(); private static readonly ConcurrentDictionary LocalEndpointCache = new ConcurrentDictionary(); + +#if !NET452 + private static readonly ConcurrentDictionary<(string, int), ZipkinEndpoint> RemoteEndpointCache = new ConcurrentDictionary<(string, int), ZipkinEndpoint>(); +#else private static readonly ConcurrentDictionary RemoteEndpointCache = new ConcurrentDictionary(); +#endif private static readonly DictionaryEnumerator.ForEachDelegate ProcessTagsRef = ProcessTags; private static readonly ListEnumerator>.ForEachDelegate ProcessActivityEventsRef = ProcessActivityEvents; @@ -101,16 +106,24 @@ internal static ZipkinSpan ToZipkinSpan(this Activity activity, ZipkinEndpoint d if ((attributeEnumerationState.RemoteEndpointServiceName == null || attributeEnumerationState.RemoteEndpointServiceNamePriority > 0) && hostNameOrIpAddress != null) { +#if !NET452 + remoteEndpoint = RemoteEndpointCache.GetOrAdd((hostNameOrIpAddress, attributeEnumerationState.Port), ZipkinEndpoint.Create); +#else var remoteEndpointStr = attributeEnumerationState.Port != default ? $"{hostNameOrIpAddress}:{attributeEnumerationState.Port}" : hostNameOrIpAddress; remoteEndpoint = RemoteEndpointCache.GetOrAdd(remoteEndpointStr, ZipkinEndpoint.Create); +#endif } if (remoteEndpoint == null && attributeEnumerationState.RemoteEndpointServiceName != null) { +#if !NET452 + remoteEndpoint = RemoteEndpointCache.GetOrAdd((attributeEnumerationState.RemoteEndpointServiceName, default), ZipkinEndpoint.Create); +#else remoteEndpoint = RemoteEndpointCache.GetOrAdd(attributeEnumerationState.RemoteEndpointServiceName, ZipkinEndpoint.Create); +#endif } } diff --git a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinEndpoint.cs b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinEndpoint.cs index 35d81c16e00..6f62be209e3 100644 --- a/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinEndpoint.cs +++ b/src/OpenTelemetry.Exporter.Zipkin/Implementation/ZipkinEndpoint.cs @@ -53,6 +53,17 @@ public static ZipkinEndpoint Create(string serviceName) return new ZipkinEndpoint(serviceName); } +#if !NET452 + public static ZipkinEndpoint Create((string name, int port) serviceNameAndPort) + { + var serviceName = serviceNameAndPort.port == default + ? serviceNameAndPort.name + : $"{serviceNameAndPort.name}:{serviceNameAndPort.port}"; + + return new ZipkinEndpoint(serviceName); + } +#endif + public ZipkinEndpoint Clone(string serviceName) { return new ZipkinEndpoint(