Clarify string content must be valid Unicode sequences

[XSAM]

Fixes #3950

Changes

• Limit primitive string value to UTF-8.
• Drop attribute mapping for non-UTF-8 strings.

[pyohannes]

Does this mean that any API which accepts string attribute values that are not valid UTF-8 strings doesn't comply with the specification?

[pellared]

.NET uses UTF-16 to encode the text in a string. I do not think it is a problem given it is properly encoded to OTLP.

[pyohannes]

I'm not sure about all languages, but Go and C++ allow arbitrary bytes sequences in default string types (string and std::string) which are used in the OpenTelemetry API.

[XSAM]

I changed the limitation to valid Unicode sequences.

BTW, the protobuf only encodes the string as UTF-8.

string := valid UTF-8 string (e.g. ASCII);

https://protobuf.dev/programming-guides/encoding/

---

I'm not sure about all languages, but Go and C++ allow arbitrary bytes sequences in default string types (string and std::string) which are used in the OpenTelemetry API.

I am not sure how C++ handles this. But for Go, since protobuf won't allow arbitrary bytes in string type, the string will be dropped silently if users put arbitrary bytes into the string type. Related issue: open-telemetry/opentelemetry-go#2314

[pyohannes]

I am not sure how C++ handles this. But for Go, since protobuf won't allow arbitrary bytes in string type, the string will be dropped silently if users put arbitrary bytes into the string type.

Besides the issue of (silent) data loss, I'd argue that this change would technically cause the OTel Go API to violate the specification, as it allows users to pass non-Unicode attribute values in API functions.

[yurishkuro]

Breaking change to the spec. Why should it be accepted? What's wrong with the existing instructions that are being deleted?

[jack-berg]

This document is experimental (somewhat surprisingly).

[yurishkuro]

Doesn't answer my question - still a breaking change. The existing wording provided lossless preservation of data.

[pellared]

I find it is language-specific how the string is encoded.

[tigrannajaryan]

We have discussed this in one of the Otel meetings a week or so ago (probably spec meeting, I can't remember exactly).

The conclusion was that the spec intentionally leaves this unspecified since string implementations are language specific and what is a valid string is different from language to language.

I believe we are not going to mandate Unicode since:

• It is a breaking change
• Enforcing it may be expensive in some languages

Blocking this since I don't think the change is desirable.

What we may want to do instead is specify more precisely how OTLP exporter deals with strings that are not valid Unicode, since Protobuf strings need to be unicode. This may need to be done on the https://github.com/open-telemetry/opentelemetry-proto repo.

[XSAM]

I believe we are not going to mandate Unicode

• Enforcing it may be expensive in some languages

I agree.

What we may want to do instead is specify more precisely how OTLP exporter deals with strings that are not valid Unicode, since Protobuf strings need to be unicode.

This is good, but I am not sure it solves all the problems. I assume we want to provide a uniform set of features that languages can all implement, meaning if a language can provide this feature, others should be able to provide the same one.

Assuming we converted invalid Unicode to bytes in the OTLP exporter to satisfy Protobuf, which is the current design, we allow certain languages to push bytes in protobuf (if this language allows arbitrary bytes in string) while others cannot do this (if this language does not allow arbitrary bytes in string).

That means to populate bytes in protobuf, we have to use only certain languages other than all languages to implement them. This breaks having a uniform set of features that languages can all implement.

[tigrannajaryan]

This breaks having a uniform set of features that languages can all implement.

I don't think we aim for uniformness at all costs. Otel mostly tries to be uniform when it is reasonable to be so. Yes, I know it is subjective.

[XSAM]

This breaks having a uniform set of features that languages can all implement.

I don't think we aim for uniformness at all costs. Otel mostly tries to be uniform when it is reasonable to be so. Yes, I know it is subjective.

I know we have discussed not adding the bytes type to the Attribute, but regarding the costs. Is adding the bytes type to the Attribute costly? By adding the bytes type, we can still provide uniformness.

[tigrannajaryan]

I know we have discussed not adding the bytes type to the Attribute, but regarding the costs. Is adding the bytes type to the Attribute costly? By adding the bytes type, we can still provide uniformness.

No, I don't think it is costly. I would be in favour of adding bytes to standard attributes.

[jack-berg]

No, I don't think it is costly. I would be in favour of adding bytes to standard attributes.

Its considered breaking: https://github.com/open-telemetry/opentelemetry-specification/blame/a03382ada8afa9415266a84dafac0510ec8c160f/specification/common/README.md#L84-L85

[pyohannes]

Its considered breaking: https://github.com/open-telemetry/opentelemetry-specification/blame/a03382ada8afa9415266a84dafac0510ec8c160f/specification/common/README.md#L84-L85

The linked note says:

Note: Extending the set of standard attribute value types is a breaking change.

And one of the following arguments reads:

Limiting attribute value types to primitives and arrays of primitives simplifies data consumers' efforts to create search indexes and perform statistical analysis.

In my understanding, it should be a breaking change to add any attribute value type that's not a primitive or array of primitives, but extending the list of supported primitives should not be a breaking change.

I wonder if others share this understanding, the wording of just the note itself could indeed be interpreted in different ways.

[tigrannajaryan]

Yes, my thinking was that bytes is just another primitive type. No strong opinion, but I think it does not violate the original goal of the restriction, which was to avoid complicating the life of receivers by introducing complex types.
If we don't think the spec allows this, then I am fine with that restriction, I don't intend to reopen that discussion.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.