From 46f609660846655d066c72c21bb80dc1f18905ca Mon Sep 17 00:00:00 2001 From: shbieng Date: Sat, 25 Jun 2022 02:55:15 +0800 Subject: [PATCH] Reverse order of attribute precedence when merging two Resources (#1501) * Reverse order of attribute precedence when merging two Resources This reflects a change in the specification itself. https://github.com/open-telemetry/opentelemetry-specification/pull/1345 * Resolves #1500 Co-authored-by: Tyler Yahn --- CHANGELOG.md | 4 ++++ sdk/resource/resource.go | 9 +++++---- sdk/resource/resource_test.go | 17 +++++++++++++++-- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c624b9fe..493c29c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Changed + +- Reverse order in which `Resource` attributes are merged, per change in spec. (#1501) + ## [0.16.0] - 2020-01-13 ### Added diff --git a/sdk/resource/resource.go b/sdk/resource/resource.go index 0141f5b1..0195bb27 100644 --- a/sdk/resource/resource.go +++ b/sdk/resource/resource.go @@ -84,7 +84,8 @@ func (r *Resource) Equal(eq *Resource) bool { // Merge creates a new resource by combining resource a and b. // // If there are common keys between resource a and b, then the value -// from resource a is preserved. +// from resource b will overwrite the value from resource a, even +// if resource b's value is empty. func Merge(a, b *Resource) *Resource { if a == nil && b == nil { return Empty() @@ -96,9 +97,9 @@ func Merge(a, b *Resource) *Resource { return a } - // Note: 'a' labels will overwrite 'b' with last-value-wins in label.Key() - // Meaning this is equivalent to: append(b.Attributes(), a.Attributes()...) - mi := label.NewMergeIterator(a.LabelSet(), b.LabelSet()) + // Note: 'b' labels will overwrite 'a' with last-value-wins in label.Key() + // Meaning this is equivalent to: append(a.Attributes(), b.Attributes()...) + mi := label.NewMergeIterator(b.LabelSet(), a.LabelSet()) combine := make([]label.KeyValue, 0, a.Len()+b.Len()) for mi.Next() { combine = append(combine, mi.Label()) diff --git a/sdk/resource/resource_test.go b/sdk/resource/resource_test.go index 74f97e23..6457792c 100644 --- a/sdk/resource/resource_test.go +++ b/sdk/resource/resource_test.go @@ -32,6 +32,7 @@ var ( kv21 = label.String("k2", "v21") kv31 = label.String("k3", "v31") kv41 = label.String("k4", "v41") + kv42 = label.String("k4", "") ) func TestNew(t *testing.T) { @@ -91,13 +92,13 @@ func TestMerge(t *testing.T) { name: "Merge with common key order1", a: resource.NewWithAttributes(kv11), b: resource.NewWithAttributes(kv12, kv21), - want: []label.KeyValue{kv11, kv21}, + want: []label.KeyValue{kv12, kv21}, }, { name: "Merge with common key order2", a: resource.NewWithAttributes(kv12, kv21), b: resource.NewWithAttributes(kv11), - want: []label.KeyValue{kv12, kv21}, + want: []label.KeyValue{kv11, kv21}, }, { name: "Merge with common key order4", @@ -135,6 +136,18 @@ func TestMerge(t *testing.T) { b: nil, want: []label.KeyValue{kv11}, }, + { + name: "Merge with first resource value empty string", + a: resource.NewWithAttributes(kv42), + b: resource.NewWithAttributes(kv41), + want: []label.KeyValue{kv41}, + }, + { + name: "Merge with second resource value empty string", + a: resource.NewWithAttributes(kv41), + b: resource.NewWithAttributes(kv42), + want: []label.KeyValue{kv42}, + }, } for _, c := range cases { t.Run(fmt.Sprintf("case-%s", c.name), func(t *testing.T) {