From 496320d270161e08ccfb17f9e84bad41d09a03fd Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Tue, 15 Oct 2019 17:59:13 -0700 Subject: [PATCH 01/24] Add sampling API --- .../src/opentelemetry/trace/sampling.py | 80 +++++++++++++++++++ .../tests/trace/test_sampling.py | 74 +++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 opentelemetry-api/src/opentelemetry/trace/sampling.py create mode 100644 opentelemetry-api/tests/trace/test_sampling.py diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py new file mode 100644 index 00000000000..e9e6f488ad0 --- /dev/null +++ b/opentelemetry-api/src/opentelemetry/trace/sampling.py @@ -0,0 +1,80 @@ +# Copyright 2019, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import abc +from typing import Dict, Mapping, Optional, Sequence + +# pylint: disable=unused-import +from opentelemetry.trace import Link, SpanContext +from opentelemetry.util.types import AttributeValue + + +class Decision: + """A sampling decision as applied to a newly-created Span. + + Args: + sampled: Whether the `Span` should be sampled. + attributes: Attributes to add to the `Span`. + """ + + def __repr__(self): + return "{}({}, attributes={})".format( + type(self).__name__, self.sampled, self.attributes + ) + + def __init__( + self, + sampled: bool = False, + attributes: Mapping[str, "AttributeValue"] = None, + ) -> None: + self.sampled = sampled + if attributes is None: + self.attributes = {} # type:Dict[str, "AttributeValue"] + else: + self.attributes = dict(attributes) + + +class Sampler(abc.ABC): + + @abc.abstractmethod + def should_sample( + self, + parent_context: Optional["SpanContext"], + trace_id: int, + span_id: int, + name: str, + links: Optional[Sequence["Link"]] = None, + ) -> "Decision": + pass + + +class StaticSampler(Sampler): + """Sampler that always returns the same decision.""" + + def __init__(self, decision: "Decision"): + self.decision = decision + + def should_sample( + self, + parent_context: Optional["SpanContext"], + trace_id: int, + span_id: int, + name: str, + links: Optional[Sequence["Link"]] = None, + ) -> "Decision": + return self.decision + + +ALWAYS_OFF = StaticSampler(Decision(False)) +ALWAYS_ON = StaticSampler(Decision(True)) diff --git a/opentelemetry-api/tests/trace/test_sampling.py b/opentelemetry-api/tests/trace/test_sampling.py new file mode 100644 index 00000000000..7258437f0a4 --- /dev/null +++ b/opentelemetry-api/tests/trace/test_sampling.py @@ -0,0 +1,74 @@ +# Copyright 2019, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +from opentelemetry import trace +from opentelemetry.trace import sampling + + +class TestSampler(unittest.TestCase): + def test_always_on(self): + unrecorded_on = sampling.ALWAYS_ON.should_sample( + trace.SpanContext( + 0x000000000000000000000000DEADBEEF, + 0x00000000DEADBEF0, + trace_options=trace.TraceOptions.DEFAULT, + ), + 0x000000000000000000000000DEADBEF1, + 0x00000000DEADBEF2, + "unrecorded parent, sampling on", + ) + self.assertTrue(unrecorded_on.sampled) + self.assertEqual(unrecorded_on.attributes, {}) + + recorded_on = sampling.ALWAYS_ON.should_sample( + trace.SpanContext( + 0x000000000000000000000000DEADBEF3, + 0x00000000DEADBEF4, + trace_options=trace.TraceOptions.DEFAULT, + ), + 0x000000000000000000000000DEADBEF5, + 0x00000000DEADBEF6, + "recorded parent, sampling on", + ) + self.assertTrue(recorded_on.sampled) + self.assertEqual(recorded_on.attributes, {}) + + def test_always_off(self): + unrecorded_off = sampling.ALWAYS_ON.should_sample( + trace.SpanContext( + 0x000000000000000000000000DEADBEF7, + 0x00000000DEADBEF8, + trace_options=trace.TraceOptions.DEFAULT, + ), + 0x000000000000000000000000DEADBEF9, + 0x00000000DEADBEFA, + "recorded parent, sampling off", + ) + self.assertTrue(unrecorded_off.sampled) + self.assertEqual(unrecorded_off.attributes, {}) + + recorded_off = sampling.ALWAYS_ON.should_sample( + trace.SpanContext( + 0x000000000000000000000000DEADBEF7, + 0x00000000DEADBEF8, + trace_options=trace.TraceOptions.RECORDED, + ), + 0x000000000000000000000000DEADBEF9, + 0x00000000DEADBEFA, + "unrecorded parent, sampling off", + ) + self.assertTrue(recorded_off.sampled) + self.assertEqual(recorded_off.attributes, {}) From 21dd08174c10df3988614704ddd8fd522cf82bb6 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 16 Oct 2019 12:05:23 -0700 Subject: [PATCH 02/24] Kowtow to mypy --- .../src/opentelemetry/trace/sampling.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index e9e6f488ad0..8df33a7ad23 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/opentelemetry-api/src/opentelemetry/trace/sampling.py @@ -28,9 +28,11 @@ class Decision: attributes: Attributes to add to the `Span`. """ - def __repr__(self): + def __repr__(self) -> str: return "{}({}, attributes={})".format( - type(self).__name__, self.sampled, self.attributes + type(self).__name__, + str(self.sampled), + str(self.attributes) ) def __init__( @@ -38,15 +40,17 @@ def __init__( sampled: bool = False, attributes: Mapping[str, "AttributeValue"] = None, ) -> None: + self.sampled: bool + self.attributes: Dict[str, "AttributeValue"] + self.sampled = sampled if attributes is None: - self.attributes = {} # type:Dict[str, "AttributeValue"] + self.attributes = {} else: self.attributes = dict(attributes) class Sampler(abc.ABC): - @abc.abstractmethod def should_sample( self, From 9ac45162227846aa0ca10410616ec4e445a7470a Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 16 Oct 2019 15:25:58 -0700 Subject: [PATCH 03/24] Use sampler in span creation --- .../src/opentelemetry/sdk/trace/__init__.py | 66 ++++++++++++++----- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index c7b33a353db..259d9b19f4b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -19,6 +19,7 @@ import typing from contextlib import contextmanager +import opentelemetry.trace.sampling as sampling from opentelemetry import trace as trace_api from opentelemetry.context import Context from opentelemetry.sdk import util @@ -104,7 +105,7 @@ class Span(trace_api.Span): context: The immutable span context parent: This span's parent, may be a `SpanContext` if the parent is remote, null if this is a root span - sampler: TODO + sampler: The sampler used to create this span trace_config: TODO resource: TODO attributes: The span's attributes to be exported @@ -124,7 +125,7 @@ def __init__( name: str, context: trace_api.SpanContext, parent: trace_api.ParentSpan = None, - sampler: None = None, # TODO + sampler: Optional[sampling.Sampler] = None, trace_config: None = None, # TODO resource: None = None, # TODO attributes: types.Attributes = None, # TODO @@ -311,12 +312,17 @@ class Tracer(trace_api.Tracer): name: The name of the tracer. """ - def __init__(self, name: str = "") -> None: + def __init__( + self, + name: str = "", + sampler: "sampling.Sampler" = trace_api.sampling.ALWAYS_ON, + ) -> None: slot_name = "current_span" if name: slot_name = "{}.current_span".format(name) self._current_span_slot = Context.register_slot(slot_name) self._active_span_processor = MultiSpanProcessor() + self.sampler = sampler def get_current_span(self): """See `opentelemetry.trace.Tracer.get_current_span`.""" @@ -350,34 +356,64 @@ def create_span( name: str, parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN, kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, - ) -> "Span": - """See `opentelemetry.trace.Tracer.create_span`.""" - span_id = generate_span_id() + ) -> "trace_api.Span": + """See `opentelemetry.trace.Tracer.create_span`. + + If `parent` is null the new span will be created as a root span, i.e. a + span with no parent context. By default, the new span will be created + as a child of the current span in this tracer's context, or as a root + span if no current span exists. + """ + if parent is Tracer.CURRENT_SPAN: parent = self.get_current_span() + if parent is None: - context = trace_api.SpanContext(generate_trace_id(), span_id) + parent_context = None + new_span_context = trace_api.SpanContext( + generate_trace_id(), generate_span_id() + ) else: if isinstance(parent, trace_api.Span): parent_context = parent.get_context() elif isinstance(parent, trace_api.SpanContext): parent_context = parent else: + # But don't... raise TypeError - context = trace_api.SpanContext( + + new_span_context = trace_api.SpanContext( parent_context.trace_id, - span_id, + generate_span_id(), parent_context.trace_options, parent_context.trace_state, ) - return Span( - name=name, - context=context, - parent=parent, - span_processor=self._active_span_processor, - kind=kind, + + # 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.sampler.should_sample( + parent_context, + new_span_context.trace_id, + new_span_context.span_id, + name, + {}, # TODO: links ) + if sampling_decision.sampled: + return Span( + name=name, + context=new_span_context, + parent=parent, + sampler=self.sampler, + attributes=sampling_decision.attributes, + span_processor=self._active_span_processor, + kind=kind, + ) + return trace_api.DefaultSpan(context=new_span_context) + @contextmanager def use_span( self, span: trace_api.Span, end_on_exit: bool = False From 5e0370a3132cdb9d8cfeb956dc605277f69fc309 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 16 Oct 2019 15:26:47 -0700 Subject: [PATCH 04/24] Docstring updates --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 1b0a3e758f7..28011efca00 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -307,8 +307,8 @@ class SpanContext: Args: trace_id: The ID of the trace that this span belongs to. span_id: This span's ID. - options: Trace options to propagate. - state: Tracing-system-specific info to propagate. + trace_options: Trace options to propagate. + trace_state: Tracing-system-specific info to propagate. """ def __init__( From bc8782d960a23f90d06ccee900b4a6c574a0dfb9 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 16 Oct 2019 15:27:04 -0700 Subject: [PATCH 05/24] DefaultSpan doesn't record events --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 28011efca00..01a25ce2710 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -361,6 +361,9 @@ def __init__(self, context: "SpanContext") -> None: def get_context(self) -> "SpanContext": return self._context + def is_recording_events(self) -> bool: + return False + INVALID_SPAN_ID = 0x0000000000000000 INVALID_TRACE_ID = 0x00000000000000000000000000000000 From 9a8f3c1236323464055badaef4ce51367c6ba150 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 16 Oct 2019 15:32:12 -0700 Subject: [PATCH 06/24] Splat typing imports --- .../src/opentelemetry/sdk/trace/__init__.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 259d9b19f4b..f12ca5ecebc 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -16,8 +16,8 @@ import logging import random import threading -import typing from contextlib import contextmanager +from typing import Iterator, Optional, Sequence, Tuple import opentelemetry.trace.sampling as sampling from opentelemetry import trace as trace_api @@ -74,7 +74,7 @@ class MultiSpanProcessor(SpanProcessor): def __init__(self): # use a tuple to avoid race conditions when adding a new span and # iterating through it on "on_start" and "on_end". - self._span_processors = () # type: typing.Tuple[SpanProcessor, ...] + self._span_processors = () # type: Tuple[SpanProcessor, ...] self._lock = threading.Lock() def add_span_processor(self, span_processor: SpanProcessor) -> None: @@ -129,8 +129,8 @@ def __init__( trace_config: None = None, # TODO resource: None = None, # TODO attributes: types.Attributes = None, # TODO - events: typing.Sequence[trace_api.Event] = None, # TODO - links: typing.Sequence[trace_api.Link] = None, # TODO + events: Sequence[trace_api.Event] = None, # TODO + links: Sequence[trace_api.Link] = None, # TODO kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, span_processor: SpanProcessor = SpanProcessor(), ) -> None: @@ -163,8 +163,8 @@ def __init__( else: self.links = BoundedList.from_seq(MAX_NUM_LINKS, links) - self.end_time = None # type: typing.Optional[int] - self.start_time = None # type: typing.Optional[int] + self.end_time = None # type: Optional[int] + self.start_time = None # type: Optional[int] def __repr__(self): return '{}(name="{}", context={})'.format( @@ -246,7 +246,7 @@ def add_lazy_link(self, link: "trace_api.Link") -> None: return self.links.append(link) - def start(self, start_time: typing.Optional[int] = None) -> None: + def start(self, start_time: Optional[int] = None) -> None: with self._lock: if not self.is_recording_events(): return @@ -315,7 +315,7 @@ class Tracer(trace_api.Tracer): def __init__( self, name: str = "", - sampler: "sampling.Sampler" = trace_api.sampling.ALWAYS_ON, + sampler: sampling.Sampler = trace_api.sampling.ALWAYS_ON, ) -> None: slot_name = "current_span" if name: @@ -345,7 +345,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, - ) -> typing.Iterator[trace_api.Span]: + ) -> Iterator[trace_api.Span]: """See `opentelemetry.trace.Tracer.start_as_current_span`.""" span = self.start_span(name, parent, kind) @@ -417,7 +417,7 @@ def create_span( @contextmanager def use_span( self, span: trace_api.Span, end_on_exit: bool = False - ) -> typing.Iterator[trace_api.Span]: + ) -> Iterator[trace_api.Span]: """See `opentelemetry.trace.Tracer.use_span`.""" try: span_snapshot = self._current_span_slot.get() From 4264f16e3a08375fe646be2117dcda89b0b8b21d Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 16 Oct 2019 16:06:38 -0700 Subject: [PATCH 07/24] Add ProbabilitySampler --- .../src/opentelemetry/trace/__init__.py | 4 ++ .../src/opentelemetry/trace/sampling.py | 41 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 01a25ce2710..d4e40450d14 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -267,6 +267,10 @@ class TraceOptions(int): def get_default(cls) -> "TraceOptions": return cls(cls.DEFAULT) + @property + def recorded(self) -> bool: + return bool(self & TraceOptions.RECORDED) + DEFAULT_TRACE_OPTIONS = TraceOptions.get_default() diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index 8df33a7ad23..341a8ab434c 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/opentelemetry-api/src/opentelemetry/trace/sampling.py @@ -80,5 +80,46 @@ def should_sample( return self.decision +class ProbabilitySampler(Sampler): + def __init__(self, rate: float): + self._rate = rate + self._bound = self.get_bound_for_rate(self._rate) + + @classmethod + def get_bound_for_rate(cls, rate: float) -> int: + return int(rate * 0xffffffffffffffff) + + @property + def rate(self) -> float: + return self._rate + + @rate.setter + def rate(self, new_rate: float) -> None: + self._rate = new_rate + self._bound = self.get_bound_for_rate(self._rate) + + @property + def bound(self) -> int: + return self._bound + + def should_sample( + self, + parent_context: Optional["SpanContext"], + trace_id: int, + span_id: int, + name: str, + links: Optional[Sequence["Link"]] = None, + ) -> "Decision": + if parent_context is not None: + return Decision(parent_context.trace_options.recorded, {}) + + return Decision(trace_id & 0xffffffffffffffff <= self.bound, {}) + +# Samplers that ignore the parent sampling decision and never/always sample. ALWAYS_OFF = StaticSampler(Decision(False)) ALWAYS_ON = StaticSampler(Decision(True)) + +# Samplers that respect the parent sampling decision, but otherwise +# never/always sample. +DEFAULT_OFF = ProbabilitySampler(0.0) +DEFAULT_ON = ProbabilitySampler(1.0) From 567a7dac763e5a0c6ecd8f9b4434bb9a6e6065f9 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 16 Oct 2019 16:46:08 -0700 Subject: [PATCH 08/24] Add ProbabilitySampler tests --- .../tests/trace/test_sampling.py | 89 +++++++++++++++---- 1 file changed, 73 insertions(+), 16 deletions(-) diff --git a/opentelemetry-api/tests/trace/test_sampling.py b/opentelemetry-api/tests/trace/test_sampling.py index 7258437f0a4..7f58c4cbffe 100644 --- a/opentelemetry-api/tests/trace/test_sampling.py +++ b/opentelemetry-api/tests/trace/test_sampling.py @@ -17,58 +17,115 @@ from opentelemetry import trace from opentelemetry.trace import sampling +TO_DEFAULT = trace.TraceOptions(trace.TraceOptions.DEFAULT) +TO_RECORDED = trace.TraceOptions(trace.TraceOptions.RECORDED) + class TestSampler(unittest.TestCase): def test_always_on(self): - unrecorded_on = sampling.ALWAYS_ON.should_sample( + no_record_always_on = sampling.ALWAYS_ON.should_sample( trace.SpanContext( 0x000000000000000000000000DEADBEEF, 0x00000000DEADBEF0, - trace_options=trace.TraceOptions.DEFAULT, + trace_options=TO_DEFAULT, ), 0x000000000000000000000000DEADBEF1, 0x00000000DEADBEF2, "unrecorded parent, sampling on", ) - self.assertTrue(unrecorded_on.sampled) - self.assertEqual(unrecorded_on.attributes, {}) + self.assertTrue(no_record_always_on.sampled) + self.assertEqual(no_record_always_on.attributes, {}) - recorded_on = sampling.ALWAYS_ON.should_sample( + recorded_always_on = sampling.ALWAYS_ON.should_sample( trace.SpanContext( 0x000000000000000000000000DEADBEF3, 0x00000000DEADBEF4, - trace_options=trace.TraceOptions.DEFAULT, + trace_options=TO_RECORDED, ), 0x000000000000000000000000DEADBEF5, 0x00000000DEADBEF6, "recorded parent, sampling on", ) - self.assertTrue(recorded_on.sampled) - self.assertEqual(recorded_on.attributes, {}) + self.assertTrue(recorded_always_on.sampled) + self.assertEqual(recorded_always_on.attributes, {}) def test_always_off(self): - unrecorded_off = sampling.ALWAYS_ON.should_sample( + no_record_always_off = sampling.ALWAYS_OFF.should_sample( + trace.SpanContext( + 0x000000000000000000000000DEADBEF7, + 0x00000000DEADBEF8, + trace_options=TO_DEFAULT, + ), + 0x000000000000000000000000DEADBEF9, + 0x00000000DEADBEFA, + "unrecorded parent, sampling off", + ) + self.assertFalse(no_record_always_off.sampled) + self.assertEqual(no_record_always_off.attributes, {}) + + recorded_always_on = sampling.ALWAYS_OFF.should_sample( trace.SpanContext( 0x000000000000000000000000DEADBEF7, 0x00000000DEADBEF8, - trace_options=trace.TraceOptions.DEFAULT, + trace_options=TO_RECORDED, ), 0x000000000000000000000000DEADBEF9, 0x00000000DEADBEFA, "recorded parent, sampling off", ) - self.assertTrue(unrecorded_off.sampled) - self.assertEqual(unrecorded_off.attributes, {}) + self.assertFalse(recorded_always_on.sampled) + self.assertEqual(recorded_always_on.attributes, {}) + + def test_default_on(self): + no_record_default_on = sampling.DEFAULT_ON.should_sample( + trace.SpanContext( + 0x000000000000000000000000DEADBEEF, + 0x00000000DEADBEF0, + trace_options=TO_DEFAULT, + ), + 0x000000000000000000000000DEADBEF1, + 0x00000000DEADBEF2, + "unrecorded parent, sampling on", + ) + self.assertFalse(no_record_default_on.sampled) + self.assertEqual(no_record_default_on.attributes, {}) - recorded_off = sampling.ALWAYS_ON.should_sample( + recorded_default_on = sampling.DEFAULT_ON.should_sample( + trace.SpanContext( + 0x000000000000000000000000DEADBEF3, + 0x00000000DEADBEF4, + trace_options=TO_RECORDED, + ), + 0x000000000000000000000000DEADBEF5, + 0x00000000DEADBEF6, + "recorded parent, sampling on", + ) + self.assertTrue(recorded_default_on.sampled) + self.assertEqual(recorded_default_on.attributes, {}) + + def test_default_off(self): + no_record_default_off = sampling.DEFAULT_OFF.should_sample( trace.SpanContext( 0x000000000000000000000000DEADBEF7, 0x00000000DEADBEF8, - trace_options=trace.TraceOptions.RECORDED, + trace_options=TO_DEFAULT, ), 0x000000000000000000000000DEADBEF9, 0x00000000DEADBEFA, "unrecorded parent, sampling off", ) - self.assertTrue(recorded_off.sampled) - self.assertEqual(recorded_off.attributes, {}) + self.assertFalse(no_record_default_off.sampled) + self.assertEqual(no_record_default_off.attributes, {}) + + recorded_default_off = sampling.DEFAULT_OFF.should_sample( + trace.SpanContext( + 0x000000000000000000000000DEADBEF7, + 0x00000000DEADBEF8, + trace_options=TO_RECORDED, + ), + 0x000000000000000000000000DEADBEF9, + 0x00000000DEADBEFA, + "recorded parent, sampling off", + ) + self.assertTrue(recorded_default_off.sampled) + self.assertEqual(recorded_default_off.attributes, {}) From e743fdc0ed643f72d04d71166b4d66e2ff51b728 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 16 Oct 2019 16:50:06 -0700 Subject: [PATCH 09/24] s/ALWAYS_ON/DEFAULT_ON/ --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index f12ca5ecebc..ecbcb1da7f7 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -315,7 +315,7 @@ class Tracer(trace_api.Tracer): def __init__( self, name: str = "", - sampler: sampling.Sampler = trace_api.sampling.ALWAYS_ON, + sampler: sampling.Sampler = trace_api.sampling.DEFAULT_ON, ) -> None: slot_name = "current_span" if name: From d90a8bc09f13bd74130517478e42e1e7c0165845 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 17 Oct 2019 15:15:26 -0700 Subject: [PATCH 10/24] Catch a sneaky bug, add ProbabilitySampler tests --- .../src/opentelemetry/trace/sampling.py | 13 +- .../tests/trace/test_sampling.py | 199 +++++++++++++++--- 2 files changed, 175 insertions(+), 37 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index 341a8ab434c..390873114a1 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/opentelemetry-api/src/opentelemetry/trace/sampling.py @@ -30,9 +30,7 @@ class Decision: def __repr__(self) -> str: return "{}({}, attributes={})".format( - type(self).__name__, - str(self.sampled), - str(self.attributes) + type(self).__name__, str(self.sampled), str(self.attributes) ) def __init__( @@ -85,9 +83,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 + @classmethod def get_bound_for_rate(cls, rate: float) -> int: - return int(rate * 0xffffffffffffffff) + return round(rate * (cls.CHECK_BYTES + 1)) @property def rate(self) -> float: @@ -113,7 +115,8 @@ def should_sample( if parent_context is not None: return Decision(parent_context.trace_options.recorded, {}) - return Decision(trace_id & 0xffffffffffffffff <= self.bound, {}) + return Decision(trace_id & self.CHECK_BYTES < self.bound, {}) + # Samplers that ignore the parent sampling decision and never/always sample. ALWAYS_OFF = StaticSampler(Decision(False)) diff --git a/opentelemetry-api/tests/trace/test_sampling.py b/opentelemetry-api/tests/trace/test_sampling.py index 7f58c4cbffe..9a89698a388 100644 --- a/opentelemetry-api/tests/trace/test_sampling.py +++ b/opentelemetry-api/tests/trace/test_sampling.py @@ -25,12 +25,12 @@ class TestSampler(unittest.TestCase): def test_always_on(self): no_record_always_on = sampling.ALWAYS_ON.should_sample( trace.SpanContext( - 0x000000000000000000000000DEADBEEF, - 0x00000000DEADBEF0, + 0xdeadbeef, + 0xdeadbef0, trace_options=TO_DEFAULT, ), - 0x000000000000000000000000DEADBEF1, - 0x00000000DEADBEF2, + 0xdeadbef1, + 0xdeadbef2, "unrecorded parent, sampling on", ) self.assertTrue(no_record_always_on.sampled) @@ -38,12 +38,12 @@ def test_always_on(self): recorded_always_on = sampling.ALWAYS_ON.should_sample( trace.SpanContext( - 0x000000000000000000000000DEADBEF3, - 0x00000000DEADBEF4, + 0xdeadbeef, + 0xdeadbef0, trace_options=TO_RECORDED, ), - 0x000000000000000000000000DEADBEF5, - 0x00000000DEADBEF6, + 0xdeadbef1, + 0xdeadbef2, "recorded parent, sampling on", ) self.assertTrue(recorded_always_on.sampled) @@ -52,12 +52,12 @@ def test_always_on(self): def test_always_off(self): no_record_always_off = sampling.ALWAYS_OFF.should_sample( trace.SpanContext( - 0x000000000000000000000000DEADBEF7, - 0x00000000DEADBEF8, + 0xdeadbeef, + 0xdeadbef0, trace_options=TO_DEFAULT, ), - 0x000000000000000000000000DEADBEF9, - 0x00000000DEADBEFA, + 0xdeadbef1, + 0xdeadbef2, "unrecorded parent, sampling off", ) self.assertFalse(no_record_always_off.sampled) @@ -65,12 +65,12 @@ def test_always_off(self): recorded_always_on = sampling.ALWAYS_OFF.should_sample( trace.SpanContext( - 0x000000000000000000000000DEADBEF7, - 0x00000000DEADBEF8, + 0xdeadbeef, + 0xdeadbef0, trace_options=TO_RECORDED, ), - 0x000000000000000000000000DEADBEF9, - 0x00000000DEADBEFA, + 0xdeadbef1, + 0xdeadbef2, "recorded parent, sampling off", ) self.assertFalse(recorded_always_on.sampled) @@ -79,12 +79,12 @@ def test_always_off(self): def test_default_on(self): no_record_default_on = sampling.DEFAULT_ON.should_sample( trace.SpanContext( - 0x000000000000000000000000DEADBEEF, - 0x00000000DEADBEF0, + 0xdeadbeef, + 0xdeadbef0, trace_options=TO_DEFAULT, ), - 0x000000000000000000000000DEADBEF1, - 0x00000000DEADBEF2, + 0xdeadbef1, + 0xdeadbef2, "unrecorded parent, sampling on", ) self.assertFalse(no_record_default_on.sampled) @@ -92,12 +92,12 @@ def test_default_on(self): recorded_default_on = sampling.DEFAULT_ON.should_sample( trace.SpanContext( - 0x000000000000000000000000DEADBEF3, - 0x00000000DEADBEF4, + 0xdeadbeef, + 0xdeadbef0, trace_options=TO_RECORDED, ), - 0x000000000000000000000000DEADBEF5, - 0x00000000DEADBEF6, + 0xdeadbef1, + 0xdeadbef2, "recorded parent, sampling on", ) self.assertTrue(recorded_default_on.sampled) @@ -106,12 +106,12 @@ def test_default_on(self): def test_default_off(self): no_record_default_off = sampling.DEFAULT_OFF.should_sample( trace.SpanContext( - 0x000000000000000000000000DEADBEF7, - 0x00000000DEADBEF8, + 0xdeadbeef, + 0xdeadbef0, trace_options=TO_DEFAULT, ), - 0x000000000000000000000000DEADBEF9, - 0x00000000DEADBEFA, + 0xdeadbef1, + 0xdeadbef2, "unrecorded parent, sampling off", ) self.assertFalse(no_record_default_off.sampled) @@ -119,13 +119,148 @@ def test_default_off(self): recorded_default_off = sampling.DEFAULT_OFF.should_sample( trace.SpanContext( - 0x000000000000000000000000DEADBEF7, - 0x00000000DEADBEF8, + 0xdeadbeef, + 0xdeadbef0, trace_options=TO_RECORDED, ), - 0x000000000000000000000000DEADBEF9, - 0x00000000DEADBEFA, + 0xdeadbef1, + 0xdeadbef2, "recorded parent, sampling off", ) self.assertTrue(recorded_default_off.sampled) self.assertEqual(recorded_default_off.attributes, {}) + + def test_probability_sampler(self): + sampler = sampling.ProbabilitySampler(.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 + ) + 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, + trace_options=TO_DEFAULT, + ), + 0x8000000000000000, + 0xdeadbeef, + "span name", + ).sampled + ) + self.assertTrue( + sampler.should_sample( + trace.SpanContext( + 0xdeadbef0, + 0xdeadbef1, + trace_options=TO_RECORDED, + ), + 0x8000000000000001, + 0xdeadbeef, + "span name", + ).sampled + ) + + + def test_probability_sampler_zero(self): + default_off = sampling.ProbabilitySampler(0.0) + self.assertFalse( + default_off.should_sample( + None, + 0x0, + 0xdeadbeef, + "span name", + ).sampled + ) + + def test_probability_sampler_one(self): + default_off = sampling.ProbabilitySampler(1.0) + self.assertTrue( + default_off.should_sample( + None, + 0xffffffffffffffff, + 0xdeadbeef, + "span name", + ).sampled + ) + + 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(1 / 2 ** 64) + self.assertTrue( + almost_always_off.should_sample( + None, + 0x0, + 0xdeadbeef, + "span name", + ).sampled + ) + self.assertFalse( + almost_always_off.should_sample( + None, + 0x1, + 0xdeadbeef, + "span name", + ).sampled + ) + self.assertEqual( + sampling.ProbabilitySampler.get_bound_for_rate(1 / 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! + # + # In practice, the highest possible sampling rate is: + # + # round(sys.float_info.epsilon * 2 ** 64) + + almost_always_on = sampling.ProbabilitySampler(1 - (1 / 2 ** 64)) + self.assertTrue( + almost_always_on.should_sample( + None, + 0xfffffffffffffffe, + 0xdeadbeef, + "span name", + ).sampled + ) + + # These tests are logically consistent, but fail because of the float + # precision issue above. Changing the sampler to check fewer bytes of + # the trace ID will cause these to pass. + + # self.assertFalse( + # almost_always_on.should_sample( + # None, + # 0xffffffffffffffff, + # 0xdeadbeef, + # "span name", + # ).sampled + # ) + # self.assertEqual( + # sampling.ProbabilitySampler.get_bound_for_rate(1 - (1 / 2 ** 64)), + # 0xffffffffffffffff, + # ) From 7e0330b36f54436726ba1018cabdcb6a0285e930 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 17 Oct 2019 15:20:06 -0700 Subject: [PATCH 11/24] Uglify via black --- .../tests/trace/test_sampling.py | 117 ++++++------------ 1 file changed, 37 insertions(+), 80 deletions(-) diff --git a/opentelemetry-api/tests/trace/test_sampling.py b/opentelemetry-api/tests/trace/test_sampling.py index 9a89698a388..6e13a161ded 100644 --- a/opentelemetry-api/tests/trace/test_sampling.py +++ b/opentelemetry-api/tests/trace/test_sampling.py @@ -25,12 +25,10 @@ class TestSampler(unittest.TestCase): def test_always_on(self): no_record_always_on = sampling.ALWAYS_ON.should_sample( trace.SpanContext( - 0xdeadbeef, - 0xdeadbef0, - trace_options=TO_DEFAULT, + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_DEFAULT ), - 0xdeadbef1, - 0xdeadbef2, + 0xDEADBEF1, + 0xDEADBEF2, "unrecorded parent, sampling on", ) self.assertTrue(no_record_always_on.sampled) @@ -38,12 +36,10 @@ def test_always_on(self): recorded_always_on = sampling.ALWAYS_ON.should_sample( trace.SpanContext( - 0xdeadbeef, - 0xdeadbef0, - trace_options=TO_RECORDED, + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_RECORDED ), - 0xdeadbef1, - 0xdeadbef2, + 0xDEADBEF1, + 0xDEADBEF2, "recorded parent, sampling on", ) self.assertTrue(recorded_always_on.sampled) @@ -52,12 +48,10 @@ def test_always_on(self): def test_always_off(self): no_record_always_off = sampling.ALWAYS_OFF.should_sample( trace.SpanContext( - 0xdeadbeef, - 0xdeadbef0, - trace_options=TO_DEFAULT, + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_DEFAULT ), - 0xdeadbef1, - 0xdeadbef2, + 0xDEADBEF1, + 0xDEADBEF2, "unrecorded parent, sampling off", ) self.assertFalse(no_record_always_off.sampled) @@ -65,12 +59,10 @@ def test_always_off(self): recorded_always_on = sampling.ALWAYS_OFF.should_sample( trace.SpanContext( - 0xdeadbeef, - 0xdeadbef0, - trace_options=TO_RECORDED, + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_RECORDED ), - 0xdeadbef1, - 0xdeadbef2, + 0xDEADBEF1, + 0xDEADBEF2, "recorded parent, sampling off", ) self.assertFalse(recorded_always_on.sampled) @@ -79,12 +71,10 @@ def test_always_off(self): def test_default_on(self): no_record_default_on = sampling.DEFAULT_ON.should_sample( trace.SpanContext( - 0xdeadbeef, - 0xdeadbef0, - trace_options=TO_DEFAULT, + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_DEFAULT ), - 0xdeadbef1, - 0xdeadbef2, + 0xDEADBEF1, + 0xDEADBEF2, "unrecorded parent, sampling on", ) self.assertFalse(no_record_default_on.sampled) @@ -92,12 +82,10 @@ def test_default_on(self): recorded_default_on = sampling.DEFAULT_ON.should_sample( trace.SpanContext( - 0xdeadbeef, - 0xdeadbef0, - trace_options=TO_RECORDED, + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_RECORDED ), - 0xdeadbef1, - 0xdeadbef2, + 0xDEADBEF1, + 0xDEADBEF2, "recorded parent, sampling on", ) self.assertTrue(recorded_default_on.sampled) @@ -106,12 +94,10 @@ def test_default_on(self): def test_default_off(self): no_record_default_off = sampling.DEFAULT_OFF.should_sample( trace.SpanContext( - 0xdeadbeef, - 0xdeadbef0, - trace_options=TO_DEFAULT, + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_DEFAULT ), - 0xdeadbef1, - 0xdeadbef2, + 0xDEADBEF1, + 0xDEADBEF2, "unrecorded parent, sampling off", ) self.assertFalse(no_record_default_off.sampled) @@ -119,36 +105,28 @@ def test_default_off(self): recorded_default_off = sampling.DEFAULT_OFF.should_sample( trace.SpanContext( - 0xdeadbeef, - 0xdeadbef0, - trace_options=TO_RECORDED, + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_RECORDED ), - 0xdeadbef1, - 0xdeadbef2, + 0xDEADBEF1, + 0xDEADBEF2, "recorded parent, sampling off", ) self.assertTrue(recorded_default_off.sampled) self.assertEqual(recorded_default_off.attributes, {}) def test_probability_sampler(self): - sampler = sampling.ProbabilitySampler(.5) + sampler = sampling.ProbabilitySampler(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", + None, 0x7FFFFFFFFFFFFFFF, 0xDEADBEEF, "span name" ).sampled ) self.assertFalse( sampler.should_sample( - None, - 0x8000000000000000, - 0xdeadbeef, - "span name", + None, 0x8000000000000000, 0xDEADBEEF, "span name" ).sampled ) @@ -157,37 +135,29 @@ def test_probability_sampler(self): self.assertFalse( sampler.should_sample( trace.SpanContext( - 0xdeadbef0, - 0xdeadbef1, - trace_options=TO_DEFAULT, + 0xDEADBEF0, 0xDEADBEF1, trace_options=TO_DEFAULT ), 0x8000000000000000, - 0xdeadbeef, + 0xDEADBEEF, "span name", ).sampled ) self.assertTrue( sampler.should_sample( trace.SpanContext( - 0xdeadbef0, - 0xdeadbef1, - trace_options=TO_RECORDED, + 0xDEADBEF0, 0xDEADBEF1, trace_options=TO_RECORDED ), 0x8000000000000001, - 0xdeadbeef, + 0xDEADBEEF, "span name", ).sampled ) - def test_probability_sampler_zero(self): default_off = sampling.ProbabilitySampler(0.0) self.assertFalse( default_off.should_sample( - None, - 0x0, - 0xdeadbeef, - "span name", + None, 0x0, 0xDEADBEEF, "span name" ).sampled ) @@ -195,10 +165,7 @@ def test_probability_sampler_one(self): default_off = sampling.ProbabilitySampler(1.0) self.assertTrue( default_off.should_sample( - None, - 0xffffffffffffffff, - 0xdeadbeef, - "span name", + None, 0xFFFFFFFFFFFFFFFF, 0xDEADBEEF, "span name" ).sampled ) @@ -210,23 +177,16 @@ def test_probability_sampler_limits(self): almost_always_off = sampling.ProbabilitySampler(1 / 2 ** 64) self.assertTrue( almost_always_off.should_sample( - None, - 0x0, - 0xdeadbeef, - "span name", + None, 0x0, 0xDEADBEEF, "span name" ).sampled ) self.assertFalse( almost_always_off.should_sample( - None, - 0x1, - 0xdeadbeef, - "span name", + None, 0x1, 0xDEADBEEF, "span name" ).sampled ) self.assertEqual( - sampling.ProbabilitySampler.get_bound_for_rate(1 / 2 ** 64), - 0x1 + sampling.ProbabilitySampler.get_bound_for_rate(1 / 2 ** 64), 0x1 ) # Sample every trace with (last 8 bytes of) trace ID less than @@ -241,10 +201,7 @@ def test_probability_sampler_limits(self): almost_always_on = sampling.ProbabilitySampler(1 - (1 / 2 ** 64)) self.assertTrue( almost_always_on.should_sample( - None, - 0xfffffffffffffffe, - 0xdeadbeef, - "span name", + None, 0xFFFFFFFFFFFFFFFE, 0xDEADBEEF, "span name" ).sampled ) From a0978b22b50675073911cec99354fa5a078187c6 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 17 Oct 2019 15:31:38 -0700 Subject: [PATCH 12/24] Revert "s/ALWAYS_ON/DEFAULT_ON/" This reverts commit e743fdc0ed643f72d04d71166b4d66e2ff51b728. --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index ecbcb1da7f7..f12ca5ecebc 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -315,7 +315,7 @@ class Tracer(trace_api.Tracer): def __init__( self, name: str = "", - sampler: sampling.Sampler = trace_api.sampling.DEFAULT_ON, + sampler: sampling.Sampler = trace_api.sampling.ALWAYS_ON, ) -> None: slot_name = "current_span" if name: From 7b1fccbab573fc5d4fde32d46efc2cfc95a82435 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 17 Oct 2019 15:48:56 -0700 Subject: [PATCH 13/24] Add trace sampler tests --- opentelemetry-sdk/tests/trace/test_trace.py | 24 +++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index b55ea897fab..2bbf001bc89 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -15,6 +15,7 @@ import unittest from unittest import mock +import opentelemetry.trace.sampling as sampling from opentelemetry import trace as trace_api from opentelemetry.sdk import trace from opentelemetry.util import time_ns @@ -26,6 +27,29 @@ def test_extends_api(self): self.assertIsInstance(tracer, trace_api.Tracer) +class TestTracerSampling(unittest.TestCase): + def test_default_sampler(self): + tracer = trace.Tracer() + + # Check that the default tracer creates real spans via the default + # sampler + root_span = tracer.create_span(name="root span", parent=None) + self.assertIsInstance(root_span, trace.Span) + child_span = tracer.create_span(name="child span", parent=root_span) + self.assertIsInstance(child_span, trace.Span) + + def test_sampler_no_sampling(self): + tracer = trace.Tracer() + tracer.sampler = sampling.ALWAYS_OFF + + # Check that the default tracer creates no-op spans if the sampler + # decides not to sampler + root_span = tracer.create_span(name="root span", parent=None) + self.assertIsInstance(root_span, trace_api.DefaultSpan) + child_span = tracer.create_span(name="child span", parent=root_span) + self.assertIsInstance(child_span, trace_api.DefaultSpan) + + class TestSpanCreation(unittest.TestCase): def test_start_span_implicit(self): tracer = trace.Tracer("test_start_span_implicit") From 9b935adbcf099e6fb7ca77d6da79878889460f3e Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 17 Oct 2019 16:05:59 -0700 Subject: [PATCH 14/24] Less flippant comment --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index f12ca5ecebc..94a53c2867e 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -379,7 +379,7 @@ def create_span( elif isinstance(parent, trace_api.SpanContext): parent_context = parent else: - # But don't... + # TODO: error handling raise TypeError new_span_context = trace_api.SpanContext( From 8de632d840fa2d13ccd2ab3e1a8c5e12fc02e22e Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 17 Oct 2019 16:08:54 -0700 Subject: [PATCH 15/24] Shuffle imports --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 2 +- opentelemetry-sdk/tests/trace/test_trace.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 94a53c2867e..461203ede65 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -19,11 +19,11 @@ from contextlib import contextmanager from typing import Iterator, Optional, Sequence, Tuple -import opentelemetry.trace.sampling as sampling from opentelemetry import trace as trace_api from opentelemetry.context import Context from opentelemetry.sdk import util from opentelemetry.sdk.util import BoundedDict, BoundedList +from opentelemetry.trace import sampling from opentelemetry.util import time_ns, types logger = logging.getLogger(__name__) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 2bbf001bc89..32e227c7755 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -15,9 +15,9 @@ import unittest from unittest import mock -import opentelemetry.trace.sampling as sampling from opentelemetry import trace as trace_api from opentelemetry.sdk import trace +from opentelemetry.trace import sampling from opentelemetry.util import time_ns From 644cf9d46776fc5f9a84652793122863428c200b Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Fri, 18 Oct 2019 12:24:52 -0700 Subject: [PATCH 16/24] Apply suggestions from code review Co-Authored-By: Reiley Yang --- opentelemetry-api/src/opentelemetry/trace/sampling.py | 4 ++-- opentelemetry-api/tests/trace/test_sampling.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index 390873114a1..bde7eb3de4a 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/opentelemetry-api/src/opentelemetry/trace/sampling.py @@ -65,7 +65,7 @@ class StaticSampler(Sampler): """Sampler that always returns the same decision.""" def __init__(self, decision: "Decision"): - self.decision = decision + self._decision = decision def should_sample( self, @@ -113,7 +113,7 @@ def should_sample( links: Optional[Sequence["Link"]] = None, ) -> "Decision": if parent_context is not None: - return Decision(parent_context.trace_options.recorded, {}) + return Decision(parent_context.trace_options.recorded) return Decision(trace_id & self.CHECK_BYTES < self.bound, {}) diff --git a/opentelemetry-api/tests/trace/test_sampling.py b/opentelemetry-api/tests/trace/test_sampling.py index 6e13a161ded..9cf5b95ca45 100644 --- a/opentelemetry-api/tests/trace/test_sampling.py +++ b/opentelemetry-api/tests/trace/test_sampling.py @@ -174,7 +174,7 @@ 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(1 / 2 ** 64) + almost_always_off = sampling.ProbabilitySampler(2 ** -64) self.assertTrue( almost_always_off.should_sample( None, 0x0, 0xDEADBEEF, "span name" From 6f0a33df18e6680fdbcede682a561c2856f69766 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Fri, 18 Oct 2019 12:37:27 -0700 Subject: [PATCH 17/24] Follow up on @reyang's comments --- opentelemetry-api/src/opentelemetry/trace/sampling.py | 2 +- opentelemetry-api/tests/trace/test_sampling.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index bde7eb3de4a..645cc2400d0 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/opentelemetry-api/src/opentelemetry/trace/sampling.py @@ -75,7 +75,7 @@ def should_sample( name: str, links: Optional[Sequence["Link"]] = None, ) -> "Decision": - return self.decision + return self._decision class ProbabilitySampler(Sampler): diff --git a/opentelemetry-api/tests/trace/test_sampling.py b/opentelemetry-api/tests/trace/test_sampling.py index 9cf5b95ca45..f3878c02ddb 100644 --- a/opentelemetry-api/tests/trace/test_sampling.py +++ b/opentelemetry-api/tests/trace/test_sampling.py @@ -186,7 +186,7 @@ def test_probability_sampler_limits(self): ).sampled ) self.assertEqual( - sampling.ProbabilitySampler.get_bound_for_rate(1 / 2 ** 64), 0x1 + sampling.ProbabilitySampler.get_bound_for_rate(2 ** -64), 0x1 ) # Sample every trace with (last 8 bytes of) trace ID less than @@ -198,7 +198,7 @@ def test_probability_sampler_limits(self): # # round(sys.float_info.epsilon * 2 ** 64) - almost_always_on = sampling.ProbabilitySampler(1 - (1 / 2 ** 64)) + almost_always_on = sampling.ProbabilitySampler(1 - 2 ** -64) self.assertTrue( almost_always_on.should_sample( None, 0xFFFFFFFFFFFFFFFE, 0xDEADBEEF, "span name" @@ -218,6 +218,6 @@ def test_probability_sampler_limits(self): # ).sampled # ) # self.assertEqual( - # sampling.ProbabilitySampler.get_bound_for_rate(1 - (1 / 2 ** 64)), + # sampling.ProbabilitySampler.get_bound_for_rate(1 - 2 ** -64)), # 0xffffffffffffffff, # ) From 0de147e9532dd4d9b55f26a134af5e331ca222d3 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 21 Oct 2019 12:39:16 -0700 Subject: [PATCH 18/24] s/RECORDED/SAMPLED/ --- .../src/opentelemetry/trace/__init__.py | 10 ++-- .../src/opentelemetry/trace/sampling.py | 2 +- .../tests/trace/test_sampling.py | 52 +++++++++---------- .../sdk/context/propagation/b3_format.py | 4 +- 4 files changed, 34 insertions(+), 34 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index d4e40450d14..e9992165b57 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -251,8 +251,8 @@ def __exit__( class TraceOptions(int): """A bitmask that represents options specific to the trace. - The only supported option is the "recorded" flag (``0x01``). If set, this - flag indicates that the trace may have been recorded upstream. + The only supported option is the "sampled" flag (``0x01``). If set, this + flag indicates that the trace may have been sampled upstream. See the `W3C Trace Context - Traceparent`_ spec for details. @@ -261,15 +261,15 @@ class TraceOptions(int): """ DEFAULT = 0x00 - RECORDED = 0x01 + SAMPLED = 0x01 @classmethod def get_default(cls) -> "TraceOptions": return cls(cls.DEFAULT) @property - def recorded(self) -> bool: - return bool(self & TraceOptions.RECORDED) + def sampled(self) -> bool: + return bool(self & TraceOptions.SAMPLED) DEFAULT_TRACE_OPTIONS = TraceOptions.get_default() diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index 645cc2400d0..740232076ad 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/opentelemetry-api/src/opentelemetry/trace/sampling.py @@ -113,7 +113,7 @@ def should_sample( links: Optional[Sequence["Link"]] = None, ) -> "Decision": if parent_context is not None: - return Decision(parent_context.trace_options.recorded) + return Decision(parent_context.trace_options.sampled) return Decision(trace_id & self.CHECK_BYTES < self.bound, {}) diff --git a/opentelemetry-api/tests/trace/test_sampling.py b/opentelemetry-api/tests/trace/test_sampling.py index f3878c02ddb..b456aa91f18 100644 --- a/opentelemetry-api/tests/trace/test_sampling.py +++ b/opentelemetry-api/tests/trace/test_sampling.py @@ -18,7 +18,7 @@ from opentelemetry.trace import sampling TO_DEFAULT = trace.TraceOptions(trace.TraceOptions.DEFAULT) -TO_RECORDED = trace.TraceOptions(trace.TraceOptions.RECORDED) +TO_SAMPLED = trace.TraceOptions(trace.TraceOptions.SAMPLED) class TestSampler(unittest.TestCase): @@ -29,21 +29,21 @@ def test_always_on(self): ), 0xDEADBEF1, 0xDEADBEF2, - "unrecorded parent, sampling on", + "unsampled parent, sampling on", ) self.assertTrue(no_record_always_on.sampled) self.assertEqual(no_record_always_on.attributes, {}) - recorded_always_on = sampling.ALWAYS_ON.should_sample( + sampled_always_on = sampling.ALWAYS_ON.should_sample( trace.SpanContext( - 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_RECORDED + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_SAMPLED ), 0xDEADBEF1, 0xDEADBEF2, - "recorded parent, sampling on", + "sampled parent, sampling on", ) - self.assertTrue(recorded_always_on.sampled) - self.assertEqual(recorded_always_on.attributes, {}) + 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( @@ -52,21 +52,21 @@ def test_always_off(self): ), 0xDEADBEF1, 0xDEADBEF2, - "unrecorded parent, sampling off", + "unsampled parent, sampling off", ) self.assertFalse(no_record_always_off.sampled) self.assertEqual(no_record_always_off.attributes, {}) - recorded_always_on = sampling.ALWAYS_OFF.should_sample( + sampled_always_on = sampling.ALWAYS_OFF.should_sample( trace.SpanContext( - 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_RECORDED + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_SAMPLED ), 0xDEADBEF1, 0xDEADBEF2, - "recorded parent, sampling off", + "sampled parent, sampling off", ) - self.assertFalse(recorded_always_on.sampled) - self.assertEqual(recorded_always_on.attributes, {}) + self.assertFalse(sampled_always_on.sampled) + self.assertEqual(sampled_always_on.attributes, {}) def test_default_on(self): no_record_default_on = sampling.DEFAULT_ON.should_sample( @@ -75,21 +75,21 @@ def test_default_on(self): ), 0xDEADBEF1, 0xDEADBEF2, - "unrecorded parent, sampling on", + "unsampled parent, sampling on", ) self.assertFalse(no_record_default_on.sampled) self.assertEqual(no_record_default_on.attributes, {}) - recorded_default_on = sampling.DEFAULT_ON.should_sample( + sampled_default_on = sampling.DEFAULT_ON.should_sample( trace.SpanContext( - 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_RECORDED + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_SAMPLED ), 0xDEADBEF1, 0xDEADBEF2, - "recorded parent, sampling on", + "sampled parent, sampling on", ) - self.assertTrue(recorded_default_on.sampled) - self.assertEqual(recorded_default_on.attributes, {}) + 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( @@ -98,21 +98,21 @@ def test_default_off(self): ), 0xDEADBEF1, 0xDEADBEF2, - "unrecorded parent, sampling off", + "unsampled parent, sampling off", ) self.assertFalse(no_record_default_off.sampled) self.assertEqual(no_record_default_off.attributes, {}) - recorded_default_off = sampling.DEFAULT_OFF.should_sample( + sampled_default_off = sampling.DEFAULT_OFF.should_sample( trace.SpanContext( - 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_RECORDED + 0xDEADBEEF, 0xDEADBEF0, trace_options=TO_SAMPLED ), 0xDEADBEF1, 0xDEADBEF2, - "recorded parent, sampling off", + "sampled parent, sampling off", ) - self.assertTrue(recorded_default_off.sampled) - self.assertEqual(recorded_default_off.attributes, {}) + self.assertTrue(sampled_default_off.sampled) + self.assertEqual(sampled_default_off.attributes, {}) def test_probability_sampler(self): sampler = sampling.ProbabilitySampler(0.5) @@ -145,7 +145,7 @@ def test_probability_sampler(self): self.assertTrue( sampler.should_sample( trace.SpanContext( - 0xDEADBEF0, 0xDEADBEF1, trace_options=TO_RECORDED + 0xDEADBEF0, 0xDEADBEF1, trace_options=TO_SAMPLED ), 0x8000000000000001, 0xDEADBEEF, diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/context/propagation/b3_format.py b/opentelemetry-sdk/src/opentelemetry/sdk/context/propagation/b3_format.py index 2eca8afaa1b..7d59fddb9e5 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/context/propagation/b3_format.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/context/propagation/b3_format.py @@ -90,7 +90,7 @@ def extract(cls, get_from_carrier, carrier): # the desire for some form of sampling, propagate if either # header is set to allow. if sampled in cls._SAMPLE_PROPAGATE_VALUES or flags == "1": - options |= trace.TraceOptions.RECORDED + options |= trace.TraceOptions.SAMPLED return trace.SpanContext( # trace an span ids are encoded in hex, so must be converted trace_id=int(trace_id, 16), @@ -101,7 +101,7 @@ def extract(cls, get_from_carrier, carrier): @classmethod def inject(cls, context, set_in_carrier, carrier): - sampled = (trace.TraceOptions.RECORDED & context.trace_options) != 0 + sampled = (trace.TraceOptions.SAMPLED & context.trace_options) != 0 set_in_carrier( carrier, cls.TRACE_ID_KEY, format_trace_id(context.trace_id) ) From c3efeb09bad2b076c93d935103296b7dc4fdf077 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 21 Oct 2019 12:46:06 -0700 Subject: [PATCH 19/24] Address @Oberon00's comments --- opentelemetry-api/src/opentelemetry/trace/sampling.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index 740232076ad..a52c8885a27 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/opentelemetry-api/src/opentelemetry/trace/sampling.py @@ -56,7 +56,7 @@ def should_sample( trace_id: int, span_id: int, name: str, - links: Optional[Sequence["Link"]] = None, + links: Sequence["Link"] = (), ) -> "Decision": pass @@ -73,7 +73,7 @@ def should_sample( trace_id: int, span_id: int, name: str, - links: Optional[Sequence["Link"]] = None, + links: Sequence["Link"] = (), ) -> "Decision": return self._decision @@ -110,12 +110,12 @@ def should_sample( trace_id: int, span_id: int, name: str, - links: Optional[Sequence["Link"]] = None, + links: Sequence["Link"] = (), ) -> "Decision": 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.CHECK_BYTES < self.bound) # Samplers that ignore the parent sampling decision and never/always sample. From b24a465a374f5619ed377891d162012634190e70 Mon Sep 17 00:00:00 2001 From: Hector Hernandez <39923391+hectorhdzg@users.noreply.github.com> Date: Wed, 16 Oct 2019 16:19:46 -0700 Subject: [PATCH 20/24] Add set_status to Span (#213) --- docs/opentelemetry.trace.rst | 12 +- docs/opentelemetry.trace.status.rst | 7 + .../src/opentelemetry/trace/__init__.py | 6 + .../src/opentelemetry/trace/status.py | 185 ++++++++++++++++++ .../src/opentelemetry/sdk/trace/__init__.py | 9 + opentelemetry-sdk/tests/trace/test_trace.py | 31 +++ 6 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 docs/opentelemetry.trace.status.rst create mode 100644 opentelemetry-api/src/opentelemetry/trace/status.py diff --git a/docs/opentelemetry.trace.rst b/docs/opentelemetry.trace.rst index cec44bd8178..a57b5dcbff8 100644 --- a/docs/opentelemetry.trace.rst +++ b/docs/opentelemetry.trace.rst @@ -1,4 +1,14 @@ opentelemetry.trace package =========================== -.. automodule:: opentelemetry.trace +Submodules +---------- + +.. toctree:: + + opentelemetry.trace.status + +Module contents +--------------- + +.. automodule:: opentelemetry.trace \ No newline at end of file diff --git a/docs/opentelemetry.trace.status.rst b/docs/opentelemetry.trace.status.rst new file mode 100644 index 00000000000..0205446c808 --- /dev/null +++ b/docs/opentelemetry.trace.status.rst @@ -0,0 +1,7 @@ +opentelemetry.trace.status +========================== + +.. automodule:: opentelemetry.trace.status + :members: + :undoc-members: + :show-inheritance: diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index e9992165b57..3e868ec2f01 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -66,6 +66,7 @@ import typing from contextlib import contextmanager +from opentelemetry.trace.status import Status from opentelemetry.util import loader, types # TODO: quarantine @@ -227,6 +228,11 @@ def is_recording_events(self) -> bool: events with the add_event operation and attributes using set_attribute. """ + def set_status(self, status: Status) -> None: + """Sets the Status of the Span. If used, this will override the default + Span status, which is OK. + """ + def __enter__(self) -> "Span": """Invoked when `Span` is used as a context manager. diff --git a/opentelemetry-api/src/opentelemetry/trace/status.py b/opentelemetry-api/src/opentelemetry/trace/status.py new file mode 100644 index 00000000000..4fc50b33e56 --- /dev/null +++ b/opentelemetry-api/src/opentelemetry/trace/status.py @@ -0,0 +1,185 @@ +# Copyright 2019, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import enum +import typing + + +class StatusCanonicalCode(enum.Enum): + """Represents the canonical set of status codes of a finished Span.""" + + OK = 0 + """Not an error, returned on success.""" + + CANCELLED = 1 + """The operation was cancelled, typically by the caller.""" + + UNKNOWN = 2 + """Unknown error. + + For example, this error may be returned when a Status value received from + another address space belongs to an error space that is not known in this + address space. Also errors raised by APIs that do not return enough error + information may be converted to this error. + """ + + INVALID_ARGUMENT = 3 + """The client specified an invalid argument. + + Note that this differs from FAILED_PRECONDITION. INVALID_ARGUMENT indicates + arguments that are problematic regardless of the state of the system (e.g., + a malformed file name). + """ + + DEADLINE_EXCEEDED = 4 + """The deadline expired before the operation could complete. + + For operations that change the state of the system, this error may be + returned even if the operation has completed successfully. For example, a + successful response from a server could have been delayed long + """ + + NOT_FOUND = 5 + """Some requested entity (e.g., file or directory) was not found. + + Note to server developers: if a request is denied for an entire class of + users, such as gradual feature rollout or undocumented whitelist, NOT_FOUND + may be used. If a request is denied for some users within a class of users, + such as user-based access control, PERMISSION_DENIED must be used. + """ + + ALREADY_EXISTS = 6 + """The entity that a client attempted to create (e.g., file or directory) + already exists. + """ + + PERMISSION_DENIED = 7 + """The caller does not have permission to execute the specified operation. + + PERMISSION_DENIED must not be used for rejections caused by exhausting some + resource (use RESOURCE_EXHAUSTED instead for those errors). + PERMISSION_DENIED must not be used if the caller can not be identified (use + UNAUTHENTICATED instead for those errors). This error code does not imply + the request is valid or the requested entity exists or satisfies other + pre-conditions. + """ + + RESOURCE_EXHAUSTED = 8 + """Some resource has been exhausted, perhaps a per-user quota, or perhaps + the entire file system is out of space. + """ + + FAILED_PRECONDITION = 9 + """The operation was rejected because the system is not in a state required + for the operation's execution. + + For example, the directory to be deleted is non-empty, an rmdir operation + is applied to a non-directory, etc. Service implementors can use the + following guidelines to decide between FAILED_PRECONDITION, ABORTED, and + UNAVAILABLE: + + (a) Use UNAVAILABLE if the client can retry just the failing call. + (b) Use ABORTED if the client should retry at a higher level (e.g., + when a client-specified test-and-set fails, indicating the client + should restart a read-modify-write sequence). + (c) Use FAILED_PRECONDITION if the client should not retry until the + system state has been explicitly fixed. + + E.g., if an "rmdir" fails because the directory is non-empty, + FAILED_PRECONDITION should be returned since the client should not retry + unless the files are deleted from the directory. + """ + + ABORTED = 10 + """The operation was aborted, typically due to a concurrency issue such as a + sequencer check failure or transaction abort. + + See the guidelines above for deciding between FAILED_PRECONDITION, ABORTED, + and UNAVAILABLE. + """ + + OUT_OF_RANGE = 11 + """The operation was attempted past the valid range. + + E.g., seeking or reading past end-of-file. Unlike INVALID_ARGUMENT, this + error indicates a problem that may be fixed if the system state changes. + For example, a 32-bit file system will generate INVALID_ARGUMENT if asked + to read at an offset that is not in the range [0,2^32-1],but it will + generate OUT_OF_RANGE if asked to read from an offset past the current file + size. There is a fair bit of overlap between FAILED_PRECONDITION and + OUT_OF_RANGE. We recommend using OUT_OF_RANGE (the more specific error) + when it applies so that callers who are iterating through a space can + easily look for an OUT_OF_RANGE error to detect when they are done. + """ + + UNIMPLEMENTED = 12 + """The operation is not implemented or is not supported/enabled in this + service. + """ + + INTERNAL = 13 + """Internal errors. + + This means that some invariants expected by the underlying system have been + broken. This error code is reserved for serious errors. + """ + + UNAVAILABLE = 14 + """The service is currently unavailable. + + This is most likely a transient condition, which can be corrected by + retrying with a backoff. Note that it is not always safe to retry + non-idempotent operations. + """ + + DATA_LOSS = 15 + """Unrecoverable data loss or corruption.""" + + UNAUTHENTICATED = 16 + """The request does not have valid authentication credentials for the + operation. + """ + + +class Status: + """Represents the status of a finished Span. + + Args: + canonical_code: The canonical status code that describes the result + status of the operation. + description: An optional description of the status. + """ + + def __init__( + self, + canonical_code: "StatusCanonicalCode" = StatusCanonicalCode.OK, + description: typing.Optional[str] = None, + ): + self._canonical_code = canonical_code + self._description = description + + @property + def canonical_code(self) -> "StatusCanonicalCode": + """Represents the canonical status code of a finished Span.""" + return self._canonical_code + + @property + def description(self) -> typing.Optional[str]: + """Status description""" + return self._description + + @property + def is_ok(self) -> bool: + """Returns false if this represents an error, true otherwise.""" + return self._canonical_code is StatusCanonicalCode.OK diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 461203ede65..d33aa01146c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -144,6 +144,7 @@ def __init__( self.kind = kind self.span_processor = span_processor + self.status = trace_api.Status() self._lock = threading.Lock() if attributes is None: @@ -286,6 +287,14 @@ def update_name(self, name: str) -> None: def is_recording_events(self) -> bool: return True + def set_status(self, status: trace_api.Status) -> None: + with self._lock: + has_ended = self.end_time is not None + if has_ended: + logger.warning("Calling set_status() on an ended span.") + return + self.status = status + def generate_span_id() -> int: """Get a new random span ID. diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 32e227c7755..c167e374aca 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -312,6 +312,24 @@ def test_start_span(self): span.start() self.assertEqual(start_time, span.start_time) + # default status + self.assertTrue(span.status.is_ok) + self.assertIs( + span.status.canonical_code, trace_api.status.StatusCanonicalCode.OK + ) + self.assertIs(span.status.description, None) + + # status + new_status = trace_api.status.Status( + trace_api.status.StatusCanonicalCode.CANCELLED, "Test description" + ) + span.set_status(new_status) + self.assertIs( + span.status.canonical_code, + trace_api.status.StatusCanonicalCode.CANCELLED, + ) + self.assertIs(span.status.description, "Test description") + def test_span_override_start_and_end_time(self): """Span sending custom start_time and end_time values""" span = trace.Span("name", mock.Mock(spec=trace_api.SpanContext)) @@ -358,6 +376,19 @@ def test_ended_span(self): root.update_name("xxx") self.assertEqual(root.name, "root") + new_status = trace_api.status.Status( + trace_api.status.StatusCanonicalCode.CANCELLED, + "Test description", + ) + root.set_status(new_status) + # default status + self.assertTrue(root.status.is_ok) + self.assertEqual( + root.status.canonical_code, + trace_api.status.StatusCanonicalCode.OK, + ) + self.assertIs(root.status.description, None) + def span_event_start_fmt(span_processor_name, span_name): return span_processor_name + ":" + span_name + ":start" From 235d74f5b38edb6f5a88a096524ca65bcf0da0af Mon Sep 17 00:00:00 2001 From: Reiley Yang Date: Thu, 17 Oct 2019 08:09:31 -0700 Subject: [PATCH 21/24] Add contextmanager for Context (#215) --- .../src/opentelemetry/context/base_context.py | 10 ++++++ .../sdk/trace/export/__init__.py | 35 +++++++++---------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/context/base_context.py b/opentelemetry-api/src/opentelemetry/context/base_context.py index f1e37aa91f4..99d6869dd52 100644 --- a/opentelemetry-api/src/opentelemetry/context/base_context.py +++ b/opentelemetry-api/src/opentelemetry/context/base_context.py @@ -14,6 +14,7 @@ import threading import typing +from contextlib import contextmanager def wrap_callable(target: "object") -> typing.Callable[[], object]: @@ -99,6 +100,15 @@ def __getitem__(self, name: str) -> "object": def __setitem__(self, name: str, value: "object") -> None: self.__setattr__(name, value) + @contextmanager # type: ignore + def use(self, **kwargs: typing.Dict[str, object]) -> typing.Iterator[None]: + snapshot = {key: self[key] for key in kwargs} + for key in kwargs: + self[key] = kwargs[key] + yield + for key in kwargs: + self[key] = snapshot[key] + def with_current_context( self, func: typing.Callable[..., "object"] ) -> typing.Callable[..., "object"]: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py index a76a658b3a3..ecdc93b0adb 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/export/__init__.py @@ -73,15 +73,12 @@ def on_start(self, span: Span) -> None: pass def on_end(self, span: Span) -> None: - suppress_instrumentation = Context.suppress_instrumentation - try: - Context.suppress_instrumentation = True - self.span_exporter.export((span,)) - # pylint: disable=broad-except - except Exception as exc: - logger.warning("Exception while exporting data: %s", exc) - finally: - Context.suppress_instrumentation = suppress_instrumentation + with Context.use(suppress_instrumentation=True): + try: + self.span_exporter.export((span,)) + # pylint: disable=broad-except + except Exception as exc: + logger.warning("Exception while exporting data: %s", exc) def shutdown(self) -> None: self.span_exporter.shutdown() @@ -185,16 +182,16 @@ def export(self) -> None: while idx < self.max_export_batch_size and self.queue: self.spans_list[idx] = self.queue.pop() idx += 1 - suppress_instrumentation = Context.suppress_instrumentation - try: - Context.suppress_instrumentation = True - # Ignore type b/c the Optional[None]+slicing is too "clever" for mypy - self.span_exporter.export(self.spans_list[:idx]) # type: ignore - # pylint: disable=broad-except - except Exception: - logger.exception("Exception while exporting data.") - finally: - Context.suppress_instrumentation = suppress_instrumentation + with Context.use(suppress_instrumentation=True): + try: + # Ignore type b/c the Optional[None]+slicing is too "clever" + # for mypy + self.span_exporter.export( + self.spans_list[:idx] + ) # type: ignore + # pylint: disable=broad-except + except Exception: + logger.exception("Exception while exporting data.") # clean up list for index in range(idx): From 3594e32833f8660329ad324b692c108b6f047231 Mon Sep 17 00:00:00 2001 From: Johannes Liebermann Date: Mon, 21 Oct 2019 17:27:32 +0200 Subject: [PATCH 22/24] Fix mypy errors (#229) In particular, the following errors are fixed in this commit: * Don't return False in __exit__ Returning a literal causes a mypy error when combined with the `typing.Optional[bool]` type hint. Furthermore, exception handling is the same when returning `False` and when returning `None` (the exception is re-raised). Therefore, it's simpler to remove the return statement and change the type hint to `None`. * Correctly initialize nested tuple Tuples of length 1 should be initialized with a trailing comma to be properly interpreted. * Pass correct type to use_context() in test * Add type annotations for test helper functions Since we have `disallow_untyped_calls = True` in our mypy config for tests, we must add type annotations to any function that is called from a test. Addditionally, bump minimal mypy version to 0.740 to consistently reproduce these errors. --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 8 ++------ .../distributedcontext/test_distributed_context.py | 10 +++++++++- opentelemetry-api/tests/metrics/test_metrics.py | 2 +- opentelemetry-api/tests/test_loader.py | 5 +++-- tox.ini | 2 +- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 3e868ec2f01..d2426fd31d1 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -245,13 +245,9 @@ def __exit__( exc_type: typing.Optional[typing.Type[BaseException]], exc_val: typing.Optional[BaseException], exc_tb: typing.Optional[python_types.TracebackType], - ) -> typing.Optional[bool]: - """Ends context manager and calls `end` on the `Span`. - - Returns False. - """ + ) -> None: + """Ends context manager and calls `end` on the `Span`.""" self.end() - return False class TraceOptions(int): diff --git a/opentelemetry-api/tests/distributedcontext/test_distributed_context.py b/opentelemetry-api/tests/distributedcontext/test_distributed_context.py index 67a60048399..c730603b162 100644 --- a/opentelemetry-api/tests/distributedcontext/test_distributed_context.py +++ b/opentelemetry-api/tests/distributedcontext/test_distributed_context.py @@ -99,6 +99,14 @@ def test_get_current_context(self): self.assertIsNone(self.manager.get_current_context()) def test_use_context(self): - expected = object() + expected = distributedcontext.DistributedContext( + ( + distributedcontext.Entry( + distributedcontext.EntryMetadata(0), + distributedcontext.EntryKey("0"), + distributedcontext.EntryValue(""), + ), + ) + ) with self.manager.use_context(expected) as output: self.assertIs(output, expected) diff --git a/opentelemetry-api/tests/metrics/test_metrics.py b/opentelemetry-api/tests/metrics/test_metrics.py index 14667f62eaa..97ac92fcdef 100644 --- a/opentelemetry-api/tests/metrics/test_metrics.py +++ b/opentelemetry-api/tests/metrics/test_metrics.py @@ -24,7 +24,7 @@ def setUp(self): def test_record_batch(self): counter = metrics.Counter() - self.meter.record_batch(("values"), ((counter, 1))) + self.meter.record_batch(("values"), ((counter, 1),)) def test_create_metric(self): metric = self.meter.create_metric("", "", "", float, metrics.Counter) diff --git a/opentelemetry-api/tests/test_loader.py b/opentelemetry-api/tests/test_loader.py index 942479ab7dc..970b6159630 100644 --- a/opentelemetry-api/tests/test_loader.py +++ b/opentelemetry-api/tests/test_loader.py @@ -16,6 +16,7 @@ import sys import unittest from importlib import reload +from typing import Any, Callable from opentelemetry import trace from opentelemetry.util import loader @@ -59,7 +60,7 @@ def test_preferred_impl(self): # NOTE: We use do_* + *_ methods because subtest wouldn't run setUp, # which we require here. - def do_test_preferred_impl(self, setter): + def do_test_preferred_impl(self, setter: Callable[[Any], Any]) -> None: setter(get_opentelemetry_implementation) tracer = trace.tracer() self.assertIs(tracer, DUMMY_TRACER) @@ -81,7 +82,7 @@ def test_try_set_again(self): ) self.assertIn("already loaded", str(einfo.exception)) - def do_test_get_envvar(self, envvar_suffix): + def do_test_get_envvar(self, envvar_suffix: str) -> None: global DUMMY_TRACER # pylint:disable=global-statement # Test is not runnable with this! diff --git a/tox.ini b/tox.ini index 0db2364f197..19816d3d9c1 100644 --- a/tox.ini +++ b/tox.ini @@ -14,7 +14,7 @@ python = [testenv] deps = - mypy,mypyinstalled: mypy~=0.711 + mypy,mypyinstalled: mypy~=0.740 setenv = mypy: MYPYPATH={toxinidir}/opentelemetry-api/src/ From 1a4f96b70a487aee357aa00eb5934cec13e09b88 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 21 Oct 2019 14:23:59 -0700 Subject: [PATCH 23/24] Lint config fixes --- .flake8 | 16 ++++++++++++++-- .pylintrc | 3 ++- docs/conf.py | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/.flake8 b/.flake8 index a3411a16147..5abd0630ea0 100644 --- a/.flake8 +++ b/.flake8 @@ -1,3 +1,15 @@ [flake8] -ignore = E501,W503,E203 -exclude = .svn,CVS,.bzr,.hg,.git,__pycache__,.tox,ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/gen/,ext/opentelemetry-ext-jaeger/build/* +ignore = + E501 # line too long, defer to black + F401 # unused import, defer to pylint + W503 # allow line breaks after binary ops, not after +exclude = + .bzr + .git + .hg + .svn + .tox + CVS + __pycache__ + ext/opentelemetry-ext-jaeger/src/opentelemetry/ext/jaeger/gen/ + ext/opentelemetry-ext-jaeger/build/* diff --git a/.pylintrc b/.pylintrc index 782fc58700e..1aa1e10d0b4 100644 --- a/.pylintrc +++ b/.pylintrc @@ -68,7 +68,8 @@ disable=missing-docstring, ungrouped-imports, # Leave this up to isort wrong-import-order, # Leave this up to isort bad-continuation, # Leave this up to black - line-too-long # Leave this up to black + line-too-long, # Leave this up to black + exec-used # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option diff --git a/docs/conf.py b/docs/conf.py index 694ba7f0056..94d17cb3eb6 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -19,7 +19,7 @@ # -- Project information ----------------------------------------------------- project = "OpenTelemetry" -copyright = "2019, OpenTelemetry Authors" +copyright = "2019, OpenTelemetry Authors" # pylint: disable=redefined-builtin author = "OpenTelemetry Authors" From d6127b0667f375c9f322d6a8ad9c8c8182be2ff7 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 21 Oct 2019 14:40:44 -0700 Subject: [PATCH 24/24] Use pre-3.6 inline type annotations --- opentelemetry-api/src/opentelemetry/trace/sampling.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index a52c8885a27..f16e80495bf 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/opentelemetry-api/src/opentelemetry/trace/sampling.py @@ -38,12 +38,9 @@ def __init__( sampled: bool = False, attributes: Mapping[str, "AttributeValue"] = None, ) -> None: - self.sampled: bool - self.attributes: Dict[str, "AttributeValue"] - - self.sampled = sampled + self.sampled = sampled # type: bool if attributes is None: - self.attributes = {} + self.attributes = {} # type: Dict[str, "AttributeValue"] else: self.attributes = dict(attributes)