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.

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

Enable priority sampling by default #774

Merged
merged 7 commits into from Dec 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions ddtrace/tracer.py
Expand Up @@ -98,7 +98,7 @@ def configure(self, enabled=None, hostname=None, port=None, sampler=None,
``Tracer.wrap()``. This is an advanced option that usually doesn't need to be changed
from the default value
:param priority_sampling: enable priority sampling, this is required for
complete distributed tracing support.
complete distributed tracing support. Enabled by default.
"""
if enabled is not None:
self.enabled = enabled
Expand All @@ -110,8 +110,12 @@ def configure(self, enabled=None, hostname=None, port=None, sampler=None,
if sampler is not None:
self.sampler = sampler

if priority_sampling:
# If priority sampling is not set or is True and no priority sampler is set yet
if priority_sampling in (None, True) and not self.priority_sampler:
brettlangdon marked this conversation as resolved.
Show resolved Hide resolved
self.priority_sampler = RateByServiceSampler()
# Explicitly disable priority sampling
elif priority_sampling is False:
self.priority_sampler = None

if hostname is not None or port is not None or filters is not None or \
priority_sampling is not None:
Expand Down
13 changes: 5 additions & 8 deletions docs/advanced_usage.rst
Expand Up @@ -128,13 +128,10 @@ priority to the following values:
- ``AUTO_REJECT``: the sampler automatically rejects the trace
- ``AUTO_KEEP``: the sampler automatically keeps the trace

Priority sampling is disabled by default. Enabling it ensures that your sampled
distributed traces will be complete. To enable priority sampling::

tracer.configure(priority_sampling=True)

Once enabled, the sampler will automatically assign a priority to your traces,
Priority sampling is enabled by default.
When enabled, the sampler will automatically assign a priority to your traces,
depending on their service and volume.
This ensures that your sampled distributed traces will be complete.
palazzem marked this conversation as resolved.
Show resolved Hide resolved

You can also set this priority manually to either drop an uninteresting trace or
to keep an important one.
Expand Down Expand Up @@ -323,7 +320,7 @@ for usage.
+---------------------+---------------------------------------------------------+---------------+
| `sampler` | see `Sampling`_ | `AllSampler` |
+---------------------+---------------------------------------------------------+---------------+
| `priority_sampling` | see `Priority Sampling`_ | `False` |
| `priority_sampling` | see `Priority Sampling`_ | `True` |
+---------------------+---------------------------------------------------------+---------------+
| `settings` | see `Advanced Usage`_ | `{}` |
+---------------------+---------------------------------------------------------+---------------+
Expand Down Expand Up @@ -448,7 +445,7 @@ The available environment variables for ``ddtrace-run`` are:
``localhost``)
* ``DATADOG_TRACE_AGENT_PORT=8126``: override the port that the default tracer
will submit to (default: 8126)
* ``DATADOG_PRIORITY_SAMPLING`` (default: false): enables :ref:`Priority
* ``DATADOG_PRIORITY_SAMPLING`` (default: true): enables :ref:`Priority
Sampling`

``ddtrace-run`` respects a variety of common entrypoints for web applications:
Expand Down
20 changes: 17 additions & 3 deletions tests/contrib/grpc/test_grpc.py
Expand Up @@ -45,7 +45,14 @@ def test_insecure_channel(self):
spans = writer.pop()
eq_(len(spans), 1)
span = spans[0]
eq_(response.message, 'x-datadog-trace-id=%d;x-datadog-parent-id=%d' % (span.trace_id, span.span_id))
eq_(
response.message,
(
# DEV: Priority sampling is enabled by default
'x-datadog-trace-id=%d;x-datadog-parent-id=%d;x-datadog-sampling-priority=1' %
(span.trace_id, span.span_id)
),
)
_check_span(span)

def test_secure_channel(self):
Expand All @@ -59,11 +66,18 @@ def test_secure_channel(self):
eq_(len(spans), 1)

span = spans[0]
eq_(response.message, 'x-datadog-trace-id=%d;x-datadog-parent-id=%d' % (span.trace_id, span.span_id))
eq_(
response.message,
(
# DEV: Priority sampling is enabled by default
'x-datadog-trace-id=%d;x-datadog-parent-id=%d;x-datadog-sampling-priority=1' %
(span.trace_id, span.span_id)
),
)
_check_span(span)

def test_priority_sampling(self):
self._tracer.configure(priority_sampling=True)
# DEV: Priority sampling is enabled by default
# Setting priority sampling reset the writer, we need to re-override it
self._tracer.writer = DummyWriter()

Expand Down
2 changes: 1 addition & 1 deletion tests/contrib/pylons/test_pylons.py
Expand Up @@ -289,7 +289,7 @@ def test_distributed_tracing_default(self):

ok_(span.trace_id != 100)
ok_(span.parent_id != 42)
ok_(span.get_metric(SAMPLING_PRIORITY_KEY) is None)
ok_(span.get_metric(SAMPLING_PRIORITY_KEY) != 2)

def test_distributed_tracing_enabled(self):
# ensure distributed tracing propagator is working
Expand Down
16 changes: 8 additions & 8 deletions tests/test_integration.py
Expand Up @@ -106,8 +106,8 @@ def test_worker_single_trace(self):
self._wait_thread_flush()
eq_(self.api._put.call_count, 1)
# check and retrieve the right call
endpoint, payload = self._get_endpoint_payload(self.api._put.call_args_list, '/v0.3/traces')
eq_(endpoint, '/v0.3/traces')
endpoint, payload = self._get_endpoint_payload(self.api._put.call_args_list, '/v0.4/traces')
eq_(endpoint, '/v0.4/traces')
eq_(len(payload), 1)
eq_(len(payload[0]), 1)
eq_(payload[0][0]['name'], 'client.testing')
Expand All @@ -124,8 +124,8 @@ def test_worker_multiple_traces(self):
self._wait_thread_flush()
eq_(self.api._put.call_count, 1)
# check and retrieve the right call
endpoint, payload = self._get_endpoint_payload(self.api._put.call_args_list, '/v0.3/traces')
eq_(endpoint, '/v0.3/traces')
endpoint, payload = self._get_endpoint_payload(self.api._put.call_args_list, '/v0.4/traces')
eq_(endpoint, '/v0.4/traces')
eq_(len(payload), 2)
eq_(len(payload[0]), 1)
eq_(len(payload[1]), 1)
Expand All @@ -143,8 +143,8 @@ def test_worker_single_trace_multiple_spans(self):
self._wait_thread_flush()
eq_(self.api._put.call_count, 1)
# check and retrieve the right call
endpoint, payload = self._get_endpoint_payload(self.api._put.call_args_list, '/v0.3/traces')
eq_(endpoint, '/v0.3/traces')
endpoint, payload = self._get_endpoint_payload(self.api._put.call_args_list, '/v0.4/traces')
eq_(endpoint, '/v0.4/traces')
eq_(len(payload), 1)
eq_(len(payload[0]), 2)
eq_(payload[0][0]['name'], 'client.testing')
Expand Down Expand Up @@ -208,8 +208,8 @@ def test_worker_filter_request(self):
# Only the second trace should have been sent
eq_(self.api._put.call_count, 1)
# check and retrieve the right call
endpoint, payload = self._get_endpoint_payload(self.api._put.call_args_list, '/v0.3/traces')
eq_(endpoint, '/v0.3/traces')
endpoint, payload = self._get_endpoint_payload(self.api._put.call_args_list, '/v0.4/traces')
eq_(endpoint, '/v0.4/traces')
eq_(len(payload), 1)
eq_(payload[0][0]['name'], 'testing.nonfilteredurl')

Expand Down
4 changes: 2 additions & 2 deletions tests/test_sampler.py
Expand Up @@ -74,7 +74,7 @@ def test_sample_rate_deviation(self):
for sample_rate in [0.1, 0.25, 0.5, 1]:
tracer = get_dummy_tracer()
writer = tracer.writer
tracer.configure(sampler=AllSampler(), priority_sampling=True)
tracer.configure(sampler=AllSampler())
# We need to set the writer because tracer.configure overrides it,
# indeed, as we enable priority sampling, we must ensure the writer
# is priority sampling aware and pass it a reference on the
Expand Down Expand Up @@ -128,7 +128,7 @@ def test_set_sample_rate_by_service(self):
]

tracer = get_dummy_tracer()
tracer.configure(sampler=AllSampler(), priority_sampling=True)
tracer.configure(sampler=AllSampler())
priority_sampler = tracer.priority_sampler
for case in cases:
priority_sampler.set_sample_rate_by_service(case)
Expand Down
20 changes: 17 additions & 3 deletions tests/utils/tracer.py
Expand Up @@ -6,9 +6,10 @@
class DummyWriter(AgentWriter):
"""DummyWriter is a small fake writer used for tests. not thread-safe."""

def __init__(self):
def __init__(self, *args, **kwargs):
# original call
super(DummyWriter, self).__init__()
super(DummyWriter, self).__init__(*args, **kwargs)

# dummy components
self.spans = []
self.traces = []
Expand Down Expand Up @@ -57,4 +58,17 @@ class DummyTracer(Tracer):
"""
def __init__(self, *args, **kwargs):
super(DummyTracer, self).__init__(*args, **kwargs)
self.writer = DummyWriter()
self._update_writer()

def _update_writer(self):
self.writer = DummyWriter(
hostname=self.writer.api.hostname,
port=self.writer.api.port,
filters=self.writer._filters,
priority_sampler=self.writer._priority_sampler,
)

def configure(self, *args, **kwargs):
super(DummyTracer, self).configure(*args, **kwargs)
# `.configure()` may reset the writer
self._update_writer()