Update Resource (#613)

* Update Resource

When looking at grouping telemetry in an exporter based on the Resource
it is ideal if a map can be make with the key being represented by a
Resource. However, given the Resource is not hashable, this is not
possible.

This add a `String` method that can be used as a map key during
grouping. Additionally, this means the Resource now implements the
`Stringer` interface providing human-readable info when prited.

The internal structure of the Resource is changed. A static slice
containing all key-values in a sorted order replaces the existing map.
Additionally a set of keys is added to accommodate lookup during
`Merge`. Also, the string representation is kept in an internal field so
as to save processing for the `String` method (all fields are assumed to
be static after creation).

The `Attributes` method now returns a sorted slice of the associated
key-values.

The `Merge` method has been updated to support the changed structure of
the Resource.

New tests are added to validate the `String` method.

* Update comment

* Change loop into returned append

* Update key-value less func

Keys are unique in this package, treat them that way.

* Remove unnecessary allocation on empty attributes

* Update `Merge` method

Remove incomplete sorting of merged slices. Instead use the `sort`
package.

Add tests to catch sorting failure identified.

* Apply suggestions from code review

Co-Authored-By: ET <evantorrie@users.noreply.github.com>

* Escape Resource string representation

To ensure uniqueness of the string representation, the key-value content
needs to be escaped.

* Switch to an eager evaluation for the `String` method

* Refactor `Merge` method

Leave optimization to the future and simplify the merge.

* Add AttributeIterator

Include a method for a user of the Resource to iterate over the related
attributes without needed to copy the attributes.

* Fix ineffectual

* Fix lint

* Add licenses

* keys -> keySet for Resource

Co-authored-by: ET <evantorrie@users.noreply.github.com>
Co-authored-by: Rahul Patel <rahulpa@google.com>

sdk/resource/iterator.go

@@ -0,0 +1,65 @@
+// 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.
+
+package resource
+
+import "go.opentelemetry.io/otel/api/core"
+
+// AttributeIterator allows iterating over an ordered set of Resource attributes.
+//
+// The typical use of the iterator assuming a Resource named `res`, is
+// something like the following:
+//
+//     for iter := res.Iter(); iter.Next(); {
+//       attr := iter.Attribute()
+//       // or, if an index is needed:
+//       // idx, attr := iter.IndexedAttribute()
+//
+//       // ...
+//     }
+type AttributeIterator struct {
+	attrs []core.KeyValue
+	idx   int
+}
+
+// NewAttributeIterator creates an iterator going over a passed attrs.
+func NewAttributeIterator(attrs []core.KeyValue) AttributeIterator {
+	return AttributeIterator{attrs: attrs, idx: -1}
+}
+
+// Next moves the iterator to the next attribute.
+// Returns false if there are no more attributes.
+func (i *AttributeIterator) Next() bool {
+	i.idx++
+	return i.idx < i.Len()
+}
+
+// Attribute returns current attribute.
+//
+// Must be called only after Next returns true.
+func (i *AttributeIterator) Attribute() core.KeyValue {
+	return i.attrs[i.idx]
+}
+
+// IndexedAttribute returns current index and attribute.
+//
+// Must be called only after Next returns true.
+func (i *AttributeIterator) IndexedAttribute() (int, core.KeyValue) {
+	return i.idx, i.Attribute()
+}
+
+// Len returns a number of attributes.
+func (i *AttributeIterator) Len() int {
+	return len(i.attrs)
+}

sdk/resource/iterator_test.go

@@ -0,0 +1,53 @@
+// 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.
+
+package resource
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"go.opentelemetry.io/otel/api/core"
+)
+
+func TestAttributeIterator(t *testing.T) {
+	one := core.Key("one").String("1")
+	two := core.Key("two").Int(2)
+	iter := NewAttributeIterator([]core.KeyValue{one, two})
+	require.Equal(t, 2, iter.Len())
+
+	require.True(t, iter.Next())
+	require.Equal(t, one, iter.Attribute())
+	idx, attr := iter.IndexedAttribute()
+	require.Equal(t, 0, idx)
+	require.Equal(t, one, attr)
+	require.Equal(t, 2, iter.Len())
+
+	require.True(t, iter.Next())
+	require.Equal(t, two, iter.Attribute())
+	idx, attr = iter.IndexedAttribute()
+	require.Equal(t, 1, idx)
+	require.Equal(t, two, attr)
+	require.Equal(t, 2, iter.Len())
+
+	require.False(t, iter.Next())
+	require.Equal(t, 2, iter.Len())
+}
+
+func TestEmptyAttributeIterator(t *testing.T) {
+	iter := NewAttributeIterator(nil)
+	require.Equal(t, 0, iter.Len())
+	require.False(t, iter.Next())
+}

sdk/resource/resource.go

