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.

Sign up for GitHub

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

Jump to bottom

Implementations SHOULD preserve the order in which attributes are set #999

Closed
codeboten opened this issue Aug 17, 2020 · 4 comments
Closed

Implementations SHOULD preserve the order in which attributes are set #999

codeboten opened this issue Aug 17, 2020 · 4 comments
Labels
good first issue Good first issue help wanted release:required-for-ga To be resolved before GA release sdk Affects the SDK package.

Comments

@codeboten
Copy link
Contributor

To be compliant with the tracing spec, Implementations SHOULD preserve the order in which attributes are set

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#set-attributes
@codeboten codeboten added good first issue Good first issue sdk Affects the SDK package. help wanted release:required-for-ga To be resolved before GA release labels Aug 17, 2020
@ocelotl ocelotl mentioned this issue Aug 17, 2020
Closed
@lzchen
Copy link
Contributor

lzchen commented Aug 24, 2020

I don't see this as a requirement for attributes?

@ffe4
Copy link
Contributor

ffe4 commented Aug 25, 2020

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/common/common.md#attributes

Attributes SHOULD preserve the order in which they're set.

@ffe4
Copy link
Contributor

ffe4 commented Sep 2, 2020

Attributes in sdk.trace.Span are implemented as a BoundedDict which is already ordered. What else do we need to do here?

@ocelotl
Copy link
Contributor

ocelotl commented Sep 2, 2020

I think we are ok with the current implementation. The order of the attributes is being used to select which key to override (the oldest one) when the maximum size of the dictionary is reached.

@lzchen lzchen closed this as completed Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good first issue help wanted release:required-for-ga To be resolved before GA release sdk Affects the SDK package.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@codeboten @ocelotl @lzchen @ffe4