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

Revisit current ptrace.TraceState API #6022

Closed
2 tasks
bogdandrutu opened this issue Sep 4, 2022 · 3 comments · Fixed by #6052
Closed
2 tasks

Revisit current ptrace.TraceState API #6022

bogdandrutu opened this issue Sep 4, 2022 · 3 comments · Fixed by #6052
Assignees
@dmitryax
Milestone
go.opentelemetry....

Comments

@bogdandrutu
Copy link
Member

  • Move to pcommon? Reason to do it is because the ids are already part of the pcommon and most likely this will become part of logs and metrics exemplars.
  • Right now is a typedef to string. If we intend to have the type doing parsing, we may look into something like what otel-go has defined for this. We can start by just making this a proper struct and do not expose implicit conversions to/from string as we do right now because of the typedef, but have AsRaw and FromRaw.
bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Sep 12, 2022
@bogdandrutu
Replace immutable TraceState with a mutable struct
157f5d1 
Fixes open-telemetry#6022

Main motivation is that we may want to allow mutating (adding an entry to the state).

Signed-off-by: Bogdan <[email protected]>
bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Sep 12, 2022
@bogdandrutu
Move TraceState to pcommon, expect usages similar with ids
a3eb58c 
Fixes open-telemetry#6022

Will keep the type immutable and will offer mutatore similar with Flags "WithEntry" which will return a new TraceState.

Signed-off-by: Bogdan <[email protected]>
bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Sep 12, 2022
@bogdandrutu
Replace immutable TraceState with a mutable struct
7f46612 
Fixes open-telemetry#6022

Main motivation is that we may want to allow mutating (adding an entry to the state).

Signed-off-by: Bogdan <[email protected]>
This was referenced Sep 12, 2022
Merged
Closed
@bogdandrutu
Copy link
Member Author

Will have to pick one solution between #6053 and #6052

bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Sep 13, 2022
@bogdandrutu
Move TraceState to pcommon, expect usages similar with ids
aec19a6 
Fixes open-telemetry#6022

Will keep the type immutable and will offer mutatore similar with Flags "WithEntry" which will return a new TraceState.

Signed-off-by: Bogdan <[email protected]>
@dmitryax
Copy link
Member

I think #6052 is a better option providing extensibility that possibly can be utilized

@bogdandrutu
Copy link
Member Author

@jpkrohling also vote for the same. So will move forward with that proposal. Will close the alternate PR.

bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Sep 13, 2022
@bogdandrutu
Replace immutable TraceState with a mutable struct
fa275eb 
Fixes open-telemetry#6022

Main motivation is that we may want to allow mutating (adding an entry to the state).

Signed-off-by: Bogdan <[email protected]>
bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Sep 13, 2022
@bogdandrutu
Replace immutable TraceState with a mutable struct
a31408e 
Fixes open-telemetry#6022

Main motivation is that we may want to allow mutating (adding an entry to the state).

Signed-off-by: Bogdan <[email protected]>
bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Sep 13, 2022
@bogdandrutu
Replace immutable TraceState with a mutable struct
e0d3b45 
Fixes open-telemetry#6022

Main motivation is that we may want to allow mutating (adding an entry to the state).

Signed-off-by: Bogdan <[email protected]>
bogdandrutu added a commit to bogdandrutu/opentelemetry-collector that referenced this issue Sep 13, 2022
@bogdandrutu
Replace immutable TraceState with a mutable struct
4a57357 
Fixes open-telemetry#6022

Main motivation is that we may want to allow mutating (adding an entry to the state).

Signed-off-by: Bogdan <[email protected]>
bogdandrutu added a commit that referenced this issue Sep 13, 2022
@bogdandrutu @dmitryax
Replace immutable TraceState with a mutable struct (#6052)
a95999c 
* Replace immutable TraceState with a mutable struct

Fixes #6022

Main motivation is that we may want to allow mutating (adding an entry to the state).

Signed-off-by: Bogdan <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Dmitrii Anoshin <[email protected]>

Signed-off-by: Bogdan <[email protected]>
Co-authored-by: Dmitrii Anoshin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dmitryax dmitryax

Labels
None yet
Projects
None yet
Milestone
go.opentelemetry.io/collector/pdata 1.0
2 participants
@bogdandrutu @dmitryax