From 2b511713a11de17ce4f4a47164df2d86819f07e8 Mon Sep 17 00:00:00 2001 From: Austin Tan Date: Tue, 26 Jan 2021 15:59:38 -0800 Subject: [PATCH 1/6] Reversing this versus other in merge priority --- src/OpenTelemetry/Resources/Resource.cs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/OpenTelemetry/Resources/Resource.cs b/src/OpenTelemetry/Resources/Resource.cs index fcb73f8fc82..adaa85d554e 100644 --- a/src/OpenTelemetry/Resources/Resource.cs +++ b/src/OpenTelemetry/Resources/Resource.cs @@ -57,8 +57,8 @@ internal Resource(IEnumerable> attributes) public IEnumerable> Attributes { get; } /// - /// Returns a new, merged by merging the current with the. - /// other . In case of a collision the current takes precedence. + /// Returns a new, merged by merging the old with the. + /// other . In case of a collision the other takes precedence. /// /// The that will be merged with. this. /// . @@ -66,25 +66,25 @@ public Resource Merge(Resource other) { var newAttributes = new Dictionary(); - foreach (var attribute in this.Attributes) - { - if (!newAttributes.TryGetValue(attribute.Key, out var value) || (value is string strValue && string.IsNullOrEmpty(strValue))) - { - newAttributes[attribute.Key] = attribute.Value; - } - } - if (other != null) { foreach (var attribute in other.Attributes) { - if (!newAttributes.TryGetValue(attribute.Key, out var value) || (value is string strValue && string.IsNullOrEmpty(strValue))) + if (!newAttributes.TryGetValue(attribute.Key, out var value)) { newAttributes[attribute.Key] = attribute.Value; } } } + foreach (var attribute in this.Attributes) + { + if (!newAttributes.TryGetValue(attribute.Key, out var value)) + { + newAttributes[attribute.Key] = attribute.Value; + } + } + return new Resource(newAttributes); } From 67a938d9ce646daebf0899a7ac0a2ff05a1ae387 Mon Sep 17 00:00:00 2001 From: Austin Tan Date: Tue, 26 Jan 2021 19:01:14 -0800 Subject: [PATCH 2/6] revised test for new behavior --- test/OpenTelemetry.Tests/Resources/ResourceTest.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/OpenTelemetry.Tests/Resources/ResourceTest.cs b/test/OpenTelemetry.Tests/Resources/ResourceTest.cs index a9d4d61dff5..3abd7845d3f 100644 --- a/test/OpenTelemetry.Tests/Resources/ResourceTest.cs +++ b/test/OpenTelemetry.Tests/Resources/ResourceTest.cs @@ -315,19 +315,19 @@ public void MergeResource_MultiAttributeSource_DuplicatedKeysInPrimary() } [Fact] - public void MergeResource_SecondaryCanOverridePrimaryEmptyAttributeValue() + public void MergeResource_UpdatingResourceOverridesCurrentResource() { // Arrange - var primaryAttributes = new Dictionary { { "value", string.Empty } }; - var secondaryAttributes = new Dictionary { { "value", "not empty" } }; - var primaryResource = new Resource(primaryAttributes); - var secondaryResource = new Resource(secondaryAttributes); + var currentAttributes = new Dictionary { { "value", "currentValue" } }; + var updatingAttributes = new Dictionary { { "value", "updatedValue" } }; + var currentResource = new Resource(currentAttributes); + var updatingResource = new Resource(updatingAttributes); - var newResource = primaryResource.Merge(secondaryResource); + var newResource = currentResource.Merge(updatingResource); // Assert Assert.Single(newResource.Attributes); - Assert.Contains(new KeyValuePair("value", "not empty"), newResource.Attributes); + Assert.Contains(new KeyValuePair("value", "updatedValue"), newResource.Attributes); } [Fact] From 15a205d0e841007ddf83705c24bb160bdb2bbd26 Mon Sep 17 00:00:00 2001 From: Austin Tan Date: Tue, 26 Jan 2021 19:08:15 -0800 Subject: [PATCH 3/6] Changelog updated --- src/OpenTelemetry/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index bcc0e5b4c27..a8af2c2fb5b 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -26,6 +26,10 @@ them to supported data types (long for int/short, double for float). For invalid attributes we now throw an exception instead of logging an error. ([#1720](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1720)) +* Merging "this" resource with another previously kept "this" resource's + attributes in the event of a conflict. We've rectified to follow a recent + change to the spec, where we now prioritize the incoming resource's tags. + ([#1728](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1728)) ## 1.0.0-rc1.1 From ddcf139ad09f77954768067ab2c70291ddd50ca1 Mon Sep 17 00:00:00 2001 From: Austin Tan Date: Wed, 27 Jan 2021 10:25:24 -0800 Subject: [PATCH 4/6] Changelog updated --- src/OpenTelemetry/CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index a8af2c2fb5b..30d6267a8fa 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -26,9 +26,9 @@ them to supported data types (long for int/short, double for float). For invalid attributes we now throw an exception instead of logging an error. ([#1720](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1720)) -* Merging "this" resource with another previously kept "this" resource's - attributes in the event of a conflict. We've rectified to follow a recent - change to the spec, where we now prioritize the incoming resource's tags. +* Merging "this" resource with an "other" resource now prioritizes the "other" + resource's attributes in a conflict. We've rectified to follow a recent + change to the spec. We previously prioritized "this" resource's tags. ([#1728](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1728)) ## 1.0.0-rc1.1 From a5706df4046d2550da03b165c7abfe46826f9316 Mon Sep 17 00:00:00 2001 From: Austin Tan Date: Wed, 27 Jan 2021 10:27:07 -0800 Subject: [PATCH 5/6] remove extra period --- src/OpenTelemetry/Resources/Resource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/Resources/Resource.cs b/src/OpenTelemetry/Resources/Resource.cs index adaa85d554e..074645c42b4 100644 --- a/src/OpenTelemetry/Resources/Resource.cs +++ b/src/OpenTelemetry/Resources/Resource.cs @@ -57,7 +57,7 @@ internal Resource(IEnumerable> attributes) public IEnumerable> Attributes { get; } /// - /// Returns a new, merged by merging the old with the. + /// Returns a new, merged by merging the old with the /// other . In case of a collision the other takes precedence. /// /// The that will be merged with. this. From aa0bcc0401dc209cd2e625c06768c614dcc94350 Mon Sep 17 00:00:00 2001 From: Austin Tan Date: Wed, 27 Jan 2021 13:36:25 -0800 Subject: [PATCH 6/6] Fixing more period issues/line breaks --- src/OpenTelemetry/Resources/Resource.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry/Resources/Resource.cs b/src/OpenTelemetry/Resources/Resource.cs index 074645c42b4..6d8f68e308c 100644 --- a/src/OpenTelemetry/Resources/Resource.cs +++ b/src/OpenTelemetry/Resources/Resource.cs @@ -58,9 +58,9 @@ internal Resource(IEnumerable> attributes) /// /// Returns a new, merged by merging the old with the - /// other . In case of a collision the other takes precedence. + /// other . In case of a collision the other takes precedence. /// - /// The that will be merged with. this. + /// The that will be merged with this. /// . public Resource Merge(Resource other) {