SamplingPriority refactor and support for UserKeep and UserDrop#59
SamplingPriority refactor and support for UserKeep and UserDrop#59willgittoes-dd merged 12 commits intomasterfrom
Conversation
…ibiity of the Trace, not the Span(Context)
…is change that was refactored
willgittoes-dd
force-pushed
the
willgittoes-dd/user-sampling
branch
from
0b32a70 to
71558f4
Compare
cgilmour
left a comment
cgilmour
left a comment
There was a problem hiding this comment.
Approving.
A couple of questions:
Should anything be done for the tag sampling.priority?
Is important information to look up the priority lost when the traces are flushed?
|
Re: sampling.priority, YES! Since that's what rnburn asked for. Re: the second question, not sure what you mean. |
|
… this. Also ensures noop span is a Datadog span.
|
Sampling priority tag added, please review the latest commit! |
| const std::string datadog_service_name_tag = "service.name"; | ||
| const std::string http_url_tag = "http.url"; | ||
| const std::string operation_name_tag = "operation"; | ||
| const std::string priority_sampling_tag = "sampling.priority"; |
There was a problem hiding this comment.
The tag is defined by opentracing in opentracing/ext/tags.h.
|
|
||
| // There's two ways we can set the sampling priority. Either directly using the method, or | ||
| // through a tag. Test both. | ||
| auto setSamplingPriority = GENERATE( |
There was a problem hiding this comment.
yeah innit?!
| if (p != nullptr) { | ||
| span->SetTag("sampling.priority", static_cast<int>(*p)); | ||
| } else { | ||
| span->SetTag("sampling.priority", ""); |
There was a problem hiding this comment.
That's what this does. So in addition to the spec of "0 = discard, anything else = keep" I'm adding a way to unset the sampling priority, by passing in an empty string.
|
A few review comments. |
willgittoes-dd
force-pushed
the
willgittoes-dd/user-sampling
branch
from
f01bcb2 to
c6ab704
Compare
willgittoes-dd
force-pushed
the
willgittoes-dd/user-sampling
branch
from
c6ab704 to
8591887
Compare
|
Still approved 👍 |
2 participants
The feature added is UserKeep and UserDrop (although they cannot yet easily be set by a user without compiling against our headers). The primary delta/change for this PR however, is a refactoring of how SamplingPriority is handled (set, assigned, and propagated).
Previously, SamplingPriority was a property of the SpanContext - which is not logical since it applies to a trace as a whole not a single span. However, it was there because it propagates via SpanContext. Now, SamplingPriority is handled at the Trace level, which means it is handled by the SpanBuffer, which contains the PendingTrace. When a PendingTrace completes, then the SamplingPriority is assigned (if relevant).
This should allow better behaviour of when users may apply UserKeep and UserDrop: at any time before a trace propagates or the root span finishes.
Also: