-
Notifications
You must be signed in to change notification settings - Fork 805
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
TraceState is mutated instead returning a new TraceState #1596
Comments
The use of
Therefore the caller of But the spec tells that What is the correct way to get an updated |
I think this is an issue with the spec, not our specific implementation of it. Fortunately for us, nothing in JS is truly immutable 😉. |
Spec tells From spec point of view it's allowed for a Propagator to create a new |
@dyladan The mutators must either return a new object (copying everything from the parent) or you can use a Builder pattern or something similar that is idiomatic in JS |
So in short to get my changes injected I have to:
I think the last step is not possible as a |
After #1589 I believe step 3 is actually possible now. |
Ah yes, I looked into 0.11- |
Usually the TraceState should be changed when the Span is created not just before propagation, probably this is a bit of a unexpected behavior. |
@bogdandrutu As far as I understand spans are created by calling Therefore I have to either "fake" a parent span with the new Another options would be to use a custom Or did I miss something? |
According to spec
TraceState
should be immutable and all mutating operations MUST return a newTraceState
.The actual implementation of e.g.
set
mutates theTraceState
and returnsvoid
.This has impact to the API which defines the interface for
set
to returnvoid
.I think
TraceState
should be implemented similar thenContext
to create a new instance and copy key/values first.The text was updated successfully, but these errors were encountered: