Add details about Sampler interface and sampling decision. #186
Conversation
trace/Sampling.md
Outdated
| * The current `TraceId`; | ||
| * The current `SpanId`; | ||
| * The current `Span`'s name; | ||
| * The current `Span`'s kind; |
There was a problem hiding this comment.
Some implementations also have HasRemoteParent, e.g. https://godoc.org/go.opencensus.io/trace#SamplingParameters. I don't have the historical background what it is but FYI.
There was a problem hiding this comment.
After we added the Span.kind that argument is not that useful. We need to decide if we still need it. As the current text mention the least can include more sampling params if needed.
| #### Sampling parameters | ||
| Based on the language the sampling parameters can be embedded into an object called | ||
| `SamplingParameters` or explicitly passed as arguments. The set of sampling parameters SHOULD | ||
| include at least: |
There was a problem hiding this comment.
what's the mechanism for sampler to set a sampling flag for downstream components? Or sampling score be added in tracestate? Or who's responsibility is it? Should propagator deal with this? Worth mentioning here
There was a problem hiding this comment.
I feel it is a race condition here, what is the order of these events:
- Modify the Tracestate
- Call sampler.
If we call sampler first, we cannot use the current Tracestate (only parent Tracestate) in the decision. If we create the Tracestate first we cannot modify it after the Sampler is called (or I guess we can create it again).
What is your thought on the order of these two events?
There was a problem hiding this comment.
Is there a good document explaining ordering of calls? @lmolkova, you had a picture I remember, can you send it to me pls?
Sampler may take tracestate object as an argument and modify when wants to.
One may also need to read and modify tracestate before span is created. See #131. This makes matter even more complicated
There was a problem hiding this comment.
@lmolkova any update on this?
There was a problem hiding this comment.
I don't have any pictures about it.
My take on this:
tracing system should have a chance to modify tracestate before sampler makes a decision to support scenarios where sampling is controlled with tracestate (mentioned by Sergey).
I don't see an obvious scenario when tracestate is modified after sampling decision is made or span is started. Do we really want to support this?
Tracing system is the only thing that should touch tracestate and it should have just one opportunity to do so: after span is created, so I don't see how we can have a race condition here.
There was a problem hiding this comment.
So I think what I called "race condition" is not the best term. Here are the sequence of events that I can see in time order:
- Tracestate is modified before the sampler gets called as you suggested.
- Sampler gets called.
- Tracestate is modified with sampling results - I heard from @SergeyKanzhelev that here some systems may want to modify the trace state with results from the sampling
Do I miss something?
There was a problem hiding this comment.
I also often accidentally say "race condition" when mean "chicken egg"
Though I wonder if tracestate needs to be exposed in-process or not. For example, if it is only expected to be written on remote links, is it necessary for us to write our own here? For example, it seems necessary to write upstream, but possibly not to write our own trace state until the edge where it is exported to headers..
There was a problem hiding this comment.
@SergeyKanzhelev could you please elaborate on the scenarios where sampler needs to update tracestate after sampling decision? (once you are back from vacation)
what's the mechanism for sampler to set a sampling flag for downstream components? Or sampling score be added in tracestate?
Is sampling score a result of sampling decision or parameter sampler takes or both? This is a chichen-egg problem for sure.
@adriancole one of scenarios for tracestate requires in-process exposure and modification. Some systems want to keep (and update) legacy ids at least for a while. assuming we can only access tracestate on the boundaries, all spans in the middle (if any) will not have legacy ids and it'll be hard to discover parents.
There was a problem hiding this comment.
@lmolkova @SergeyKanzhelev any conclusion on this? I would like to finish this document.
|
@adriancole <https://github.com/adriancole> one of scenarios for
tracestate requires in-process exposure and modification. Some systems want
to keep (and update) legacy ids at least for a while. assuming we can only
access tracestate on the boundaries, all spans in the middle (if any) will
not have legacy ids and it'll be hard to discover parents.
ok. thanks. thinking about this.. we have some scenarios like this but it
isnt a user api thing rather an SPI one. I personally would not like to
expose a mutable api for trace state to in routine APIs, but this is
different than wanting to allow hooks to be developed.
… |
|
PS I think reserializing the tracestate into a string for every
intermediate span will be unnecessary overhead for a lot of folks. We
already imply a lot of overhead to eagerly serialize traceparent.
Consider that between an entry and exit we have arbitrary number of
intermediate steps.
I think this is another case of something we are trying to go to spec
about with very little practice. Is there a way libraries can just
experiment and see if they really need something *before* it goes into
a spec?
…On Fri, Oct 12, 2018 at 11:50 AM Adrian Cole ***@***.***> wrote:
>
> @adriancole one of scenarios for tracestate requires in-process exposure and modification. Some systems want to keep (and update) legacy ids at least for a while. assuming we can only access tracestate on the boundaries, all spans in the middle (if any) will not have legacy ids and it'll be hard to discover parents.
ok. thanks. thinking about this.. we have some scenarios like this but it isnt a user api thing rather an SPI one. I personally would not like to expose a mutable api for trace state to in routine APIs, but this is different than wanting to allow hooks to be developed.
|
No description provided.