[dual sampling] support -1 as a priority sampling value (#391)

* [dual sampling] updated tests to make sure -1 and 2 are handled
* [dual sampling] updated doc to mention rules about -1 and 2 sampling priority values
* [dual sampling] introduced constants for sampling priorities

Author: ufoot

ddtrace/ext/priority.py

@@ -0,0 +1,24 @@
+"""
+Priority is a hint given to the backend so that it knows which traces to reject or kept.
+In a distributed context, it should be set before any context propagation (fork, RPC calls) to be effective.
+
+For example:
+
+from ddtrace.ext.priority import USER_REJECT, USER_KEEP
+
+context = tracer.context_provider.active()
+# Indicate to not keep the trace
+context.sampling_priority = USER_REJECT
+
+# Indicate to keep the trace
+span.context.sampling_priority = USER_KEEP
+"""
+
+# Use this to explicitely inform the backend that a trace should be rejected and not stored.
+USER_REJECT = -1
+# Used by the builtin sampler to inform the backend that a trace should be rejected and not stored.
+AUTO_REJECT = 0
+# Used by the builtin sampler to inform the backend that a trace should be kept and stored.
+AUTO_KEEP = 1
+# Use this to explicitely inform the backend that a trace should be kept and stored.
+USER_KEEP = 2

ddtrace/tracer.py

@@ -10,6 +10,7 @@
 from .span import Span
 from .constants import FILTERS_KEY, SAMPLE_RATE_METRIC_KEY
 from . import compat
+from .ext.priority import AUTO_REJECT, AUTO_KEEP
 
 
 log = logging.getLogger(__name__)
@@ -219,9 +220,9 @@ def start_span(self, name, child_of=None, service=None, resource=None, span_type
                     # priority sampler will use the default sampling rate, which might
                     # lead to oversampling (that is, dropping too many traces).
                     if self.priority_sampler.sample(span):
-                        context.sampling_priority = 1
+                        context.sampling_priority = AUTO_KEEP
                     else:
-                        context.sampling_priority = 0
+                        context.sampling_priority = AUTO_REJECT
             else:
                 if self.priority_sampler:
                     # If dropped by the local sampler, distributed instrumentation can drop it too.

docs/index.rst

@@ -358,9 +358,10 @@ Priority sampling
 Priority sampling consists in deciding if a trace will be kept by using a `priority` attribute that will be propagated
 for distributed traces. Its value gives indication to the Agent and to the backend on how important the trace is.
 
-- 0: Don't keep the trace.
-- 1: The sampler automatically decided to keep the trace.
-- 2: The user asked the keep the trace.
+The sampler can set the priority to the following values::
+
+- `AUTO_REJECT`: the sampler automatically decided to reject the trace
+- `AUTO_KEEP`: the sampler automatically decided to keep the trace
 
 For now, priority sampling is disabled by default. Enabling it ensures that your sampled distributed traces will be complete.
 To enable the priority sampling::
@@ -370,15 +371,29 @@ To enable the priority sampling::
 Once enabled, the sampler will automatically assign a priority of 0 or 1 to traces, depending on their service and volume.
 
 You can also set this priority manually to either drop a non-interesting trace or to keep an important one.
-For that, set the `context.sampling_priority` to 0 or 2. It has to be done before any context propagation (fork, RPC calls)
-to be effective::
+For that, set the `context.sampling_priority` to -1 or 2.
+
+- `USER_REJECT`: the user asked to reject the trace
+- `USER_KEEP`: the user asked to keep the trace
+
+When not using distributed tracing, you may change the priority at any time,
+as long as the trace is not finished yet.
+But it has to be done before any context propagation (fork, RPC calls) to be effective in a distributed context.
+Changing the priority after context has been propagated causes different parts of a distributed trace
+to use different priorities. Some parts might be kept, some parts might be rejected,
+and this can cause the trace to be partially stored and remain incomplete.
+
+If you change the priority, we recommend you do it as soon as possible, when the root span has just been created.
+
+
+    from ddtrace.ext.priority import USER_REJECT, USER_KEEP
 
     context = tracer.context_provider.active()
     # Indicate to not keep the trace
-    context.sampling_priority = 0
+    context.sampling_priority = USER_REJECT
 
     # Indicate to keep the trace
-    span.context.sampling_priority = 2
+    span.context.sampling_priority = USER_KEEP
 
 
 Pre-sampling
@@ -586,4 +601,3 @@ Indices and tables
 * :ref:`genindex`
 * :ref:`modindex`
 * :ref:`search`
-

tests/test_context.py

@@ -7,6 +7,7 @@
 
 from ddtrace.span import Span
 from ddtrace.context import Context, ThreadLocalContext
+from ddtrace.ext.priority import USER_REJECT, AUTO_REJECT, AUTO_KEEP, USER_KEEP
 
 
 class TestTracingContext(TestCase):
@@ -31,6 +32,20 @@ def test_context_sampled(self):
         ok_(ctx._sampled is True)
         ok_(ctx.sampling_priority is None)
 
+    def test_context_priority(self):
+        # a context is sampled if the spans are sampled
+        ctx = Context()
+        for priority in [USER_REJECT, AUTO_REJECT, AUTO_KEEP, USER_KEEP, None, 999]:
+            ctx.sampling_priority = priority
+            span = Span(tracer=None, name=('fake_span_%s' % repr(priority)))
+            ctx.add_span(span)
+            # It's "normal" to have sampled be true even when priority sampling is
+            # set to 0 or -1. It would stay false even even with priority set to 2.
+            # The only criteria to send (or not) the spans to the agent should be
+            # this "sampled" attribute, as it's tightly related to the trace weight.
+            ok_(ctx._sampled is True, 'priority has no impact on sampled status')
+            eq_(priority, ctx.sampling_priority)
+
     def test_current_span(self):
         # it should return the current active span
         ctx = Context()

tests/test_span.py

@@ -216,23 +216,22 @@ def test_span_boolean_err():
     eq_(d["error"], 1)
     eq_(type(d["error"]), int)
 
-def test_span_to_dict_priority():
-    for i in range(10):
-        s = Span(tracer=None, name="test.span", service="s", resource="r")
-        s.span_type = "foo"
-        s.set_tag("a", "1")
-        s.set_meta("b", "2")
-        s.finish()
-
-        d = s.to_dict()
-        assert d
-        eq_(d["span_id"], s.span_id)
-        eq_(d["trace_id"], s.trace_id)
-        eq_(d["parent_id"], s.parent_id)
-        eq_(d["meta"], {"a": "1", "b": "2"})
-        eq_(d["type"], "foo")
-        eq_(d["error"], 0)
-        eq_(type(d["error"]), int)
+def test_span_to_dict():
+    s = Span(tracer=None, name="test.span", service="s", resource="r")
+    s.span_type = "foo"
+    s.set_tag("a", "1")
+    s.set_meta("b", "2")
+    s.finish()
+
+    d = s.to_dict()
+    assert d
+    eq_(d["span_id"], s.span_id)
+    eq_(d["trace_id"], s.trace_id)
+    eq_(d["parent_id"], s.parent_id)
+    eq_(d["meta"], {"a": "1", "b": "2"})
+    eq_(d["type"], "foo")
+    eq_(d["error"], 0)
+    eq_(type(d["error"]), int)
 
 class DummyTracer(object):
     def __init__(self):