diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index 2425c27c07..503c2e03eb 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/opentelemetry-api/src/opentelemetry/trace/sampling.py @@ -82,13 +82,13 @@ def __init__(self, rate: float): self._rate = rate self._bound = self.get_bound_for_rate(self._rate) - # The sampler checks the last 8 bytes of the trace ID to decide whether to - # sample a given trace. - CHECK_BYTES = 0xFFFFFFFFFFFFFFFF + # For compatibility with 64 bit trace IDs, the sampler checks the 64 + # low-order bits of the trace ID to decide whether to sample a given trace. + TRACE_ID_LIMIT = (1 << 64) - 1 @classmethod def get_bound_for_rate(cls, rate: float) -> int: - return round(rate * (cls.CHECK_BYTES + 1)) + return round(rate * (cls.TRACE_ID_LIMIT + 1)) @property def rate(self) -> float: @@ -115,7 +115,7 @@ def should_sample( if parent_context is not None: return Decision(parent_context.trace_options.sampled) - return Decision(trace_id & self.CHECK_BYTES < self.bound) + return Decision(trace_id & self.TRACE_ID_LIMIT < self.bound) # Samplers that ignore the parent sampling decision and never/always sample. diff --git a/opentelemetry-api/tests/trace/test_sampling.py b/opentelemetry-api/tests/trace/test_sampling.py index b456aa91f1..f04aecef45 100644 --- a/opentelemetry-api/tests/trace/test_sampling.py +++ b/opentelemetry-api/tests/trace/test_sampling.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import sys import unittest from opentelemetry import trace @@ -137,7 +138,7 @@ def test_probability_sampler(self): trace.SpanContext( 0xDEADBEF0, 0xDEADBEF1, trace_options=TO_DEFAULT ), - 0x8000000000000000, + 0x7FFFFFFFFFFFFFFF, 0xDEADBEEF, "span name", ).sampled @@ -147,7 +148,7 @@ def test_probability_sampler(self): trace.SpanContext( 0xDEADBEF0, 0xDEADBEF1, trace_options=TO_SAMPLED ), - 0x8000000000000001, + 0x8000000000000000, 0xDEADBEEF, "span name", ).sampled @@ -189,14 +190,13 @@ def test_probability_sampler_limits(self): sampling.ProbabilitySampler.get_bound_for_rate(2 ** -64), 0x1 ) - # Sample every trace with (last 8 bytes of) trace ID less than - # 0xffffffffffffffff. In principle this is the highest possible - # sampling rate less than 1, but we can't actually express this rate as - # a float! + # Sample every trace with trace ID less than 0xffffffffffffffff. In + # principle this is the highest possible sampling rate less than 1, but + # we can't actually express this rate as a float! # # In practice, the highest possible sampling rate is: # - # round(sys.float_info.epsilon * 2 ** 64) + # 1 - sys.float_info.epsilon almost_always_on = sampling.ProbabilitySampler(1 - 2 ** -64) self.assertTrue( @@ -212,12 +212,29 @@ def test_probability_sampler_limits(self): # self.assertFalse( # almost_always_on.should_sample( # None, - # 0xffffffffffffffff, - # 0xdeadbeef, + # 0xFFFFFFFFFFFFFFFF, + # 0xDEADBEEF, # "span name", # ).sampled # ) # self.assertEqual( # sampling.ProbabilitySampler.get_bound_for_rate(1 - 2 ** -64)), - # 0xffffffffffffffff, + # 0xFFFFFFFFFFFFFFFF, # ) + + # Check that a sampler with the highest effective sampling rate < 1 + # refuses to sample traces with trace ID 0xffffffffffffffff. + almost_almost_always_on = sampling.ProbabilitySampler( + 1 - sys.float_info.epsilon + ) + self.assertFalse( + almost_almost_always_on.should_sample( + None, 0xFFFFFFFFFFFFFFFF, 0xDEADBEEF, "span name" + ).sampled + ) + # Check that the higest effective sampling rate is actually lower than + # the highest theoretical sampling rate. If this test fails the test + # above is wrong. + self.assertLess( + almost_almost_always_on.bound, 0xFFFFFFFFFFFFFFFF, + )