Skip to content

Commit

Permalink
Allow for custom samplers to be used with Parent
Browse files Browse the repository at this point in the history
Relates #800

Signed-off-by: Harold Dost <h.dost@criteo.com>
  • Loading branch information
hdost committed Jul 14, 2022
1 parent b3e6e6a commit 3b20f08
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 3 deletions.
7 changes: 7 additions & 0 deletions opentelemetry-sdk/CHANGELOG.md
@@ -1,5 +1,12 @@
# Changelog

## to be released

### Changed

- *BREAKING* `struct`s which implement `ShouldSample` a.k.a Custom Samplers must now
implement `Clone`. This enables (#833)

## v0.1.0

- SDK split from `opentelemetry` crate
55 changes: 53 additions & 2 deletions opentelemetry-sdk/src/trace/sampler.rs
Expand Up @@ -66,7 +66,7 @@ use opentelemetry_http::HttpClient;
/// [`SpanProcessor`]: crate::trace::SpanProcessor
/// [`Span`]: opentelemetry_api::trace::Span
/// [`Span::is_recording()`]: opentelemetry_api::trace::Span#tymethod.is_recording
pub trait ShouldSample: Send + Sync + std::fmt::Debug {
pub trait ShouldSample: CloneShouldSample + Send + Sync + std::fmt::Debug {
/// Returns the [`SamplingDecision`] for a [`Span`] to be created.
///
/// The [`should_sample`] function can use any of the information provided to it in order to
Expand All @@ -88,6 +88,26 @@ pub trait ShouldSample: Send + Sync + std::fmt::Debug {
) -> SamplingResult;
}

/// This trait should not be used directly instead users should use [`ShouldSample`].
pub trait CloneShouldSample {
fn box_clone(&self) -> Box<dyn ShouldSample>;
}

impl<T> CloneShouldSample for T
where
T: ShouldSample + Clone + 'static,
{
fn box_clone(&self) -> Box<dyn ShouldSample> {
Box::new(self.clone())
}
}

impl Clone for Box<dyn ShouldSample> {
fn clone(&self) -> Self {
self.box_clone()
}
}

/// Default Sampling options
///
/// The [built-in samplers] allow for simple decisions. For more complex scenarios consider
Expand All @@ -102,7 +122,7 @@ pub enum Sampler {
/// Never sample the trace
AlwaysOff,
/// Respects the parent span's sampling decision or delegates a delegate sampler for root spans.
ParentBased(Box<Sampler>),
ParentBased(Box<dyn ShouldSample>),
/// Sample a given fraction of traces. Fractions >= 1 will always sample. If the parent span is
/// sampled, then it's child spans will automatically be sampled. Fractions < 0 are treated as
/// zero, but spans may still be sampled if their parent is.
Expand Down Expand Up @@ -358,6 +378,37 @@ mod tests {
}
}

#[test]
fn clone_a_parent_sampler() {
let sampler = Sampler::ParentBased(Box::new(Sampler::AlwaysOn));
let cloned_sampler = sampler.clone();

let cx = Context::current_with_value("some_value");
let instrumentation_library = InstrumentationLibrary::default();

let result = sampler.should_sample(
Some(&cx),
TraceId::from_u128(1),
"should sample",
&SpanKind::Internal,
&Default::default(),
&[],
&instrumentation_library,
);

let cloned_result = cloned_sampler.should_sample(
Some(&cx),
TraceId::from_u128(1),
"should sample",
&SpanKind::Internal,
&Default::default(),
&[],
&instrumentation_library,
);

assert_eq!(result, cloned_result);
}

#[test]
fn filter_parent_sampler_for_active_spans() {
let sampler = Sampler::ParentBased(Box::new(Sampler::AlwaysOn));
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/trace/tracer.rs
Expand Up @@ -306,7 +306,7 @@ mod tests {
Context, Key, Value,
};

#[derive(Debug)]
#[derive(Clone, Debug)]
struct TestSampler {}

impl ShouldSample for TestSampler {
Expand Down

0 comments on commit 3b20f08

Please sign in to comment.