Skip to content

Conversation

@pellared
Copy link
Member

@pellared pellared commented May 22, 2025

Prototype for open-telemetry/opentelemetry-specification#4485

This PR is to validate that it is possible to extend attributes with complex attributes without making breaking changes and making sure that Value is still comparable.

It does not implement everything. I was just focused how we are able to handle the following new attribute value types in a backwards compatible way:

  • empty value
  • heterogeneous array
  • (heterogeneous) map
  • I was too lazy to implement bytes

This PR does not update the code dependent on attributes package. Exporters do not export complex attributes and the Logs API do not use this package. Nothing prevents doing it and I think this is not necessary to validate the OTEP.

@pellared pellared added the Skip Changelog PRs that do not require a CHANGELOG.md entry label May 22, 2025
@pellared
Copy link
Member Author

pellared commented May 22, 2025

@open-telemetry/go-maintainers, do you think that there anything more that I should add/check in order to validate the open-telemetry/opentelemetry-specification#4485?

@pellared pellared changed the title [Prototype] attribute: add complex attributes [DO NOT MERGE] [Prototype] attribute: add complex attributes May 22, 2025
@pellared pellared marked this pull request as ready for review May 22, 2025 12:17
@pellared pellared changed the title [DO NOT MERGE] [Prototype] attribute: add complex attributes [DO NOT MERGE] [Prototype] attribute: add complex value types May 22, 2025
@pellared
Copy link
Member Author

SIG meeting:
Looks good for the Go SIG.
The only feedback was that it would be safer to have EMPTY a new type (distinct from INVALID) and creating empty values would require "factory functions".

@pellared pellared changed the title [DO NOT MERGE] [Prototype] attribute: add complex value types [Prototype] attribute: add complex value types May 22, 2025
@pellared pellared marked this pull request as draft May 22, 2025 17:35
}

// MapValue creates a MAP Value.
// v is sorted by key.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also deduplicate keys, but this is an implementation detail not relevant for the prototype.

//
// If creating both a key and value at the same time, use the provided
// convenience function instead -- Slice(name, value).
func (k Key) Slice(v []Value) KeyValue {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be better called Array to match the OTel Spec.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😁 For reference only, a recommendation from AI: Sequence.

I consulted AI to obtain an answer based on a neutral stance across multiple languages.

https://chatgpt.com/share/68beda19-2a1c-800c-8eb6-0e31bf7a8170 —— The content is in Chinese; please translate it into English yourself for reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants