opentelemetry-specification
opentelemetry-specification copied to clipboard
open-telemetry
Disallow control characters in instrument unit and description
Instrument units MUST be ASCII strings.
Instrument descriptions MUST support BMP (Unicode Plane 0), which is basically only the first three bytes of UTF-8.
Both of these allow control characters to be included. We should update the language to exclude control characters, which are not printable.
Both of these allow control characters to be included. We should update the language to exclude control characters, which are not printable.
The wording is intentional, because the SDK is treating both as opaque strings (the SDK doesn't perform any check):
It SHOULD be treated as an opaque string from the API and SDK (e.g. the SDK is not expected to validate the unit of measurement, or perform the unit conversion).
It MUST be treated as an opaque string from the API and SDK.
FYI this is what individual exporter implementation can choose to do https://github.com/open-telemetry/opentelemetry-dotnet/blob/0741a9de24d2ebb5fb65335335807c747aca6f3a/src/OpenTelemetry.Exporter.Prometheus/Implementation/PrometheusSerializer.cs#L260-L269.
It MUST be treated as an opaque string from the API and SDK.
That seemingly contradicts another part of the specification, which says the unit must be an ascii string with a maximum length of 63 characters. If validation is already taking place, why not also disallow control characters?
Would love to get some other opinions on this. To me, it also currently still reads as a contradiction, and I don't think that you can both require something to be opaque and also impose content restrictions. Even if you disagree with that or think it's nitpicky, do we really think that the API spec is the place to outline content validation? Doesn't that work better as an SDK requirement?
