Make all StringKeyValue non-nullable (open-telemetry#2019)

* Make all StringKeyValue non-nullable

Add benchmarks for logs and metrics similar to traces.

Before:

```
goos: darwin
goarch: amd64
pkg: go.opentelemetry.io/collector/consumer/pdata
BenchmarkMetricsFromOtlp
BenchmarkMetricsFromOtlp-16    	     298	   3999204 ns/op	 2424909 B/op	   70785 allocs/op
PASS
```

After:

```
goos: darwin
goarch: amd64
pkg: go.opentelemetry.io/collector/consumer/pdata
BenchmarkMetricsFromOtlp
BenchmarkMetricsFromOtlp-16    	     326	   3630227 ns/op	 2271248 B/op	   51577 allocs/op
PASS
```

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

* Update consumer/pdata/common_test.go

Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>

Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>

Makefile

@@ -273,7 +273,6 @@ genproto_sub:
 	@echo Modify them in the intermediate directory.
 	$(foreach file,$(OPENTELEMETRY_PROTO_FILES),$(call exec-command,sed -f proto_patch.sed $(OPENTELEMETRY_PROTO_SRC_DIR)/$(file) > $(PROTO_INTERMEDIATE_DIR)/$(file)))
 
-
 	@echo Generate Go code from .proto files in intermediate directory.
 	$(foreach file,$(OPENTELEMETRY_PROTO_FILES),$(call exec-command,$(PROTOC) $(PROTO_INCLUDES) --gogofaster_out=plugins=grpc:./ $(file)))
 

consumer/pdata/common.go

@@ -724,7 +724,8 @@ func (am AttributeMap) Len() int {
 // })
 func (am AttributeMap) ForEach(f func(k string, v AttributeValue)) {
 	for i := range *am.orig {
-		f((*am.orig)[i].Key, AttributeValue{&(*am.orig)[i].Value})
+		kv := &(*am.orig)[i]
+		f(kv.Key, AttributeValue{&kv.Value})
 	}
 }
 
@@ -792,22 +793,22 @@ func (akv StringValue) SetValue(v string) {
 	akv.orig.Value = v
 }
 
-func newStringKeyValue(k, v string) *otlpcommon.StringKeyValue {
-	return &otlpcommon.StringKeyValue{Key: k, Value: v}
+func newStringKeyValue(k, v string) otlpcommon.StringKeyValue {
+	return otlpcommon.StringKeyValue{Key: k, Value: v}
 }
 
 // StringMap stores a map of attribute keys to values.
 type StringMap struct {
-	orig *[]*otlpcommon.StringKeyValue
+	orig *[]otlpcommon.StringKeyValue
 }
 
 // NewStringMap creates a StringMap with 0 elements.
 func NewStringMap() StringMap {
-	orig := []*otlpcommon.StringKeyValue(nil)
+	orig := []otlpcommon.StringKeyValue(nil)
 	return StringMap{&orig}
 }
 
-func newStringMap(orig *[]*otlpcommon.StringKeyValue) StringMap {
+func newStringMap(orig *[]otlpcommon.StringKeyValue) StringMap {
 	return StringMap{orig}
 }
 
@@ -818,37 +819,36 @@ func newStringMap(orig *[]*otlpcommon.StringKeyValue) StringMap {
 // assert.EqualValues(t, NewStringMap().InitFromMap(map[string]string{...}), actual)
 func (sm StringMap) InitFromMap(attrMap map[string]string) StringMap {
 	if len(attrMap) == 0 {
-		*sm.orig = []*otlpcommon.StringKeyValue(nil)
+		*sm.orig = []otlpcommon.StringKeyValue(nil)
 		return sm
 	}
 	origs := make([]otlpcommon.StringKeyValue, len(attrMap))
-	wrappers := make([]*otlpcommon.StringKeyValue, len(attrMap))
 	ix := 0
 	for k, v := range attrMap {
-		wrappers[ix] = &origs[ix]
-		wrappers[ix].Key = k
-		wrappers[ix].Value = v
+		origs[ix].Key = k
+		origs[ix].Value = v
 		ix++
 	}
-	*sm.orig = wrappers
+	*sm.orig = origs
 	return sm
 }
 
 // InitEmptyWithCapacity constructs an empty StringMap with predefined slice capacity.
 func (sm StringMap) InitEmptyWithCapacity(cap int) {
 	if cap == 0 {
-		*sm.orig = []*otlpcommon.StringKeyValue(nil)
+		*sm.orig = []otlpcommon.StringKeyValue(nil)
 	}
-	*sm.orig = make([]*otlpcommon.StringKeyValue, 0, cap)
+	*sm.orig = make([]otlpcommon.StringKeyValue, 0, cap)
 }
 
 // Get returns the StringValue associated with the key and true,
 // otherwise an invalid instance of the StringKeyValue and false.
 // Calling any functions on the returned invalid instance will cause a panic.
 func (sm StringMap) Get(k string) (StringValue, bool) {
-	for _, a := range *sm.orig {
-		if a != nil && a.Key == k {
-			return StringValue{a}, true
+	for i := range *sm.orig {
+		skv := &(*sm.orig)[i]
+		if skv.Key == k {
+			return StringValue{skv}, true
 		}
 	}
 	return StringValue{nil}, false
@@ -857,8 +857,9 @@ func (sm StringMap) Get(k string) (StringValue, bool) {
 // Delete deletes the entry associated with the key and returns true if the key
 // was present in the map, otherwise returns false.
 func (sm StringMap) Delete(k string) bool {
-	for i, a := range *sm.orig {
-		if a != nil && a.Key == k {
+	for i := range *sm.orig {
+		skv := &(*sm.orig)[i]
+		if skv.Key == k {
 			(*sm.orig)[i] = (*sm.orig)[len(*sm.orig)-1]
 			*sm.orig = (*sm.orig)[:len(*sm.orig)-1]
 			return true
@@ -910,38 +911,35 @@ func (sm StringMap) Len() int {
 //   ...
 // })
 func (sm StringMap) ForEach(f func(k string, v StringValue)) {
-	for _, kv := range *sm.orig {
-		if kv == nil {
-			continue
-		}
-		f(kv.Key, StringValue{kv})
+	for i := range *sm.orig {
+		skv := &(*sm.orig)[i]
+		f(skv.Key, StringValue{skv})
 	}
 }
 
 // CopyTo copies all elements from the current map to the dest.
 func (sm StringMap) CopyTo(dest StringMap) {
 	newLen := len(*sm.orig)
 	if newLen == 0 {
-		*dest.orig = []*otlpcommon.StringKeyValue(nil)
+		*dest.orig = []otlpcommon.StringKeyValue(nil)
 		return
 	}
 	oldLen := len(*dest.orig)
 	if newLen <= oldLen {
 		*dest.orig = (*dest.orig)[:newLen]
-		for i, kv := range *sm.orig {
-			(*dest.orig)[i].Key = kv.Key
-			(*dest.orig)[i].Value = kv.Value
+		for i := range *sm.orig {
+			skv := &(*sm.orig)[i]
+			(*dest.orig)[i].Key = skv.Key
+			(*dest.orig)[i].Value = skv.Value
 		}
 		return
 	}
 	origs := make([]otlpcommon.StringKeyValue, len(*sm.orig))
-	wrappers := make([]*otlpcommon.StringKeyValue, len(*sm.orig))
 	for i, kv := range *sm.orig {
-		wrappers[i] = &origs[i]
-		wrappers[i].Key = kv.Key
-		wrappers[i].Value = kv.Value
+		origs[i].Key = kv.Key
+		origs[i].Value = kv.Value
 	}
-	*dest.orig = wrappers
+	*dest.orig = origs
 }
 
 // Sort sorts the entries in the StringMap so two instances can be compared.
@@ -950,7 +948,7 @@ func (sm StringMap) CopyTo(dest StringMap) {
 func (sm StringMap) Sort() StringMap {
 	sort.SliceStable(*sm.orig, func(i, j int) bool {
 		// Intention is to move the nil values at the end.
-		return ((*sm.orig)[j] == nil) || ((*sm.orig)[i] != nil && (*sm.orig)[i].Key < (*sm.orig)[j].Key)
+		return (*sm.orig)[i].Key < (*sm.orig)[j].Key
 	})
 	return sm
 }

consumer/pdata/common_test.go

@@ -715,14 +715,13 @@ func TestNilStringMap(t *testing.T) {
 	assert.EqualValues(t, NewStringMap(), NewStringMap().Sort())
 }
 
-func TestStringMapWithNilValues(t *testing.T) {
-	origWithNil := []*otlpcommon.StringKeyValue{
-		nil,
+func TestStringMapWithEmpty(t *testing.T) {
+	origWithNil := []otlpcommon.StringKeyValue{
+		{},
 		{
 			Key:   "test_key",
 			Value: "test_value",
 		},
-		nil,
 	}
 	sm := StringMap{
 		orig: &origWithNil,
@@ -844,38 +843,6 @@ func TestStringMap_ForEach(t *testing.T) {
 	assert.EqualValues(t, 0, len(rawMap))
 }
 
-func TestStringMap_ForEach_WithNils(t *testing.T) {
-	rawMap := map[string]string{"k0": "v0", "k1": "v1", "k2": "v2"}
-	rawOrigWithNil := []*otlpcommon.StringKeyValue{
-		nil,
-		{
-			Key:   "k0",
-			Value: "v0",
-		},
-		nil,
-		{
-			Key:   "k1",
-			Value: "v1",
-		},
-		nil,
-		{
-			Key:   "k2",
-			Value: "v2",
-		},
-		nil,
-	}
-	sm := StringMap{
-		orig: &rawOrigWithNil,
-	}
-	assert.EqualValues(t, 7, sm.Len())
-
-	sm.ForEach(func(k string, v StringValue) {
-		assert.EqualValues(t, rawMap[k], v.Value())
-		delete(rawMap, k)
-	})
-	assert.EqualValues(t, 0, len(rawMap))
-}
-
 func TestStringMap_CopyTo(t *testing.T) {
 	dest := NewStringMap()
 	// Test CopyTo to empty
@@ -896,7 +863,7 @@ func TestStringMap_InitFromMap(t *testing.T) {
 	assert.EqualValues(t, NewStringMap(), sm)
 
 	rawMap := map[string]string{"k0": "v0", "k1": "v1", "k2": "v2"}
-	rawOrig := []*otlpcommon.StringKeyValue{
+	rawOrig := []otlpcommon.StringKeyValue{
 		{
 			Key:   "k0",
 			Value: "v0",
@@ -987,15 +954,15 @@ func BenchmarkAttributeMap_RangeOverMap(b *testing.B) {
 
 func BenchmarkStringMap_ForEach(b *testing.B) {
 	const numElements = 20
-	rawOrigWithNil := make([]*otlpcommon.StringKeyValue, 2*numElements)
+	rawOrig := make([]otlpcommon.StringKeyValue, numElements)
 	for i := 0; i < numElements; i++ {
-		rawOrigWithNil[i*2] = &otlpcommon.StringKeyValue{
+		rawOrig[i] = otlpcommon.StringKeyValue{
 			Key:   "k" + strconv.Itoa(i),
 			Value: "v" + strconv.Itoa(i),
 		}
 	}
 	sm := StringMap{
-		orig: &rawOrigWithNil,
+		orig: &rawOrig,
 	}
 	b.ResetTimer()
 	for n := 0; n < b.N; n++ {
@@ -1014,7 +981,8 @@ func BenchmarkStringMap_RangeOverMap(b *testing.B) {
 	rawOrig := make(map[string]StringValue, numElements)
 	for i := 0; i < numElements; i++ {
 		key := "k" + strconv.Itoa(i)
-		rawOrig[key] = StringValue{newStringKeyValue(key, "v"+strconv.Itoa(i))}
+		skv := newStringKeyValue(key, "v"+strconv.Itoa(i))
+		rawOrig[key] = StringValue{&skv}
 	}
 
 	b.ResetTimer()

consumer/pdata/log_test.go

@@ -18,6 +18,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/collector/internal"
 	otlplogs "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/logs/v1"
@@ -105,3 +106,29 @@ func BenchmarkLogsClone(b *testing.B) {
 		}
 	}
 }
+
+func BenchmarkLogsToOtlp(b *testing.B) {
+	traces := NewLogs()
+	fillTestResourceLogsSlice(traces.ResourceLogs())
+	b.ResetTimer()
+	for n := 0; n < b.N; n++ {
+		buf, err := traces.ToOtlpProtoBytes()
+		require.NoError(b, err)
+		assert.NotEqual(b, 0, len(buf))
+	}
+}
+
+func BenchmarkLogsFromOtlp(b *testing.B) {
+	baseLogs := NewLogs()
+	fillTestResourceLogsSlice(baseLogs.ResourceLogs())
+	buf, err := baseLogs.ToOtlpProtoBytes()
+	require.NoError(b, err)
+	assert.NotEqual(b, 0, len(buf))
+	b.ResetTimer()
+	b.ReportAllocs()
+	for n := 0; n < b.N; n++ {
+		traces := NewLogs()
+		require.NoError(b, traces.FromOtlpProtoBytes(buf))
+		assert.Equal(b, baseLogs.ResourceLogs().Len(), traces.ResourceLogs().Len())
+	}
+}