-
Notifications
You must be signed in to change notification settings - Fork 412
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
Client sampling #8
Changes from 10 commits
39ac2d1
69b5673
d4f900b
b597b68
113097e
585d9fe
4ec676c
83715d0
2c865f1
8af16fd
ea55443
fc0983d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,21 +62,22 @@ def _finish_span(self, response=None, exception=None): | |
""" Close and finsh the active span if it exists. """ | ||
span = getattr(g, 'flask_datadog_span', None) | ||
if span: | ||
error = 0 | ||
code = response.status_code if response else None | ||
|
||
# if we didn't get a response, but we did get an exception, set | ||
# codes accordingly. | ||
if not response and exception: | ||
error = 1 | ||
code = 500 | ||
span.set_tag(errors.ERROR_TYPE, type(exception)) | ||
span.set_tag(errors.ERROR_MSG, exception) | ||
|
||
span.resource = str(request.endpoint or "").lower() | ||
span.set_tag(http.URL, str(request.base_url or "")) | ||
span.set_tag(http.STATUS_CODE, code) | ||
span.error = error | ||
if not span.sampled: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find it confusing, I'd rather see the spans "sampled" to be the one we keep, as per definition. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Absolutely right, switching them. |
||
error = 0 | ||
code = response.status_code if response else None | ||
|
||
# if we didn't get a response, but we did get an exception, set | ||
# codes accordingly. | ||
if not response and exception: | ||
error = 1 | ||
code = 500 | ||
span.set_tag(errors.ERROR_TYPE, type(exception)) | ||
span.set_tag(errors.ERROR_MSG, exception) | ||
|
||
span.resource = str(request.endpoint or "").lower() | ||
span.set_tag(http.URL, str(request.base_url or "")) | ||
span.set_tag(http.STATUS_CODE, code) | ||
span.error = error | ||
span.finish() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't this be in the if block as well ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The span already got created. Even if it is not sampled (and we didn't put the various attributes), we still have to finish it (to keep a valid tree). |
||
# Clear our span just in case. | ||
g.flask_datadog_span = None | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
from .span import MAX_TRACE_ID | ||
|
||
class Sampler(object): | ||
"""Sampler manages the client-side trace sampling | ||
|
||
Keep (100 * sample_rate)% of the traces. | ||
Any sampled trace should be entirely ignored by the instrumentation and won't be written. | ||
It samples randomly, its main purpose is to reduce the instrumentation footprint. | ||
""" | ||
|
||
def __init__(self, sample_rate): | ||
self.sample_rate = sample_rate | ||
self.sampling_id_threshold = sample_rate * MAX_TRACE_ID | ||
|
||
def should_sample(self, span): | ||
return span.trace_id >= self.sampling_id_threshold |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think this should be mandatory. in fact, people might depend on it for logging, etc. if it were me, sampling would only be for submission.