Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource merge precedence #1728

Merged
merged 12 commits into from
Jan 28, 2021
4 changes: 4 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Austin-Tan marked this conversation as resolved.
Show resolved Hide resolved
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

Expand Down
22 changes: 11 additions & 11 deletions src/OpenTelemetry/Resources/Resource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,34 +57,34 @@ internal Resource(IEnumerable<KeyValuePair<string, object>> attributes)
public IEnumerable<KeyValuePair<string, object>> Attributes { get; }

/// <summary>
/// Returns a new, merged <see cref="Resource"/> by merging the current <see cref="Resource"/> with the.
/// <code>other</code> <see cref="Resource"/>. In case of a collision the current <see cref="Resource"/> takes precedence.
/// Returns a new, merged <see cref="Resource"/> by merging the old <see cref="Resource"/> with the.
Austin-Tan marked this conversation as resolved.
Show resolved Hide resolved
/// <code>other</code> <see cref="Resource"/>. In case of a collision the other <see cref="Resource"/> takes precedence.
/// </summary>
/// <param name="other">The <see cref="Resource"/> that will be merged with. <code>this</code>.</param>
/// <returns><see cref="Resource"/>.</returns>
public Resource Merge(Resource other)
{
var newAttributes = new Dictionary<string, object>();

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);
}

Expand Down
14 changes: 7 additions & 7 deletions test/OpenTelemetry.Tests/Resources/ResourceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -315,19 +315,19 @@ public void MergeResource_MultiAttributeSource_DuplicatedKeysInPrimary()
}

[Fact]
public void MergeResource_SecondaryCanOverridePrimaryEmptyAttributeValue()
public void MergeResource_UpdatingResourceOverridesCurrentResource()
{
// Arrange
var primaryAttributes = new Dictionary<string, object> { { "value", string.Empty } };
var secondaryAttributes = new Dictionary<string, object> { { "value", "not empty" } };
var primaryResource = new Resource(primaryAttributes);
var secondaryResource = new Resource(secondaryAttributes);
var currentAttributes = new Dictionary<string, object> { { "value", "currentValue" } };
var updatingAttributes = new Dictionary<string, object> { { "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<string, object>("value", "not empty"), newResource.Attributes);
Assert.Contains(new KeyValuePair<string, object>("value", "updatedValue"), newResource.Attributes);
}

[Fact]
Expand Down