Skip to content
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

[WIP] Proposal: Move log.Value et al to cmplxattr module #6168

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pellared
Copy link
Member

@pellared pellared commented Jan 16, 2025

Per Slack thread: https://cloud-native.slack.com/archives/C062HUREGUV/p1736545245331779

Related to #6158

I know that some stuff is not building but I think this is good enough for discussion purposes.

@pellared pellared self-assigned this Jan 16, 2025
@pellared
Copy link
Member Author

pellared commented Jan 16, 2025

Initial feedback from the SIG meeting:

We would need to clearly define what "ComplexAttributes" (cmplxattr) is going to solve

If it is about adding support for nested attributes for new signals (e.g. for profiling, audit logging) then it may be acceptable. However, having two common packages for attributes can lead to confusion. Probably having each API defining its own "attributes" types is safer for backwards and forward compatibility. On the other hand, it hurts re-usability but the same is also true if we would have both cmplxattr and attribute packages. It would be only truly reusable if there is a single attributes abstraction.

If it is about adding support for nested attributes in existing signals (e.g. tracing) then it would be better to extend the standard attributes to avoid confusion and API design flaws (an API accepting both extended and standard attributes will be confusing for the user). However, it does not seem any longer acceptable per open-telemetry/opentelemetry-specification#3858

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

1 participant