Update label.ArrayValue to store copies of 1D arrays (#1226)

* Update label.ArrayValue to store copies of 1D arrays

* Update OTLP transform of array attributes

* Add changes to CHANGELOG

* Add PR number to changes

* Update value function documentation

* Remove redundant checks

* Add Array test for invalid array types

* Add test to ensure return from AsArray is a type

* Clean up iteration

CHANGELOG.md

@@ -25,6 +25,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - The `StatusCode` field of the `SpanData` struct in the `go.opentelemetry.io/otel/sdk/export/trace` package now uses the codes package from this package instead of the gRPC project. (#1214)
 - Move the `go.opentelemetry.io/otel/api/baggage` package into `go.opentelemetry.io/otel/propagators`. (#1217)
 
+### Fixed
+
+- Copies of data from arrays and slices passed to `go.opentelemetry.io/otel/label.ArrayValue()` are now used in the returned `Value` instead of using the mutable data itself. (#1226)
+
 ### Removed
 
 - The `ExtractHTTP` and `InjectHTTP` fuctions from the `go.opentelemetry.io/otel/api/propagation` package were removed. (#1212)
@@ -33,6 +37,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - The `SetAttribute` method of the `Span` from the `go.opentelemetry.io/otel/api/trace` package was removed given its redundancy with the `SetAttributes` method. (#1216)
 - The internal implementation of Baggage storage is removed in favor of using the new Baggage API functionality. (#1217)
 - Remove duplicate hostname key `HostHostNameKey` in Resource semantic conventions. (#1219)
+- Nested array/slice support has been removed. (#1226)
 
 ## [0.12.0] - 2020-09-24
 

exporters/otlp/internal/transform/attribute.go

@@ -15,6 +15,8 @@
 package transform
 
 import (
+	"reflect"
+
 	commonpb "go.opentelemetry.io/otel/exporters/otlp/internal/opentelemetry-proto-gen/common/v1"
 	"go.opentelemetry.io/otel/label"
 
@@ -75,7 +77,11 @@ func toAttribute(v label.KeyValue) *commonpb.KeyValue {
 			StringValue: v.Value.AsString(),
 		}
 	case label.ARRAY:
-		result.Value.Value = toArrayAttribute(v)
+		result.Value.Value = &commonpb.AnyValue_ArrayValue{
+			ArrayValue: &commonpb.ArrayValue{
+				Values: arrayValues(v),
+			},
+		}
 	default:
 		result.Value.Value = &commonpb.AnyValue_StringValue{
 			StringValue: "INVALID",
@@ -84,164 +90,56 @@ func toAttribute(v label.KeyValue) *commonpb.KeyValue {
 	return result
 }
 
-func toArrayAttribute(v label.KeyValue) *commonpb.AnyValue_ArrayValue {
-	array := v.Value.AsArray()
-	var resultValues []*commonpb.AnyValue
-
-	switch typedArray := array.(type) {
-	case []bool:
-		resultValues = getValuesFromBoolArray(typedArray)
-	case []int:
-		resultValues = getValuesFromIntArray(typedArray)
-	case []int32:
-		resultValues = getValuesFromInt32Array(typedArray)
-	case []int64:
-		resultValues = getValuesFromInt64Array(typedArray)
-	case []uint:
-		resultValues = getValuesFromUIntArray(typedArray)
-	case []uint32:
-		resultValues = getValuesFromUInt32Array(typedArray)
-	case []uint64:
-		resultValues = getValuesFromUInt64Array(typedArray)
-	case []float32:
-		resultValues = getValuesFromFloat32Array(typedArray)
-	case []float64:
-		resultValues = getValuesFromFloat64Array(typedArray)
-	case []string:
-		resultValues = getValuesFromStringArray(typedArray)
-	default:
-		resultValues = []*commonpb.AnyValue{
-			{
-				Value: &commonpb.AnyValue_StringValue{
-					StringValue: "INVALID",
+func arrayValues(kv label.KeyValue) []*commonpb.AnyValue {
+	a := kv.Value.AsArray()
+	aType := reflect.TypeOf(a)
+	var valueFunc func(reflect.Value) *commonpb.AnyValue
+	switch aType.Elem().Kind() {
+	case reflect.Bool:
+		valueFunc = func(v reflect.Value) *commonpb.AnyValue {
+			return &commonpb.AnyValue{
+				Value: &commonpb.AnyValue_BoolValue{
+					BoolValue: v.Bool(),
 				},
-			},
+			}
+		}
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+		valueFunc = func(v reflect.Value) *commonpb.AnyValue {
+			return &commonpb.AnyValue{
+				Value: &commonpb.AnyValue_IntValue{
+					IntValue: v.Int(),
+				},
+			}
+		}
+	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
+		valueFunc = func(v reflect.Value) *commonpb.AnyValue {
+			return &commonpb.AnyValue{
+				Value: &commonpb.AnyValue_IntValue{
+					IntValue: int64(v.Uint()),
+				},
+			}
+		}
+	case reflect.Float32, reflect.Float64:
+		valueFunc = func(v reflect.Value) *commonpb.AnyValue {
+			return &commonpb.AnyValue{
+				Value: &commonpb.AnyValue_DoubleValue{
+					DoubleValue: v.Float(),
+				},
+			}
+		}
+	case reflect.String:
+		valueFunc = func(v reflect.Value) *commonpb.AnyValue {
+			return &commonpb.AnyValue{
+				Value: &commonpb.AnyValue_StringValue{
+					StringValue: v.String(),
+				},
+			}
 		}
 	}
 
-	return &commonpb.AnyValue_ArrayValue{
-		ArrayValue: &commonpb.ArrayValue{
-			Values: resultValues,
-		},
+	results := make([]*commonpb.AnyValue, aType.Len())
+	for i, aValue := 0, reflect.ValueOf(a); i < aValue.Len(); i++ {
+		results[i] = valueFunc(aValue.Index(i))
 	}
-}
-
-func getValuesFromBoolArray(boolArray []bool) []*commonpb.AnyValue {
-	result := []*commonpb.AnyValue{}
-	for _, b := range boolArray {
-		result = append(result, &commonpb.AnyValue{
-			Value: &commonpb.AnyValue_BoolValue{
-				BoolValue: b,
-			},
-		})
-	}
-	return result
-}
-
-func getValuesFromIntArray(intArray []int) []*commonpb.AnyValue {
-	result := []*commonpb.AnyValue{}
-	for _, i := range intArray {
-		result = append(result, &commonpb.AnyValue{
-			Value: &commonpb.AnyValue_IntValue{
-				IntValue: int64(i),
-			},
-		})
-	}
-	return result
-}
-
-func getValuesFromInt32Array(int32Array []int32) []*commonpb.AnyValue {
-	result := []*commonpb.AnyValue{}
-	for _, i := range int32Array {
-		result = append(result, &commonpb.AnyValue{
-			Value: &commonpb.AnyValue_IntValue{
-				IntValue: int64(i),
-			},
-		})
-	}
-	return result
-}
-
-func getValuesFromInt64Array(int64Array []int64) []*commonpb.AnyValue {
-	result := []*commonpb.AnyValue{}
-	for _, i := range int64Array {
-		result = append(result, &commonpb.AnyValue{
-			Value: &commonpb.AnyValue_IntValue{
-				IntValue: i,
-			},
-		})
-	}
-	return result
-}
-
-func getValuesFromUIntArray(uintArray []uint) []*commonpb.AnyValue {
-	result := []*commonpb.AnyValue{}
-	for _, i := range uintArray {
-		result = append(result, &commonpb.AnyValue{
-			Value: &commonpb.AnyValue_IntValue{
-				IntValue: int64(i),
-			},
-		})
-	}
-	return result
-}
-
-func getValuesFromUInt32Array(uint32Array []uint32) []*commonpb.AnyValue {
-	result := []*commonpb.AnyValue{}
-	for _, i := range uint32Array {
-		result = append(result, &commonpb.AnyValue{
-			Value: &commonpb.AnyValue_IntValue{
-				IntValue: int64(i),
-			},
-		})
-	}
-	return result
-}
-
-func getValuesFromUInt64Array(uint64Array []uint64) []*commonpb.AnyValue {
-	result := []*commonpb.AnyValue{}
-	for _, i := range uint64Array {
-		result = append(result, &commonpb.AnyValue{
-			Value: &commonpb.AnyValue_IntValue{
-				IntValue: int64(i),
-			},
-		})
-	}
-	return result
-}
-
-func getValuesFromFloat32Array(float32Array []float32) []*commonpb.AnyValue {
-	result := []*commonpb.AnyValue{}
-	for _, f := range float32Array {
-		result = append(result, &commonpb.AnyValue{
-			Value: &commonpb.AnyValue_DoubleValue{
-				DoubleValue: float64(f),
-			},
-		})
-	}
-	return result
-}
-
-func getValuesFromFloat64Array(float64Array []float64) []*commonpb.AnyValue {
-	result := []*commonpb.AnyValue{}
-	for _, f := range float64Array {
-		result = append(result, &commonpb.AnyValue{
-			Value: &commonpb.AnyValue_DoubleValue{
-				DoubleValue: f,
-			},
-		})
-	}
-	return result
-}
-
-func getValuesFromStringArray(stringArray []string) []*commonpb.AnyValue {
-	result := []*commonpb.AnyValue{}
-	for _, s := range stringArray {
-		result = append(result, &commonpb.AnyValue{
-			Value: &commonpb.AnyValue_StringValue{
-				StringValue: s,
-			},
-		})
-	}
-	return result
+	return results
 }

exporters/otlp/internal/transform/attribute_test.go

@@ -156,6 +156,21 @@ func TestArrayAttributes(t *testing.T) {
 	// "uint", "uint32", "uint64"
 	for _, test := range []attributeTest{
 		{nil, nil},
+		{
+			[]label.KeyValue{
+				label.Array("invalid", [][]string{{"1", "2"}, {"a"}}),
+			},
+			[]*commonpb.KeyValue{
+				{
+					Key: "invalid",
+					Value: &commonpb.AnyValue{
+						Value: &commonpb.AnyValue_StringValue{
+							StringValue: "INVALID",
+						},
+					},
+				},
+			},
+		},
 		{
 			[]label.KeyValue{
 				label.Array("bool array to bool array", []bool{true, false}),
@@ -191,17 +206,20 @@ func TestArrayAttributes(t *testing.T) {
 
 		for i, actualArrayAttr := range actualArrayAttributes {
 			expectedArrayAttr := expectedArrayAttributes[i]
-			if !assert.Equal(t, expectedArrayAttr.Key, actualArrayAttr.Key) {
+			expectedKey, actualKey := expectedArrayAttr.Key, actualArrayAttr.Key
+			if !assert.Equal(t, expectedKey, actualKey) {
 				continue
 			}
 
-			expectedArrayValue := expectedArrayAttr.Value.GetArrayValue()
-			assert.NotNil(t, expectedArrayValue)
-
-			actualArrayValue := actualArrayAttr.Value.GetArrayValue()
-			assert.NotNil(t, actualArrayValue)
-
-			assertExpectedArrayValues(t, expectedArrayValue.Values, actualArrayValue.Values)
+			expected := expectedArrayAttr.Value.GetArrayValue()
+			actual := actualArrayAttr.Value.GetArrayValue()
+			if expected == nil {
+				assert.Nil(t, actual)
+				continue
+			}
+			if assert.NotNil(t, actual, "expected not nil for %s", actualKey) {
+				assertExpectedArrayValues(t, expected.Values, actual.Values)
+			}
 		}
 
 	}

label/value.go

@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"reflect"
 	"strconv"
-	"strings"
 	"unsafe"
 
 	"go.opentelemetry.io/otel/internal"
@@ -93,7 +92,7 @@ func Int32Value(v int32) Value {
 	}
 }
 
-// Uint32 creates a UINT32 Value.
+// Uint32Value creates a UINT32 Value.
 func Uint32Value(v uint32) Value {
 	return Value{
 		vtype:   UINT32,
@@ -101,7 +100,7 @@ func Uint32Value(v uint32) Value {
 	}
 }
 
-// Float32 creates a FLOAT32 Value.
+// Float32Value creates a FLOAT32 Value.
 func Float32Value(v float32) Value {
 	return Value{
 		vtype:   FLOAT32,
@@ -109,7 +108,7 @@ func Float32Value(v float32) Value {
 	}
 }
 
-// String creates a STRING Value.
+// StringValue creates a STRING Value.
 func StringValue(v string) Value {
 	return Value{
 		vtype:    STRING,
@@ -117,7 +116,7 @@ func StringValue(v string) Value {
 	}
 }
 
-// Int creates either an INT32 or an INT64 Value, depending on whether
+// IntValue creates either an INT32 or an INT64 Value, depending on whether
 // the int type is 32 or 64 bits wide.
 func IntValue(v int) Value {
 	if unsafe.Sizeof(v) == 4 {
@@ -126,8 +125,8 @@ func IntValue(v int) Value {
 	return Int64Value(int64(v))
 }
 
-// Uint creates either a UINT32 or a UINT64 Value, depending on
-// whether the uint type is 32 or 64 bits wide.
+// UintValue creates either a UINT32 or a UINT64 Value, depending on whether
+// the uint type is 32 or 64 bits wide.
 func UintValue(v uint) Value {
 	if unsafe.Sizeof(v) == 4 {
 		return Uint32Value(uint32(v))
@@ -135,27 +134,32 @@ func UintValue(v uint) Value {
 	return Uint64Value(uint64(v))
 }
 
-// Array creates an ARRAY value.
-func ArrayValue(array interface{}) Value {
-	switch reflect.TypeOf(array).Kind() {
+// ArrayValue creates an ARRAY value from an array or slice.
+// Only arrays or slices of bool, int, int32, int64, uint, uint32, uint64,
+// float, float32, float64, or string types are allowed. Specifically, arrays
+// and slices can not contain other arrays, slices, structs, or non-standard
+// types. If the passed value is not an array or slice of these types an
+// INVALID value is returned.
+func ArrayValue(v interface{}) Value {
+	switch reflect.TypeOf(v).Kind() {
 	case reflect.Array, reflect.Slice:
-		isValidType := func() bool {
-			// get array type regardless of dimensions
-			typeName := reflect.TypeOf(array).String()
-			typeName = typeName[strings.LastIndex(typeName, "]")+1:]
-			switch typeName {
-			case "bool", "int", "int32", "int64",
-				"float32", "float64", "string",
-				"uint", "uint32", "uint64":
-				return true
-			}
-			return false
-		}()
-		if isValidType {
+		// get array type regardless of dimensions
+		typ := reflect.TypeOf(v).Elem()
+		kind := typ.Kind()
+		switch kind {
+		case reflect.Bool, reflect.Int, reflect.Int32, reflect.Int64,
+			reflect.Float32, reflect.Float64, reflect.String,
+			reflect.Uint, reflect.Uint32, reflect.Uint64:
+			val := reflect.ValueOf(v)
+			length := val.Len()
+			frozen := reflect.Indirect(reflect.New(reflect.ArrayOf(length, typ)))
+			reflect.Copy(frozen, val)
 			return Value{
 				vtype: ARRAY,
-				array: array,
+				array: frozen.Interface(),
 			}
+		default:
+			return Value{vtype: INVALID}
 		}
 	}
 	return Value{vtype: INVALID}

label/value_test.go

@@ -15,6 +15,7 @@
 package label_test
 
 import (
+	"reflect"
 	"testing"
 	"unsafe"
 
@@ -42,7 +43,7 @@ func TestValue(t *testing.T) {
 			name:      "Key.Array([]bool) correctly return key's internal bool values",
 			value:     k.Array([]bool{true, false}).Value,
 			wantType:  label.ARRAY,
-			wantValue: []bool{true, false},
+			wantValue: [2]bool{true, false},
 		},
 		{
 			name:      "Key.Int64() correctly returns keys's internal int64 value",
@@ -102,67 +103,70 @@ func TestValue(t *testing.T) {
 			name:      "Key.Array([]int64) correctly returns keys's internal int64 values",
 			value:     k.Array([]int64{42, 43}).Value,
 			wantType:  label.ARRAY,
-			wantValue: []int64{42, 43},
+			wantValue: [2]int64{42, 43},
 		},
 		{
 			name:      "KeyArray([]uint64) correctly returns keys's internal uint64 values",
 			value:     k.Array([]uint64{42, 43}).Value,
 			wantType:  label.ARRAY,
-			wantValue: []uint64{42, 43},
+			wantValue: [2]uint64{42, 43},
 		},
 		{
 			name:      "Key.Array([]float64) correctly returns keys's internal float64 values",
 			value:     k.Array([]float64{42, 43}).Value,
 			wantType:  label.ARRAY,
-			wantValue: []float64{42, 43},
+			wantValue: [2]float64{42, 43},
 		},
 		{
 			name:      "Key.Array([]int32) correctly returns keys's internal int32 values",
 			value:     k.Array([]int32{42, 43}).Value,
 			wantType:  label.ARRAY,
-			wantValue: []int32{42, 43},
+			wantValue: [2]int32{42, 43},
 		},
 		{
 			name:      "Key.Array([]uint32) correctly returns keys's internal uint32 values",
 			value:     k.Array([]uint32{42, 43}).Value,
 			wantType:  label.ARRAY,
-			wantValue: []uint32{42, 43},
+			wantValue: [2]uint32{42, 43},
 		},
 		{
 			name:      "Key.Array([]float32) correctly returns keys's internal float32 values",
 			value:     k.Array([]float32{42, 43}).Value,
 			wantType:  label.ARRAY,
-			wantValue: []float32{42, 43},
+			wantValue: [2]float32{42, 43},
 		},
 		{
 			name:      "Key.Array([]string) correctly return key's internal string values",
 			value:     k.Array([]string{"foo", "bar"}).Value,
 			wantType:  label.ARRAY,
-			wantValue: []string{"foo", "bar"},
+			wantValue: [2]string{"foo", "bar"},
 		},
 		{
 			name:      "Key.Array([]int) correctly returns keys's internal signed integral values",
 			value:     k.Array([]int{42, 43}).Value,
 			wantType:  label.ARRAY,
-			wantValue: []int{42, 43},
+			wantValue: [2]int{42, 43},
 		},
 		{
 			name:      "Key.Array([]uint) correctly returns keys's internal unsigned integral values",
 			value:     k.Array([]uint{42, 43}).Value,
 			wantType:  label.ARRAY,
-			wantValue: []uint{42, 43},
+			wantValue: [2]uint{42, 43},
 		},
 		{
-			name:      "Key.Array([][]int) correctly return key's multi dimensional array",
+			name:      "Key.Array([][]int) refuses to create multi-dimensional array",
 			value:     k.Array([][]int{{1, 2}, {3, 4}}).Value,
-			wantType:  label.ARRAY,
-			wantValue: [][]int{{1, 2}, {3, 4}},
+			wantType:  label.INVALID,
+			wantValue: nil,
 		},
 	} {
 		t.Logf("Running test case %s", testcase.name)
 		if testcase.value.Type() != testcase.wantType {
 			t.Errorf("wrong value type, got %#v, expected %#v", testcase.value.Type(), testcase.wantType)
 		}
+		if testcase.wantType == label.INVALID {
+			continue
+		}
 		got := testcase.value.AsInterface()
 		if diff := cmp.Diff(testcase.wantValue, got); diff != "" {
 			t.Errorf("+got, -want: %s", diff)
@@ -199,3 +203,11 @@ func getBitlessInfo(i int) bitlessInfo {
 		unsignedValue: uint64(i),
 	}
 }
+
+func TestAsArrayValue(t *testing.T) {
+	v := label.ArrayValue([]uint{1, 2, 3}).AsArray()
+	// Ensure the returned dynamic type is stable.
+	if got, want := reflect.TypeOf(v).Kind(), reflect.Array; got != want {
+		t.Errorf("AsArray() returned %T, want %T", got, want)
+	}
+}