From c435600ea465b6de0c6f49a18ba3c95a4ead734e Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 31 Aug 2020 12:07:32 -0700 Subject: [PATCH] Align sampling specs in SDK (#1034) --- .../exporter/datadog/exporter.py | 2 +- .../tests/test_datadog_exporter.py | 2 +- opentelemetry-api/CHANGELOG.md | 2 + .../src/opentelemetry/trace/__init__.py | 8 +- .../src/opentelemetry/trace/span.py | 4 +- .../tests/test_implementation.py | 6 +- opentelemetry-sdk/CHANGELOG.md | 2 + .../src/opentelemetry/sdk/trace/__init__.py | 55 +++---- .../src/opentelemetry/sdk/trace/sampling.py | 153 +++++++++++++----- .../tests/trace/test_implementation.py | 6 +- .../tests/trace/test_sampling.py | 147 ++++++++++------- opentelemetry-sdk/tests/trace/test_trace.py | 39 ++--- 12 files changed, 261 insertions(+), 165 deletions(-) diff --git a/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/exporter.py b/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/exporter.py index c23288442dc..37c78187f8e 100644 --- a/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/exporter.py +++ b/exporter/opentelemetry-exporter-datadog/src/opentelemetry/exporter/datadog/exporter.py @@ -247,7 +247,7 @@ def _get_sampling_rate(span): return ( span.sampler.rate if ctx.trace_flags.sampled - and isinstance(span.sampler, sampling.ProbabilitySampler) + and isinstance(span.sampler, sampling.TraceIdRatioBased) else None ) diff --git a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_exporter.py b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_exporter.py index 47a0280c0a3..73c8cb3bf82 100644 --- a/exporter/opentelemetry-exporter-datadog/tests/test_datadog_exporter.py +++ b/exporter/opentelemetry-exporter-datadog/tests/test_datadog_exporter.py @@ -498,7 +498,7 @@ def test_sampling_rate(self): is_remote=False, trace_flags=trace_api.TraceFlags(trace_api.TraceFlags.SAMPLED), ) - sampler = sampling.ProbabilitySampler(0.5) + sampler = sampling.TraceIdRatioBased(0.5) span = trace.Span( name="sampled", context=context, parent=None, sampler=sampler diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index 048da1f1667..a22f98585ec 100644 --- a/opentelemetry-api/CHANGELOG.md +++ b/opentelemetry-api/CHANGELOG.md @@ -8,6 +8,8 @@ ([#1023](https://github.com/open-telemetry/opentelemetry-python/pull/1023)) - Change return value type of `correlationcontext.get_correlations` to immutable `MappingProxyType` ([#1024](https://github.com/open-telemetry/opentelemetry-python/pull/1024)) +- Change is_recording_events to is_recording + ([#1034](https://github.com/open-telemetry/opentelemetry-python/pull/1034)) - Remove lazy Event and Link API from Span interface ([#1045](https://github.com/open-telemetry/opentelemetry-python/pull/1045)) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 254c6de3419..1795192254f 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -229,7 +229,7 @@ def start_span( name: str, parent: ParentSpan = CURRENT_SPAN, kind: SpanKind = SpanKind.INTERNAL, - attributes: typing.Optional[types.Attributes] = None, + attributes: types.Attributes = None, links: typing.Sequence[Link] = (), start_time: typing.Optional[int] = None, set_status_on_exception: bool = True, @@ -281,7 +281,7 @@ def start_as_current_span( name: str, parent: ParentSpan = CURRENT_SPAN, kind: SpanKind = SpanKind.INTERNAL, - attributes: typing.Optional[types.Attributes] = None, + attributes: types.Attributes = None, links: typing.Sequence[Link] = (), ) -> typing.Iterator["Span"]: """Context manager for creating a new span and set it @@ -357,7 +357,7 @@ def start_span( name: str, parent: ParentSpan = Tracer.CURRENT_SPAN, kind: SpanKind = SpanKind.INTERNAL, - attributes: typing.Optional[types.Attributes] = None, + attributes: types.Attributes = None, links: typing.Sequence[Link] = (), start_time: typing.Optional[int] = None, set_status_on_exception: bool = True, @@ -371,7 +371,7 @@ def start_as_current_span( name: str, parent: ParentSpan = Tracer.CURRENT_SPAN, kind: SpanKind = SpanKind.INTERNAL, - attributes: typing.Optional[types.Attributes] = None, + attributes: types.Attributes = None, links: typing.Sequence[Link] = (), ) -> typing.Iterator["Span"]: # pylint: disable=unused-argument,no-self-use diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index ecec7572823..27bbc223368 100644 --- a/opentelemetry-api/src/opentelemetry/trace/span.py +++ b/opentelemetry-api/src/opentelemetry/trace/span.py @@ -62,7 +62,7 @@ def update_name(self, name: str) -> None: """ @abc.abstractmethod - def is_recording_events(self) -> bool: + def is_recording(self) -> bool: """Returns whether this span will be recorded. Returns true if this Span is active and recording information like @@ -203,7 +203,7 @@ def __init__(self, context: "SpanContext") -> None: def get_context(self) -> "SpanContext": return self._context - def is_recording_events(self) -> bool: + def is_recording(self) -> bool: return False def end(self, end_time: typing.Optional[int] = None) -> None: diff --git a/opentelemetry-api/tests/test_implementation.py b/opentelemetry-api/tests/test_implementation.py index d0f9404a911..0d5b22b18f5 100644 --- a/opentelemetry-api/tests/test_implementation.py +++ b/opentelemetry-api/tests/test_implementation.py @@ -39,13 +39,13 @@ def test_default_tracer(self): with tracer.start_span("test") as span: self.assertEqual(span.get_context(), trace.INVALID_SPAN_CONTEXT) self.assertEqual(span, trace.INVALID_SPAN) - self.assertIs(span.is_recording_events(), False) + self.assertIs(span.is_recording(), False) with tracer.start_span("test2") as span2: self.assertEqual( span2.get_context(), trace.INVALID_SPAN_CONTEXT ) self.assertEqual(span2, trace.INVALID_SPAN) - self.assertIs(span2.is_recording_events(), False) + self.assertIs(span2.is_recording(), False) def test_span(self): with self.assertRaises(TypeError): @@ -55,7 +55,7 @@ def test_span(self): def test_default_span(self): span = trace.DefaultSpan(trace.INVALID_SPAN_CONTEXT) self.assertEqual(span.get_context(), trace.INVALID_SPAN_CONTEXT) - self.assertIs(span.is_recording_events(), False) + self.assertIs(span.is_recording(), False) # METER diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 625e60ce3e6..acd1ce1d7d1 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -4,6 +4,8 @@ - Moved samplers from API to SDK ([#1023](https://github.com/open-telemetry/opentelemetry-python/pull/1023)) +- Sampling spec changes + ([#1034](https://github.com/open-telemetry/opentelemetry-python/pull/1034)) - Remove lazy Event and Link API from Span interface ([#1045](https://github.com/open-telemetry/opentelemetry-python/pull/1045)) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 1a1efa96bf7..95d04d85895 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -515,7 +515,7 @@ def get_context(self): def set_attribute(self, key: str, value: types.AttributeValue) -> None: with self._lock: - if not self.is_recording_events(): + if not self.is_recording(): return has_ended = self.end_time is not None if has_ended: @@ -541,7 +541,7 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: def _add_event(self, event: EventBase) -> None: with self._lock: - if not self.is_recording_events(): + if not self.is_recording(): return has_ended = self.end_time is not None @@ -569,7 +569,7 @@ def add_event( def start(self, start_time: Optional[int] = None) -> None: with self._lock: - if not self.is_recording_events(): + if not self.is_recording(): return has_started = self.start_time is not None if not has_started: @@ -583,7 +583,7 @@ def start(self, start_time: Optional[int] = None) -> None: def end(self, end_time: Optional[int] = None) -> None: with self._lock: - if not self.is_recording_events(): + if not self.is_recording(): return if self.start_time is None: raise RuntimeError("Calling end() on a not started span.") @@ -610,7 +610,7 @@ def update_name(self, name: str) -> None: return self.name = name - def is_recording_events(self) -> bool: + def is_recording(self) -> bool: return True def set_status(self, status: trace_api.Status) -> None: @@ -703,7 +703,7 @@ def start_as_current_span( name: str, parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN, kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, - attributes: Optional[types.Attributes] = None, + attributes: types.Attributes = None, links: Sequence[trace_api.Link] = (), ) -> Iterator[trace_api.Span]: span = self.start_span(name, parent, kind, attributes, links) @@ -714,7 +714,7 @@ def start_span( # pylint: disable=too-many-locals name: str, parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN, kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, - attributes: Optional[types.Attributes] = None, + attributes: types.Attributes = None, links: Sequence[trace_api.Link] = (), start_time: Optional[int] = None, set_status_on_exception: bool = True, @@ -741,6 +741,20 @@ def start_span( # pylint: disable=too-many-locals trace_flags = parent_context.trace_flags trace_state = parent_context.trace_state + # The sampler decides whether to create a real or no-op span at the + # time of span creation. No-op spans do not record events, and are not + # exported. + # The sampler may also add attributes to the newly-created span, e.g. + # to include information about the sampling result. + sampling_result = self.source.sampler.should_sample( + parent_context, trace_id, name, attributes, links, + ) + + trace_flags = ( + trace_api.TraceFlags(trace_api.TraceFlags.SAMPLED) + if sampling_result.decision.is_sampled() + else trace_api.TraceFlags(trace_api.TraceFlags.DEFAULT) + ) context = trace_api.SpanContext( trace_id, generate_span_id(), @@ -749,29 +763,8 @@ def start_span( # pylint: disable=too-many-locals trace_state=trace_state, ) - # The sampler decides whether to create a real or no-op span at the - # time of span creation. No-op spans do not record events, and are not - # exported. - # The sampler may also add attributes to the newly-created span, e.g. - # to include information about the sampling decision. - sampling_decision = self.source.sampler.should_sample( - parent_context, - context.trace_id, - context.span_id, - name, - attributes, - links, - ) - - if sampling_decision.sampled: - options = context.trace_flags | trace_api.TraceFlags.SAMPLED - context.trace_flags = trace_api.TraceFlags(options) - if attributes is None: - span_attributes = sampling_decision.attributes - else: - # apply sampling decision attributes after initial attributes - span_attributes = attributes.copy() - span_attributes.update(sampling_decision.attributes) + # Only record if is_recording() is true + if sampling_result.decision.is_recording(): # pylint:disable=protected-access span = Span( name=name, @@ -779,7 +772,7 @@ def start_span( # pylint: disable=too-many-locals parent=parent_context, sampler=self.source.sampler, resource=self.source.resource, - attributes=span_attributes, + attributes=sampling_result.attributes, span_processor=self.source._active_span_processor, kind=kind, links=links, diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 4429e33a48a..c723e18adba 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -18,18 +18,20 @@ OpenTelemetry provides two types of samplers: - `StaticSampler` -- `ProbabilitySampler` +- `TraceIdRatioBased` -A `StaticSampler` always returns the same sampling decision regardless of the conditions. Both possible StaticSamplers are already created: +A `StaticSampler` always returns the same sampling result regardless of the conditions. Both possible StaticSamplers are already created: -- Always sample spans: `ALWAYS_ON` -- Never sample spans: `ALWAYS_OFF` +- Always sample spans: ALWAYS_ON +- Never sample spans: ALWAYS_OFF -A `ProbabilitySampler` makes a random sampling decision based on the sampling probability given. If the span being sampled has a parent, `ProbabilitySampler` will respect the parent span's sampling decision. +A `TraceIdRatioBased` sampler makes a random sampling result based on the sampling probability given. -Currently, sampling decisions are always made during the creation of the span. However, this might not always be the case in the future (see `OTEP #115 `_). +If the span being sampled has a parent, `ParentBased` will respect the parent span's sampling result. Otherwise, it returns the sampling result from the given delegate sampler. -Custom samplers can be created by subclassing `Sampler` and implementing `Sampler.should_sample`. +Currently, sampling results are always made during the creation of the span. However, this might not always be the case in the future (see `OTEP #115 `_). + +Custom samplers can be created by subclassing `Sampler` and implementing `Sampler.should_sample` as well as `Sampler.get_description`. To use a sampler, pass it into the tracer provider constructor. For example: @@ -41,10 +43,10 @@ ConsoleSpanExporter, SimpleExportSpanProcessor, ) - from opentelemetry.sdk.trace.sampling import ProbabilitySampler + from opentelemetry.sdk.trace.sampling import TraceIdRatioBased # sample 1 in every 1000 traces - sampler = ProbabilitySampler(1/1000) + sampler = TraceIdRatioBased(1/1000) # set the sampler onto the global tracer provider trace.set_tracer_provider(TracerProvider(sampler=sampler)) @@ -54,41 +56,57 @@ SimpleExportSpanProcessor(ConsoleSpanExporter()) ) - # created spans will now be sampled by the ProbabilitySampler + # created spans will now be sampled by the TraceIdRatioBased sampler with trace.get_tracer(__name__).start_as_current_span("Test Span"): ... """ import abc -from typing import Dict, Mapping, Optional, Sequence +import enum +from typing import Optional, Sequence # pylint: disable=unused-import from opentelemetry.trace import Link, SpanContext -from opentelemetry.util.types import Attributes, AttributeValue +from opentelemetry.util.types import Attributes + + +class Decision(enum.Enum): + # IsRecording() == false, span will not be recorded and all events and attributes will be dropped. + NOT_RECORD = 0 + # IsRecording() == true, but Sampled flag MUST NOT be set. + RECORD = 1 + # IsRecording() == true AND Sampled flag` MUST be set. + RECORD_AND_SAMPLED = 2 + + def is_recording(self): + return self in (Decision.RECORD, Decision.RECORD_AND_SAMPLED) + def is_sampled(self): + return self is Decision.RECORD_AND_SAMPLED -class Decision: - """A sampling decision as applied to a newly-created Span. + +class SamplingResult: + """A sampling result as applied to a newly-created Span. Args: - sampled: Whether the `opentelemetry.trace.Span` should be sampled. + decision: A sampling decision based off of whether the span is recorded + and the sampled flag in trace flags in the span context. attributes: Attributes to add to the `opentelemetry.trace.Span`. """ def __repr__(self) -> str: return "{}({}, attributes={})".format( - type(self).__name__, str(self.sampled), str(self.attributes) + type(self).__name__, str(self.decision), str(self.attributes) ) def __init__( - self, - sampled: bool = False, - attributes: Optional[Mapping[str, "AttributeValue"]] = None, + self, decision: Decision, attributes: Attributes = None, ) -> None: - self.sampled = sampled # type: bool + self.decision = decision + # TODO: attributes must be immutable if attributes is None: - self.attributes = {} # type: Dict[str, "AttributeValue"] + self.attributes = {} else: - self.attributes = dict(attributes) + self.attributes = attributes class Sampler(abc.ABC): @@ -97,11 +115,14 @@ def should_sample( self, parent_context: Optional["SpanContext"], trace_id: int, - span_id: int, name: str, - attributes: Optional[Attributes] = None, + attributes: Attributes = None, links: Sequence["Link"] = (), - ) -> "Decision": + ) -> "SamplingResult": + pass + + @abc.abstractmethod + def get_description(self) -> str: pass @@ -115,15 +136,21 @@ def should_sample( self, parent_context: Optional["SpanContext"], trace_id: int, - span_id: int, name: str, - attributes: Optional[Attributes] = None, + attributes: Attributes = None, links: Sequence["Link"] = (), - ) -> "Decision": - return self._decision + ) -> "SamplingResult": + if self._decision is Decision.NOT_RECORD: + return SamplingResult(self._decision) + return SamplingResult(self._decision, attributes) + def get_description(self) -> str: + if self._decision is Decision.NOT_RECORD: + return "AlwaysOffSampler" + return "AlwaysOnSampler" -class ProbabilitySampler(Sampler): + +class TraceIdRatioBased(Sampler): """ Sampler that makes sampling decisions probabalistically based on `rate`, while also respecting the parent span sampling decision. @@ -133,6 +160,8 @@ class ProbabilitySampler(Sampler): """ def __init__(self, rate: float): + if rate < 0.0 or rate > 1.0: + raise ValueError("Probability must be in range [0.0, 1.0].") self._rate = rate self._bound = self.get_bound_for_rate(self._rate) @@ -161,26 +190,70 @@ def should_sample( self, parent_context: Optional["SpanContext"], trace_id: int, - span_id: int, name: str, - attributes: Optional[Attributes] = None, # TODO + attributes: Attributes = None, # TODO links: Sequence["Link"] = (), - ) -> "Decision": + ) -> "SamplingResult": + decision = Decision.NOT_RECORD + if trace_id & self.TRACE_ID_LIMIT < self.bound: + decision = Decision.RECORD_AND_SAMPLED + if decision is Decision.NOT_RECORD: + return SamplingResult(decision) + return SamplingResult(decision, attributes) + + def get_description(self) -> str: + return "TraceIdRatioBased{{{}}}".format(self._rate) + + +class ParentBased(Sampler): + """ + If a parent is set, follows the same sampling decision as the parent. + Otherwise, uses the delegate provided at initialization to make a + decision. + + Args: + delegate: The delegate sampler to use if parent is not set. + """ + + def __init__(self, delegate: Sampler): + self._delegate = delegate + + def should_sample( + self, + parent_context: Optional["SpanContext"], + trace_id: int, + name: str, + attributes: Attributes = None, # TODO + links: Sequence["Link"] = (), + ) -> "SamplingResult": if parent_context is not None: - return Decision(parent_context.trace_flags.sampled) + if ( + not parent_context.is_valid + or not parent_context.trace_flags.sampled + ): + return SamplingResult(Decision.NOT_RECORD) + return SamplingResult(Decision.RECORD_AND_SAMPLED, attributes) + + return self._delegate.should_sample( + parent_context=parent_context, + trace_id=trace_id, + name=name, + attributes=attributes, + links=links, + ) - return Decision(trace_id & self.TRACE_ID_LIMIT < self.bound) + def get_description(self): + return "ParentBased{{{}}}".format(self._delegate.get_description()) -ALWAYS_OFF = StaticSampler(Decision(False)) +ALWAYS_OFF = StaticSampler(Decision.NOT_RECORD) """Sampler that never samples spans, regardless of the parent span's sampling decision.""" -ALWAYS_ON = StaticSampler(Decision(True)) +ALWAYS_ON = StaticSampler(Decision.RECORD_AND_SAMPLED) """Sampler that always samples spans, regardless of the parent span's sampling decision.""" - -DEFAULT_OFF = ProbabilitySampler(0.0) +DEFAULT_OFF = ParentBased(ALWAYS_OFF) """Sampler that respects its parent span's sampling decision, but otherwise never samples.""" -DEFAULT_ON = ProbabilitySampler(1.0) +DEFAULT_ON = ParentBased(ALWAYS_ON) """Sampler that respects its parent span's sampling decision, but otherwise always samples.""" diff --git a/opentelemetry-sdk/tests/trace/test_implementation.py b/opentelemetry-sdk/tests/trace/test_implementation.py index cd53a643951..4582b069578 100644 --- a/opentelemetry-sdk/tests/trace/test_implementation.py +++ b/opentelemetry-sdk/tests/trace/test_implementation.py @@ -31,11 +31,11 @@ def test_tracer(self): with tracer.start_span("test") as span: self.assertNotEqual(span.get_context(), INVALID_SPAN_CONTEXT) self.assertNotEqual(span, INVALID_SPAN) - self.assertIs(span.is_recording_events(), True) + self.assertIs(span.is_recording(), True) with tracer.start_span("test2") as span2: self.assertNotEqual(span2.get_context(), INVALID_SPAN_CONTEXT) self.assertNotEqual(span2, INVALID_SPAN) - self.assertIs(span2.is_recording_events(), True) + self.assertIs(span2.is_recording(), True) def test_span(self): with self.assertRaises(Exception): @@ -44,4 +44,4 @@ def test_span(self): span = trace.Span("name", INVALID_SPAN_CONTEXT) self.assertEqual(span.get_context(), INVALID_SPAN_CONTEXT) - self.assertIs(span.is_recording_events(), True) + self.assertIs(span.is_recording(), True) diff --git a/opentelemetry-sdk/tests/trace/test_sampling.py b/opentelemetry-sdk/tests/trace/test_sampling.py index a198bd32965..2c03f0bf737 100644 --- a/opentelemetry-sdk/tests/trace/test_sampling.py +++ b/opentelemetry-sdk/tests/trace/test_sampling.py @@ -30,10 +30,12 @@ def test_always_on(self): ), 0xDEADBEF1, 0xDEADBEF2, - "unsampled parent, sampling on", + {"unsampled parent": "sampling on"}, + ) + self.assertTrue(no_record_always_on.decision.is_sampled()) + self.assertEqual( + no_record_always_on.attributes, {"unsampled parent": "sampling on"} ) - self.assertTrue(no_record_always_on.sampled) - self.assertEqual(no_record_always_on.attributes, {}) sampled_always_on = sampling.ALWAYS_ON.should_sample( trace.SpanContext( @@ -41,10 +43,12 @@ def test_always_on(self): ), 0xDEADBEF1, 0xDEADBEF2, - "sampled parent, sampling on", + {"sampled parent": "sampling on"}, + ) + self.assertTrue(no_record_always_on.decision.is_sampled()) + self.assertEqual( + sampled_always_on.attributes, {"sampled parent": "sampling on"} ) - self.assertTrue(sampled_always_on.sampled) - self.assertEqual(sampled_always_on.attributes, {}) def test_always_off(self): no_record_always_off = sampling.ALWAYS_OFF.should_sample( @@ -55,7 +59,7 @@ def test_always_off(self): 0xDEADBEF2, "unsampled parent, sampling off", ) - self.assertFalse(no_record_always_off.sampled) + self.assertFalse(no_record_always_off.decision.is_sampled()) self.assertEqual(no_record_always_off.attributes, {}) sampled_always_on = sampling.ALWAYS_OFF.should_sample( @@ -66,7 +70,7 @@ def test_always_off(self): 0xDEADBEF2, "sampled parent, sampling off", ) - self.assertFalse(sampled_always_on.sampled) + self.assertFalse(sampled_always_on.decision.is_sampled()) self.assertEqual(sampled_always_on.attributes, {}) def test_default_on(self): @@ -78,7 +82,7 @@ def test_default_on(self): 0xDEADBEF2, "unsampled parent, sampling on", ) - self.assertFalse(no_record_default_on.sampled) + self.assertFalse(no_record_default_on.decision.is_sampled()) self.assertEqual(no_record_default_on.attributes, {}) sampled_default_on = sampling.DEFAULT_ON.should_sample( @@ -87,10 +91,20 @@ def test_default_on(self): ), 0xDEADBEF1, 0xDEADBEF2, - "sampled parent, sampling on", + {"sampled parent": "sampling on"}, + ) + self.assertTrue(sampled_default_on.decision.is_sampled()) + self.assertEqual( + sampled_default_on.attributes, {"sampled parent": "sampling on"} + ) + + default_on = sampling.DEFAULT_ON.should_sample( + None, 0xDEADBEF1, 0xDEADBEF2, {"sampled parent": "sampling on"}, + ) + self.assertTrue(default_on.decision.is_sampled()) + self.assertEqual( + default_on.attributes, {"sampled parent": "sampling on"} ) - self.assertTrue(sampled_default_on.sampled) - self.assertEqual(sampled_default_on.attributes, {}) def test_default_off(self): no_record_default_off = sampling.DEFAULT_OFF.should_sample( @@ -101,7 +115,7 @@ def test_default_off(self): 0xDEADBEF2, "unsampled parent, sampling off", ) - self.assertFalse(no_record_default_off.sampled) + self.assertFalse(no_record_default_off.decision.is_sampled()) self.assertEqual(no_record_default_off.attributes, {}) sampled_default_off = sampling.DEFAULT_OFF.should_sample( @@ -110,70 +124,49 @@ def test_default_off(self): ), 0xDEADBEF1, 0xDEADBEF2, - "sampled parent, sampling off", + {"sampled parent": "sampling on"}, + ) + self.assertTrue(sampled_default_off.decision.is_sampled()) + self.assertEqual( + sampled_default_off.attributes, {"sampled parent": "sampling on"} + ) + + default_off = sampling.DEFAULT_OFF.should_sample( + None, 0xDEADBEF1, 0xDEADBEF2, "unsampled parent, sampling off", ) - self.assertTrue(sampled_default_off.sampled) - self.assertEqual(sampled_default_off.attributes, {}) + self.assertFalse(default_off.decision.is_sampled()) + self.assertEqual(default_off.attributes, {}) def test_probability_sampler(self): - sampler = sampling.ProbabilitySampler(0.5) + sampler = sampling.TraceIdRatioBased(0.5) # Check that we sample based on the trace ID if the parent context is # null self.assertTrue( sampler.should_sample( None, 0x7FFFFFFFFFFFFFFF, 0xDEADBEEF, "span name" - ).sampled + ).decision.is_sampled() ) self.assertFalse( sampler.should_sample( None, 0x8000000000000000, 0xDEADBEEF, "span name" - ).sampled - ) - - # Check that the sampling decision matches the parent context if given, - # and that the sampler ignores the trace ID - self.assertFalse( - sampler.should_sample( - trace.SpanContext( - 0xDEADBEF0, - 0xDEADBEF1, - is_remote=False, - trace_flags=TO_DEFAULT, - ), - 0x7FFFFFFFFFFFFFFF, - 0xDEADBEEF, - "span name", - ).sampled - ) - self.assertTrue( - sampler.should_sample( - trace.SpanContext( - 0xDEADBEF0, - 0xDEADBEF1, - is_remote=False, - trace_flags=TO_SAMPLED, - ), - 0x8000000000000000, - 0xDEADBEEF, - "span name", - ).sampled + ).decision.is_sampled() ) def test_probability_sampler_zero(self): - default_off = sampling.ProbabilitySampler(0.0) + default_off = sampling.TraceIdRatioBased(0.0) self.assertFalse( default_off.should_sample( None, 0x0, 0xDEADBEEF, "span name" - ).sampled + ).decision.is_sampled() ) def test_probability_sampler_one(self): - default_off = sampling.ProbabilitySampler(1.0) + default_off = sampling.TraceIdRatioBased(1.0) self.assertTrue( default_off.should_sample( None, 0xFFFFFFFFFFFFFFFF, 0xDEADBEEF, "span name" - ).sampled + ).decision.is_sampled() ) def test_probability_sampler_limits(self): @@ -181,19 +174,19 @@ def test_probability_sampler_limits(self): # Sample one of every 2^64 (= 5e-20) traces. This is the lowest # possible meaningful sampling rate, only traces with trace ID 0x0 # should get sampled. - almost_always_off = sampling.ProbabilitySampler(2 ** -64) + almost_always_off = sampling.TraceIdRatioBased(2 ** -64) self.assertTrue( almost_always_off.should_sample( None, 0x0, 0xDEADBEEF, "span name" - ).sampled + ).decision.is_sampled() ) self.assertFalse( almost_always_off.should_sample( None, 0x1, 0xDEADBEEF, "span name" - ).sampled + ).decision.is_sampled() ) self.assertEqual( - sampling.ProbabilitySampler.get_bound_for_rate(2 ** -64), 0x1 + sampling.TraceIdRatioBased.get_bound_for_rate(2 ** -64), 0x1 ) # Sample every trace with trace ID less than 0xffffffffffffffff. In @@ -204,11 +197,11 @@ def test_probability_sampler_limits(self): # # 1 - sys.float_info.epsilon - almost_always_on = sampling.ProbabilitySampler(1 - 2 ** -64) + almost_always_on = sampling.TraceIdRatioBased(1 - 2 ** -64) self.assertTrue( almost_always_on.should_sample( None, 0xFFFFFFFFFFFFFFFE, 0xDEADBEEF, "span name" - ).sampled + ).decision.is_sampled() ) # These tests are logically consistent, but fail because of the float @@ -224,19 +217,19 @@ def test_probability_sampler_limits(self): # ).sampled # ) # self.assertEqual( - # sampling.ProbabilitySampler.get_bound_for_rate(1 - 2 ** -64)), + # sampling.TraceIdRatioBased.get_bound_for_rate(1 - 2 ** -64)), # 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( + almost_almost_always_on = sampling.TraceIdRatioBased( 1 - sys.float_info.epsilon ) self.assertFalse( almost_almost_always_on.should_sample( None, 0xFFFFFFFFFFFFFFFF, 0xDEADBEEF, "span name" - ).sampled + ).decision.is_sampled() ) # Check that the higest effective sampling rate is actually lower than # the highest theoretical sampling rate. If this test fails the test @@ -244,3 +237,35 @@ def test_probability_sampler_limits(self): self.assertLess( almost_almost_always_on.bound, 0xFFFFFFFFFFFFFFFF, ) + + def test_parent_based(self): + sampler = sampling.ParentBased(sampling.ALWAYS_ON) + # Check that the sampling decision matches the parent context if given + self.assertFalse( + sampler.should_sample( + trace.SpanContext( + 0xDEADBEF0, + 0xDEADBEF1, + is_remote=False, + trace_flags=TO_DEFAULT, + ), + 0x7FFFFFFFFFFFFFFF, + 0xDEADBEEF, + "span name", + ).decision.is_sampled() + ) + + sampler2 = sampling.ParentBased(sampling.ALWAYS_OFF) + self.assertTrue( + sampler2.should_sample( + trace.SpanContext( + 0xDEADBEF0, + 0xDEADBEF1, + is_remote=False, + trace_flags=TO_SAMPLED, + ), + 0x8000000000000000, + 0xDEADBEEF, + "span name", + ).decision.is_sampled() + ) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index acc2005826e..1c272048cae 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -140,6 +140,12 @@ def test_default_sampler(self): child_span = tracer.start_span(name="child span", parent=root_span) self.assertIsInstance(child_span, trace.Span) self.assertTrue(root_span.context.trace_flags.sampled) + self.assertEqual( + root_span.get_context().trace_flags, trace_api.TraceFlags.SAMPLED + ) + self.assertEqual( + child_span.get_context().trace_flags, trace_api.TraceFlags.SAMPLED + ) def test_sampler_no_sampling(self): tracer_provider = trace.TracerProvider(sampling.ALWAYS_OFF) @@ -151,6 +157,12 @@ def test_sampler_no_sampling(self): self.assertIsInstance(root_span, trace_api.DefaultSpan) child_span = tracer.start_span(name="child span", parent=root_span) self.assertIsInstance(child_span, trace_api.DefaultSpan) + self.assertEqual( + root_span.get_context().trace_flags, trace_api.TraceFlags.DEFAULT + ) + self.assertEqual( + child_span.get_context().trace_flags, trace_api.TraceFlags.DEFAULT + ) class TestSpanCreation(unittest.TestCase): @@ -201,7 +213,7 @@ def test_invalid_instrumentation_info(self): tracer1.instrumentation_info, InstrumentationInfo ) span1 = tracer1.start_span("foo") - self.assertTrue(span1.is_recording_events()) + self.assertTrue(span1.is_recording()) self.assertEqual(tracer1.instrumentation_info.version, "") self.assertEqual( tracer1.instrumentation_info.name, "ERROR:MISSING MODULE NAME" @@ -521,36 +533,25 @@ def test_check_attribute_helper(self): self.assertTrue(trace._is_valid_attribute_value(15)) def test_sampling_attributes(self): - decision_attributes = { + sampling_attributes = { "sampler-attr": "sample-val", "attr-in-both": "decision-attr", } tracer_provider = trace.TracerProvider( - sampling.StaticSampler( - sampling.Decision(sampled=True, attributes=decision_attributes) - ) + sampling.StaticSampler(sampling.Decision.RECORD_AND_SAMPLED,) ) self.tracer = tracer_provider.get_tracer(__name__) - with self.tracer.start_as_current_span("root2") as root: - self.assertEqual(len(root.attributes), 2) - self.assertEqual(root.attributes["sampler-attr"], "sample-val") - self.assertEqual(root.attributes["attr-in-both"], "decision-attr") - - attributes = { - "attr-key": "val", - "attr-key2": "val2", - "attr-in-both": "span-attr", - } with self.tracer.start_as_current_span( - "root2", attributes=attributes + name="root2", attributes=sampling_attributes ) as root: - self.assertEqual(len(root.attributes), 4) - self.assertEqual(root.attributes["attr-key"], "val") - self.assertEqual(root.attributes["attr-key2"], "val2") + self.assertEqual(len(root.attributes), 2) self.assertEqual(root.attributes["sampler-attr"], "sample-val") self.assertEqual(root.attributes["attr-in-both"], "decision-attr") + self.assertEqual( + root.get_context().trace_flags, trace_api.TraceFlags.SAMPLED + ) def test_events(self): self.assertEqual(trace_api.get_current_span(), trace_api.INVALID_SPAN)