AttributeValues accepting a list could lead to invalid types being added

[toumorokoshi]

#348 is tackling validation for attribute values, of which list is one of them. However, lists can be mutated as long as someone has a handle, so one could potentially misuse the Span api by mutating lists afterward.

The purpose of adding validation during the setting of span attributes was to remove the need to perform such validations in the exporters themselves.

Should we freeze lists as they come in? or should we potentially just allow tuples rather than lists as attribute values?

[ocelotl]

We could accept any sequence and copy all of its values in a tuple when we store them.

Does the OpenTelemetry specification says anything about sequences that hold sequences themselves?

[jakemalachowski]

@ocelotl I was wondering that when implementing attribute validation. The OT spec says:

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.

Since the array values must all be of a primitive values, this seems to indicate that a list of lists is invalid.

[ocelotl]

Correct, there should not be sequences of sequences. 👍

[Oberon00]

I don't think we need to make the SDK completely fool-proof. Checking list values on every set_attribute sounds rather expensive.

[toumorokoshi]

@Oberon00 looks like you're looking at #348, which is a good venue to discuss the choice of validation at all.

looks like there's roughly a 183ns cost to including this in the trace construction, at the gain of immutability. I think there's merit to make this change:

# On an Intel i7-3820
$ python -m timeit "tuple(['a','b','c'])"
10000000 loops, best of 3: 0.183 usec per loop

I'll put help wanted on this, as it's a good starter task.

[AndrewAXue]

I can take this

[AndrewAXue]

Created a PR here, PTAL https://github.com/open-telemetry/opentelemetry-python/pull/678/files