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

feat: move away from a ParentBased sampling approach. #96

Merged
merged 1 commit into from
Mar 6, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 17 additions & 19 deletions src/honeycomb/opentelemetry/sampler.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
from logging import getLogger

from opentelemetry.sdk.trace.sampling import (
DEFAULT_OFF,
DEFAULT_ON,
ParentBasedTraceIdRatio,
ALWAYS_OFF,
ALWAYS_ON,
TraceIdRatioBased,
Sampler,
SamplingResult
)
Expand All @@ -24,11 +24,11 @@ def configure_sampler(
"""Configures and returns an OpenTelemetry Sampler that is
configured based on the sample_rate determined in HoneycombOptions.
The configuration initializes a DeterministicSampler with
an inner sampler of either DefaultOn (1), Default Off (0),
an inner sampler of either AlwaysOn (1), AlwaysOff (0),
or a TraceIdRatio as 1/N.

Each of these samplers is ParentBased, meaning it respects
its parent span's sampling decision.
These samplers do not take into account the parent span's
sampling decision.

Args:
options (HoneycombOptions): the HoneycombOptins containing
Expand All @@ -42,34 +42,32 @@ def configure_sampler(

class DeterministicSampler(Sampler):
"""Implementation of :class:`Sampler` that uses an inner sampler
of either DefaultOn (1), Default Off (0), or a TraceIdRatio as 1/N
of either AlwaysOn (1), AlwaysOff (0), or a TraceIdRatio as 1/N
to determine a SamplingResult and SamplingDecision for a given span
in a trace. We append a SampleRate attribute to the span with the
given sample rate.

Note: Each of these samplers is ParentBased, meaning it respects
its parent span's sampling decision.
Note: These samplers do not take into account the parent span's
sampling decision.
"""

def __init__(self, rate: int):
self.rate = rate

if self.rate <= 0:
# Sampler that respects its parent span's sampling decision,
# but otherwise never samples. If it's negative, we assume
# a sample rate of 0
self._sampler = DEFAULT_OFF
# Sampler that never samples spans, regardless of the
# parent span's sampling decision
self._sampler = ALWAYS_OFF

elif self.rate == 1:
# Sampler that respects its parent span's sampling decision,
# but otherwise always samples.
self._sampler = DEFAULT_ON
# Sampler that always samples spans, regardless of the
# parent span's sampling decision
self._sampler = ALWAYS_ON

else:
# Sampler that respects its parent span's sampling decision,
# but otherwise samples probabalistically based on `rate`.
# Sampler that samples probabalistically based on rate..
ratio = 1.0 / self.rate
self._sampler = ParentBasedTraceIdRatio(ratio)
self._sampler = TraceIdRatioBased(ratio)

# pylint: disable=too-many-arguments
def should_sample(
Expand Down
42 changes: 21 additions & 21 deletions tests/test_sampler.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
from opentelemetry import context
from opentelemetry.trace import SpanKind
from opentelemetry.sdk.trace.sampling import (
DEFAULT_OFF,
DEFAULT_ON,
ParentBasedTraceIdRatio,
ALWAYS_OFF,
ALWAYS_ON,
TraceIdRatioBased,
Decision
)

Expand All @@ -29,13 +29,13 @@ def get_sampling_result(test_sampler):
)


def test_sample_with_undefined_rate_defaults_to_DEFAULT_ON_and_recorded():
def test_sample_with_undefined_rate_defaults_to_ALWAYS_ON_and_recorded():
undefined_rate_sampler = configure_sampler()
# test the `DEFAULT_SAMPLE_RATE` is applied (i.e. 1)
assert undefined_rate_sampler.rate == DEFAULT_SAMPLE_RATE
# test the inner DeterministicSampler choice
inner_sampler = undefined_rate_sampler._sampler
assert inner_sampler == DEFAULT_ON
assert inner_sampler == ALWAYS_ON
assert isinstance(undefined_rate_sampler, DeterministicSampler)
# test the SamplingResult is as expected
sampling_result = get_sampling_result(undefined_rate_sampler)
Expand All @@ -46,46 +46,46 @@ def test_sample_with_undefined_rate_defaults_to_DEFAULT_ON_and_recorded():
}


def test_sampler_with_rate_of_one_is_DEFAULT_ON_and_recorded():
def test_sampler_with_rate_of_one_is_ALWAYS_ON_and_recorded():
sample_rate_one = HoneycombOptions(sample_rate=1)
default_on_sampler = configure_sampler(sample_rate_one)
always_on_sampler = configure_sampler(sample_rate_one)
# test the inner DeterministicSampler choice and rate
inner_sampler = default_on_sampler._sampler
assert inner_sampler == DEFAULT_ON
assert isinstance(default_on_sampler, DeterministicSampler)
assert default_on_sampler.rate == 1
inner_sampler = always_on_sampler._sampler
assert inner_sampler == ALWAYS_ON
assert isinstance(always_on_sampler, DeterministicSampler)
assert always_on_sampler.rate == 1
# test the SamplingResult is as expected
sampling_result = get_sampling_result(default_on_sampler)
sampling_result = get_sampling_result(always_on_sampler)
assert sampling_result.decision.is_sampled()
assert sampling_result.attributes == {
'existing_attr': 'is intact',
'SampleRate': 1
}


def test_sampler_with_rate_of_zero_is_DEFAULT_OFF_and_DROP():
def test_sampler_with_rate_of_zero_is_ALWAYS_OFF_and_DROP():
sample_rate_zero = HoneycombOptions(sample_rate=0)
default_off_sampler = configure_sampler(sample_rate_zero)
always_off_sampler = configure_sampler(sample_rate_zero)
# test the inner DeterministicSampler choice and rate
inner_sampler = default_off_sampler._sampler
assert inner_sampler == DEFAULT_OFF
assert isinstance(default_off_sampler, DeterministicSampler)
assert default_off_sampler.rate == 0
inner_sampler = always_off_sampler._sampler
assert inner_sampler == ALWAYS_OFF
assert isinstance(always_off_sampler, DeterministicSampler)
assert always_off_sampler.rate == 0
# test the SamplingResult is as expected
sampling_result = get_sampling_result(default_off_sampler)
sampling_result = get_sampling_result(always_off_sampler)
assert sampling_result.decision.is_sampled() is False
assert sampling_result.decision == Decision.DROP
assert sampling_result.attributes == {}


def test_sampler_with_rate_of_ten_configures_ParentBasedTraceIdRatio():
def test_sampler_with_rate_of_ten_configures_TraceIdRatioBased():
sample_rate_ten = HoneycombOptions(sample_rate=10)
trace_id_ratio_sampler = configure_sampler(sample_rate_ten)
# test the inner DeterministicSampler choice and rate
inner_sampler = trace_id_ratio_sampler._sampler
assert isinstance(
inner_sampler,
ParentBasedTraceIdRatio
TraceIdRatioBased
)
assert isinstance(
trace_id_ratio_sampler,
Expand Down