@@ -17,30 +17,79 @@
 package resource
 
 import (
-	"reflect"
+	"sort"
+	"strings"
 
 	"go.opentelemetry.io/otel/api/core"
 )
 
 // Resource describes an entity about which identifying information and metadata is exposed.
 type Resource struct {
-	labels map[core.Key]core.Value
+	sorted []core.KeyValue
+	keySet map[core.Key]struct{}
 }
 
 // New creates a resource from a set of attributes.
 // If there are duplicates keys then the first value of the key is preserved.
 func New(kvs ...core.KeyValue) *Resource {
-	res := &Resource{
-		labels: map[core.Key]core.Value{},
-	}
+	res := &Resource{keySet: make(map[core.Key]struct{})}
 	for _, kv := range kvs {
-		if _, ok := res.labels[kv.Key]; !ok {
-			res.labels[kv.Key] = kv.Value
+		// First key wins.
+		if _, ok := res.keySet[kv.Key]; !ok {
+			res.keySet[kv.Key] = struct{}{}
+			res.sorted = append(res.sorted, kv)
 		}
 	}
+	sort.Slice(res.sorted, func(i, j int) bool {
+		return res.sorted[i].Key < res.sorted[j].Key
+	})
 	return res
 }
 
+// String implements the Stringer interface and provides a reproducibly
+// hashable representation of a Resource.
+func (r Resource) String() string {
+	// Ensure unique strings if key/value contains '=', ',', or '\'.
+	escaper := strings.NewReplacer("=", `\=`, ",", `\,`, `\`, `\\`)
+
+	var b strings.Builder
+	// Note: this could be further optimized by precomputing the size of
+	// the resulting buffer and adding a call to b.Grow
+	b.WriteString("Resource(")
+	if len(r.sorted) > 0 {
+		b.WriteString(escaper.Replace(string(r.sorted[0].Key)))
+		b.WriteRune('=')
+		b.WriteString(escaper.Replace(r.sorted[0].Value.Emit()))
+		for _, s := range r.sorted[1:] {
+			b.WriteRune(',')
+			b.WriteString(escaper.Replace(string(s.Key)))
+			b.WriteRune('=')
+			b.WriteString(escaper.Replace(s.Value.Emit()))
+		}
+
+	}
+	b.WriteRune(')')
+
+	return b.String()
+}
+
+// Attributes returns a copy of attributes from the resource in a sorted order.
+func (r Resource) Attributes() []core.KeyValue {
+	return append([]core.KeyValue(nil), r.sorted...)
+}
+
+// Iter returns an interator of the Resource attributes.
+//
+// This is ideal to use if you do not want a copy of the attributes.
+func (r Resource) Iter() AttributeIterator {
+	return NewAttributeIterator(r.sorted)
+}
+
+// Equal returns true if other Resource is equal to r.
+func (r Resource) Equal(other Resource) bool {
+	return r.String() == other.String()
+}
+
 // Merge creates a new resource by combining resource a and b.
 // If there are common key between resource a and b then value from resource a is preserved.
 // If one of the resources is nil then the other resource is returned without creating a new one.
@@ -51,29 +100,12 @@ func Merge(a, b *Resource) *Resource {
 	if b == nil {
 		return a
 	}
-	res := &Resource{
-		labels: map[core.Key]core.Value{},
-	}
-	for k, v := range b.labels {
-		res.labels[k] = v
-	}
-	// labels from resource a overwrite labels from resource b.
-	for k, v := range a.labels {
-		res.labels[k] = v
-	}
-	return res
-}
 
-// Attributes returns a copy of attributes from the resource.
-func (r Resource) Attributes() []core.KeyValue {
-	attrs := make([]core.KeyValue, 0, len(r.labels))
-	for k, v := range r.labels {
-		attrs = append(attrs, core.KeyValue{Key: k, Value: v})
-	}
-	return attrs
-}
+	// Note: the following could be optimized by implementing a dedicated merge sort.
 
-// Equal returns true if other Resource is the equal to r.
-func (r Resource) Equal(other Resource) bool {
-	return reflect.DeepEqual(r.labels, other.labels)
+	kvs := make([]core.KeyValue, 0, len(a.sorted)+len(b.sorted))
+	kvs = append(kvs, a.sorted...)
+	// a overwrites b, so b needs to be at the end.
+	kvs = append(kvs, b.sorted...)
+	return New(kvs...)
 }

sdk/resource/resource_test.go

@@ -16,7 +16,6 @@ package resource_test
 
 import (
 	"fmt"
-	"sort"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -52,15 +51,15 @@ func TestNew(t *testing.T) {
 		{
 			name: "New with nil",
 			in:   nil,
-			want: []core.KeyValue{},
+			want: nil,
 		},
 	}
 	for _, c := range cases {
 		t.Run(fmt.Sprintf("case-%s", c.name), func(t *testing.T) {
 			res := resource.New(c.in...)
 			if diff := cmp.Diff(
-				sortedAttributes(res.Attributes()),
-				sortedAttributes(c.want),
+				res.Attributes(),
+				c.want,
 				cmp.AllowUnexported(core.Value{})); diff != "" {
 				t.Fatalf("unwanted result: diff %+v,", diff)
 			}
@@ -80,11 +79,17 @@ func TestMerge(t *testing.T) {
 			b:    resource.New(kv21, kv41),
 			want: []core.KeyValue{kv11, kv21, kv31, kv41},
 		},
+		{
+			name: "Merge with no overlap, no nil, not interleaved",
+			a:    resource.New(kv11, kv21),
+			b:    resource.New(kv31, kv41),
+			want: []core.KeyValue{kv11, kv21, kv31, kv41},
+		},
 		{
 			name: "Merge with common key order1",
 			a:    resource.New(kv11),
 			b:    resource.New(kv12, kv21),
-			want: []core.KeyValue{kv21, kv11},
+			want: []core.KeyValue{kv11, kv21},
 		},
 		{
 			name: "Merge with common key order2",
@@ -92,6 +97,30 @@ func TestMerge(t *testing.T) {
 			b:    resource.New(kv11),
 			want: []core.KeyValue{kv12, kv21},
 		},
+		{
+			name: "Merge with common key order4",
+			a:    resource.New(kv11, kv21, kv41),
+			b:    resource.New(kv31, kv41),
+			want: []core.KeyValue{kv11, kv21, kv31, kv41},
+		},
+		{
+			name: "Merge with no keys",
+			a:    resource.New(),
+			b:    resource.New(),
+			want: nil,
+		},
+		{
+			name: "Merge with first resource no keys",
+			a:    resource.New(),
+			b:    resource.New(kv21),
+			want: []core.KeyValue{kv21},
+		},
+		{
+			name: "Merge with second resource no keys",
+			a:    resource.New(kv11),
+			b:    resource.New(),
+			want: []core.KeyValue{kv11},
+		},
 		{
 			name: "Merge with first resource nil",
 			a:    nil,
@@ -109,8 +138,8 @@ func TestMerge(t *testing.T) {
 		t.Run(fmt.Sprintf("case-%s", c.name), func(t *testing.T) {
 			res := resource.Merge(c.a, c.b)
 			if diff := cmp.Diff(
-				sortedAttributes(res.Attributes()),
-				sortedAttributes(c.want),
+				res.Attributes(),
+				c.want,
 				cmp.AllowUnexported(core.Value{})); diff != "" {
 				t.Fatalf("unwanted result: diff %+v,", diff)
 			}
@@ -118,9 +147,66 @@ func TestMerge(t *testing.T) {
 	}
 }
 
-func sortedAttributes(attrs []core.KeyValue) []core.KeyValue {
-	sort.Slice(attrs[:], func(i, j int) bool {
-		return attrs[i].Key < attrs[j].Key
-	})
-	return attrs
+func TestString(t *testing.T) {
+	for _, test := range []struct {
+		kvs  []core.KeyValue
+		want string
+	}{
+		{
+			kvs:  nil,
+			want: "Resource()",
+		},
+		{
+			kvs:  []core.KeyValue{},
+			want: "Resource()",
+		},
+		{
+			kvs:  []core.KeyValue{kv11},
+			want: "Resource(k1=v11)",
+		},
+		{
+			kvs:  []core.KeyValue{kv11, kv12},
+			want: "Resource(k1=v11)",
+		},
+		{
+			kvs:  []core.KeyValue{kv11, kv21},
+			want: "Resource(k1=v11,k2=v21)",
+		},
+		{
+			kvs:  []core.KeyValue{kv21, kv11},
+			want: "Resource(k1=v11,k2=v21)",
+		},
+		{
+			kvs:  []core.KeyValue{kv11, kv21, kv31},
+			want: "Resource(k1=v11,k2=v21,k3=v31)",
+		},
+		{
+			kvs:  []core.KeyValue{kv31, kv11, kv21},
+			want: "Resource(k1=v11,k2=v21,k3=v31)",
+		},
+		{
+			kvs:  []core.KeyValue{core.Key("A").String("a"), core.Key("B").String("b")},
+			want: "Resource(A=a,B=b)",
+		},
+		{
+			kvs:  []core.KeyValue{core.Key("A").String("a,B=b")},
+			want: `Resource(A=a\,B\=b)`,
+		},
+		{
+			kvs:  []core.KeyValue{core.Key("A").String(`a,B\=b`)},
+			want: `Resource(A=a\,B\\\=b)`,
+		},
+		{
+			kvs:  []core.KeyValue{core.Key("A=a,B").String(`b`)},
+			want: `Resource(A\=a\,B=b)`,
+		},
+		{
+			kvs:  []core.KeyValue{core.Key(`A=a\,B`).String(`b`)},
+			want: `Resource(A\=a\\\,B=b)`,
+		},
+	} {
+		if got := resource.New(test.kvs...).String(); got != test.want {
+			t.Errorf("Resource(%v).String() = %q, want %q", test.kvs, got, test.want)
+		}
+	}
 }