Support for array of span attributes

[dackroyd]

Support for attribute arrays does not appear to be implemented (yet?), but is covered by the spec:

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#set-attributes

An Attribute is defined by the following properties:

• (Required) The attribute key, which MUST be a non-null and non-empty string.
• (Required) The attribute value, which is either:
  • A primitive type: string, boolean or numeric.
  • An array of primitive type values. The array MUST be homogeneous, i.e. it MUST NOT contain values of different types.

I'd imagine the API for this looking something like the following for each primitive type:

func Strings(k, v ...string) core.KeyValue {
	return New(k).Strings(v...)
}
...
func (k Key) Strings(v ...string) KeyValue {
	return KeyValue{
		Key:   k,
		Value: <slice>
	}
}

[stefanprisca]

Is this what you are looking for?

opentelemetry-go/sdk/trace/provider.go

Line 200 in 88100f0

func WithResourceAttributes(attrs ...core.KeyValue) ProviderOption {

[dackroyd]

I don't believe so? provider.WithResourceAttributes (like span.SetAttributes) allows multiple distinct key/values to be set, with values being overridden if there is a clash.

What I'm understanding for the 'array' support in the spec is that it should be possible for the values to accept primitive arrays for a single key, e.g:

val := []string{"bar1", "bar2", "bar3"}
span.SetAttributes(key.Strings("foo", val...))

open-telemetry/opentelemetry-specification#368

Array values are useful for representing attributes that can have
multiple values (e.g. representing a Memcached get that can be
over multiple keys and making the keys a span attribute).

This change allows homogeneous array values for span attributes
and specifies that array values should be JSON encoded string when
exporting using protocols that do not natively support array values.

This was added to the DotNet implementation back in February: open-telemetry/opentelemetry-dotnet#500

[MrAlias]

The ask from the SIG meeting today was to make this more universal:

• Allow nil, nulls.
• Allow all "primitive" types.
• Allow maps, slices, and maybe structs.

This might need to update the specification to be in compliance if the above are included.

[mujtaba-ahmed12]

@MrAlias I would like to work on this issue.

[mujtaba-ahmed12]

@MrAlias I am thinking of two ways of doing this.

1. Create generic method for array and check if its homogeneous array as done here in .NET implementation https://github.com/open-telemetry/opentelemetry-dotnet/pull/500/files#diff-61ad2bb28c6f75dcacf313e09a9b6488R840 and determine type at runtime.

// Array creates array Value of homogeneous type
func Array(collection []interface{}) Value {
	foreach := func(collection []interface{}) bool {
		for _, item := range collection {
			if reflect.TypeOf(collection[0]) != reflect.TypeOf(item) {
				return false
			}
		}
		return true
	}

	// Check if array is homogeneous or not
	if !foreach(collection) {
		return Value{
			vtype:   ARRAY,
			numbers: nil,
			strings: nil,
		}
	}

	if reflect.TypeOf(collection[0]).Kind() == reflect.String {
		return Value{
			vtype:   ARRAY_STRING,
			// Strings assignment
		}
	}

	return Value{
		vtype: ARRAY_<TYPE>,
		// Numeric Assignment
	}

}

2. Create separate method for each type.

[MrAlias]

@mujtaba-ahmed12:

I think you could try to leverage the Go type system a bit more which already guarantees slice/array homogeneity. I haven't worked through the full example, but I imagine something like this would suffice:

// Array creates array Value of homogeneous type
func Array(collection interface{}) Value {
	rt := reflect.TypeOf(v)
	switch rt.Kind() {
	case reflect.Slice, reflect.Array:
		// TODO: marshal slice/array
		// TODO: figure out what to do with multi-level values
	default:
		// TODO: return [Value] (?)
	}
	// ...
}

I would probably support multi-level slices/arrays here because the spec is a bit vague about this (primitive type is not well defined (is it language specific?)) and our value functions here are not setup for returning errors here.