Investigate if adding new types to common::AttributeValue breaks ABI compatibility

[maxgolov]

There are several types that are presently missing in OpenTelemetry typing system. For example, blob or byte array - aka uint8_t[] . These types are available in other systems, e.g. MsgPack, ETW, etc.. However, not defined in OpenTelemetry spec.

Related spec issue that is advocating for addition of byte arrays:
open-telemetry/opentelemetry-specification#780

One problem we face is that the code that deals with common::AttributeValue may require a MAJOR version update and a hassle of reworking the public API, if we freeze it in time at v1 and it turns out that we cannot safely enhance it by maintaining ABI stability guarantee.

It seems like this would have worked, as in - it would have worked forward-compatible with C++11 variant using anonymous union . But need to explore what happens precisely with C++ exported function / method signatures when we add new type.

In other words, (our optimistic) expectation is that adding new byte array type to common::AttributeValue in SDK v1.2 should work, and should not break the apps that were compiled against SDK v1.1. The goal of this issue is to verify: compile the SDK with new types and compare using ABI checker, e.g. this one:
https://lvc.github.io/abi-compliance-checker/

[pyohannes]

I think the biggest problems here might lie with the usage of nostd::visit. Here are the requirements for the argument to nostd::visit:

a Callable that accepts every possible alternative from every variant

When we add a new type to a variant, recompilation might fail for certain usages of nostd::visit (so technically this even breaks API compatibility).

[lalitb]

As of now, opentelemetry proto supports these types:
https://github.com/open-telemetry/opentelemetry-proto/blob/286810dc20d40f6483abf719f2b8de28f543fc78/opentelemetry/proto/common/v1/common.proto#L27

message AnyValue {
// The value is one of the listed fields. It is valid for all values to be unspecified
// in which case this AnyValue is considered to be "null".
oneof value {
string string_value = 1;
bool bool_value = 2;
int64 int_value = 3;
double double_value = 4;
ArrayValue array_value = 5;
KeyValueList kvlist_value = 6;
}
}

Any changes in opentelemetry-proto will be directly affecting implementation across all the languages, and so I don't foresee them adding any new data-type in above after bumping up MAJOR version.

[maxgolov]

Confirmed that it will break ABI compat. For that reason @pyohannes kept back the reserved types for future use, so we don't need to adjust the API surface when the types get added to the spec in future. There is also a precedent of C# SDK supporting some extra types already. So we should be good for now. I'm gonna close this issue, since all types we immediately need have been kept on API.