-
Notifications
You must be signed in to change notification settings - Fork 926
Add empty value attribute #4595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
CC @open-telemetry/go-maintainers |
Co-authored-by: Tyler Yahn <[email protected]>
@@ -83,21 +81,6 @@ reflects that LogRecord attributes are expected to model data produced from | |||
external log APIs, which do not necessarily have the same value type | |||
restrictions as the standard attribute definition. | |||
|
|||
Note: Extending the set of standard attribute value types is a breaking change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to start with proto changes and replace comments like https://github.com/open-telemetry/opentelemetry-proto/blob/be5d58470429d0255ffdd49491f0815a3a63d6ef/opentelemetry/proto/trace/v1/trace.proto#L209-L213
// The OpenTelemetry API specification further restricts the allowed value types:
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/README.md#attribute
// Attribute keys MUST be unique (it is not allowed to have more than one
// attribute with the same key).
we probably need to replace it with comment that they are going to be allowed in 6 months
opentelemetry-specification/oteps/4485-extending-attributes-to-support-complex-values.md
Lines 111 to 112 in 8aa1d27
- Stable exporters will be prohibited from emitting complex attributes by default on signals | |
other than Logs until at least 6 months after this OTEP is merged. |
Or, at least, leave a note here that complex attributes are coming and their support is in development, nobody should stabilize them in the next 6 month.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably need to replace it with comment that they are going to be allowed in 6 months
I feel like this is an orthogonal PR given the changes of https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/README.md#attribute are marked with Development status.
Therefore, I think a PR in proto repo can be done in parallel. Are you able to help with it?
Related OTEP: https://github.com/open-telemetry/opentelemetry-specification/blob/main/oteps/4485-extending-attributes-to-support-complex-values.md
To be precise:
opentelemetry-specification/oteps/4485-extending-attributes-to-support-complex-values.md
Lines 40 to 48 in 8aa1d27
This is necessary to be able to represent an empty (that is the default) Log Record Body using "Standard Attributes".
This is needed towards stabilization of OTel Go Logs API and SDK (we need it "stable" though; not only "development"); more:
It is also worth mentioning that it is already allowed by OTLP:
Prototype:
Also fixes #4468
as it removes the portion which said that extending the attribute types is a breaking change as it was already approved via #4485.