From 7d58c22472602058b1b0e846546c1bbbddff459f Mon Sep 17 00:00:00 2001 From: jerevoss Date: Tue, 11 Oct 2022 16:02:11 -0700 Subject: [PATCH 01/19] Enable custom sampler configuration via env vars --- CHANGELOG.md | 2 + .../sdk/_configuration/__init__.py | 22 +--- .../src/opentelemetry/sdk/trace/sampling.py | 72 +++++++++++-- .../src/opentelemetry/sdk/util/__init__.py | 29 ++++- opentelemetry-sdk/tests/test_configurator.py | 2 +- opentelemetry-sdk/tests/trace/test_trace.py | 101 ++++++++++++++++++ 6 files changed, 191 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d3b1d2fedb..98b413725d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.13.0...HEAD) +- Enabled custom samplers via entry points + ([#2972](https://github.com/open-telemetry/opentelemetry-python/pull/2972)) - Update explicit histogram bucket boundaries ([#2947](https://github.com/open-telemetry/opentelemetry-python/pull/2947)) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py index 159d471900a..1194894d837 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py @@ -23,7 +23,6 @@ from os import environ from typing import Dict, Optional, Sequence, Tuple, Type -from pkg_resources import iter_entry_points from typing_extensions import Literal from opentelemetry.environment_variables import ( @@ -55,6 +54,7 @@ from opentelemetry.sdk.trace import TracerProvider from opentelemetry.sdk.trace.export import BatchSpanProcessor, SpanExporter from opentelemetry.sdk.trace.id_generator import IdGenerator +from opentelemetry.sdk.util import _import_config_components from opentelemetry.semconv.resource import ResourceAttributes from opentelemetry.trace import set_tracer_provider @@ -228,26 +228,6 @@ def _init_logging( logging.getLogger().addHandler(handler) -def _import_config_components( - selected_components, entry_point_name -) -> Sequence[Tuple[str, object]]: - component_entry_points = { - ep.name: ep for ep in iter_entry_points(entry_point_name) - } - component_impls = [] - for selected_component in selected_components: - entry_point = component_entry_points.get(selected_component, None) - if not entry_point: - raise RuntimeError( - f"Requested component '{selected_component}' not found in entry points for '{entry_point_name}'" - ) - - component_impl = entry_point.load() - component_impls.append((selected_component, component_impl)) - - return component_impls - - def _import_exporters( trace_exporter_names: Sequence[str], metric_exporter_names: Sequence[str], diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 3cdd34cfe8c..dcb87e8b7db 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -64,7 +64,7 @@ ... The tracer sampler can also be configured via environment variables ``OTEL_TRACES_SAMPLER`` and ``OTEL_TRACES_SAMPLER_ARG`` (only if applicable). -The list of known values for ``OTEL_TRACES_SAMPLER`` are: +The list of built-in values for ``OTEL_TRACES_SAMPLER`` are: * always_on - Sampler that always samples spans, regardless of the parent span's sampling decision. * always_off - Sampler that never samples spans, regardless of the parent span's sampling decision. @@ -73,7 +73,24 @@ * parentbased_always_off - Sampler that respects its parent span's sampling decision, but otherwise never samples. * parentbased_traceidratio - Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on rate. -Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is traceidratio or parentbased_traceidratio, when not provided rate will be set to 1.0 (maximum rate possible). +In order to configure a custom sampler via environment variables, create an entry point for the custom sampler class under the entry point group, ``opentelemtry_traces_sampler``. Then, set the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. For example, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and ``OTEL_TRACES_SAMPLER_ARG=0.5`` after creating the following entry point: + +.. code:: python + + setup( + ... + entry_points={ + ... + "opentelemtry_traces_sampler": [ + "custom_sampler_name = path.to.sampler.module:CustomSampler" + ] + } + ) + ... + class CustomSampler(TraceIdRatioBased): + ... + +Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is a ``TraceIdRatioBased`` Sampler, such as ``traceidratio`` and ``parentbased_traceidratio``. When not provided rate will be set to 1.0 (maximum rate possible). Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. @@ -111,10 +128,13 @@ OTEL_TRACES_SAMPLER, OTEL_TRACES_SAMPLER_ARG, ) +from opentelemetry.sdk.util import _import_config_components from opentelemetry.trace import Link, SpanKind, get_current_span from opentelemetry.trace.span import TraceState from opentelemetry.util.types import Attributes +# from opentelemetry.sdk._configuration import _import_config_components + _logger = getLogger(__name__) @@ -161,6 +181,9 @@ def __init__( self.trace_state = trace_state +_OTEL_SAMPLER_ENTRY_POINT_GROUP = "opentelemtry_traces_sampler" + + class Sampler(abc.ABC): @abc.abstractmethod def should_sample( @@ -350,7 +373,7 @@ def get_description(self): """Sampler that respects its parent span's sampling decision, but otherwise always samples.""" -class ParentBasedTraceIdRatio(ParentBased): +class ParentBasedTraceIdRatio(ParentBased, TraceIdRatioBased): """ Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on `rate`. @@ -361,33 +384,48 @@ def __init__(self, rate: float): super().__init__(root=root) -_KNOWN_SAMPLERS = { +_KNOWN_INITIALIZED_SAMPLERS = { "always_on": ALWAYS_ON, "always_off": ALWAYS_OFF, "parentbased_always_on": DEFAULT_ON, "parentbased_always_off": DEFAULT_OFF, +} + +_KNOWN_SAMPLER_CLASSES = { "traceidratio": TraceIdRatioBased, "parentbased_traceidratio": ParentBasedTraceIdRatio, } def _get_from_env_or_default() -> Sampler: - trace_sampler = os.getenv( + trace_sampler_name = os.getenv( OTEL_TRACES_SAMPLER, "parentbased_always_on" ).lower() - if trace_sampler not in _KNOWN_SAMPLERS: - _logger.warning("Couldn't recognize sampler %s.", trace_sampler) - trace_sampler = "parentbased_always_on" - if trace_sampler in ("traceidratio", "parentbased_traceidratio"): + if trace_sampler_name in _KNOWN_INITIALIZED_SAMPLERS: + return _KNOWN_INITIALIZED_SAMPLERS[trace_sampler_name] + + trace_sampler_impl = None + if trace_sampler_name in _KNOWN_SAMPLER_CLASSES: + trace_sampler_impl = _KNOWN_SAMPLER_CLASSES[trace_sampler_name] + else: + try: + trace_sampler_impl = _import_sampler(trace_sampler_name) + except RuntimeError as err: + _logger.warning( + "Unable to recognize sampler %s: %s", trace_sampler_name, err + ) + return _KNOWN_INITIALIZED_SAMPLERS["parentbased_always_on"] + + if issubclass(trace_sampler_impl, TraceIdRatioBased): try: rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) except ValueError: _logger.warning("Could not convert TRACES_SAMPLER_ARG to float.") rate = 1.0 - return _KNOWN_SAMPLERS[trace_sampler](rate) + return trace_sampler_impl(rate) - return _KNOWN_SAMPLERS[trace_sampler] + return trace_sampler_impl() def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: @@ -395,3 +433,15 @@ def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: if parent_span_context is None or not parent_span_context.is_valid: return None return parent_span_context.trace_state + + +def _import_sampler(sampler_name: str) -> Sampler: + # pylint: disable=unbalanced-tuple-unpacking + [(sampler_name, sampler_impl)] = _import_config_components( + [sampler_name.strip()], _OTEL_SAMPLER_ENTRY_POINT_GROUP + ) + + if issubclass(sampler_impl, Sampler): + return sampler_impl + + raise RuntimeError(f"{sampler_name} is not an Sampler") diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py index e1857d8e62d..14fa28fd234 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py @@ -15,10 +15,11 @@ import datetime import threading from collections import OrderedDict, deque -from collections.abc import MutableMapping, Sequence -from typing import Optional +from collections import abc +from typing import Optional, Sequence, Tuple from deprecated import deprecated +from pkg_resources import iter_entry_points def ns_to_iso_str(nanoseconds): @@ -41,7 +42,27 @@ def get_dict_as_key(labels): ) -class BoundedList(Sequence): +def _import_config_components( + selected_components, entry_point_name +) -> Sequence[Tuple[str, object]]: + component_entry_points = { + ep.name: ep for ep in iter_entry_points(entry_point_name) + } + component_impls = [] + for selected_component in selected_components: + entry_point = component_entry_points.get(selected_component, None) + if not entry_point: + raise RuntimeError( + f"Requested component '{selected_component}' not found in entry points for '{entry_point_name}'" + ) + + component_impl = entry_point.load() + component_impls.append((selected_component, component_impl)) + + return component_impls + + +class BoundedList(abc.Sequence): """An append only list with a fixed max size. Calls to `append` and `extend` will drop the oldest elements if there is @@ -92,7 +113,7 @@ def from_seq(cls, maxlen, seq): @deprecated(version="1.4.0") # type: ignore -class BoundedDict(MutableMapping): +class BoundedDict(abc.MutableMapping): """An ordered dict with a fixed max capacity. Oldest elements are dropped when the dict is full and a new element is diff --git a/opentelemetry-sdk/tests/test_configurator.py b/opentelemetry-sdk/tests/test_configurator.py index 4aae8aa53be..e64607e8acd 100644 --- a/opentelemetry-sdk/tests/test_configurator.py +++ b/opentelemetry-sdk/tests/test_configurator.py @@ -257,7 +257,7 @@ def test_trace_init_otlp(self): @patch.dict(environ, {OTEL_PYTHON_ID_GENERATOR: "custom_id_generator"}) @patch("opentelemetry.sdk._configuration.IdGenerator", new=IdGenerator) - @patch("opentelemetry.sdk._configuration.iter_entry_points") + @patch("opentelemetry.sdk.util.iter_entry_points") def test_trace_init_custom_id_generator(self, mock_iter_entry_points): mock_iter_entry_points.configure_mock( return_value=[ diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 8b8d33faa45..cb2cc25692f 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -139,6 +139,63 @@ def test_tracer_provider_accepts_concurrent_multi_span_processor(self): ) +class CustomSampler(sampling.Sampler): + def __init__(self) -> None: + super().__init__() + + def get_description(self) -> str: + return super().get_description() + + def should_sample( + self, + parent_context, + trace_id, + name, + kind, + attributes, + links, + trace_state, + ): + return super().should_sample( + parent_context, + trace_id, + name, + kind, + attributes, + links, + trace_state, + ) + + +class CustomRatioSampler(sampling.TraceIdRatioBased): + def __init__(self, ratio): + self.ratio = ratio + super().__init__(ratio) + + def get_description(self) -> str: + return super().get_description() + + def should_sample( + self, + parent_context, + trace_id, + name, + kind, + attributes, + links, + trace_state, + ): + return super().should_sample( + parent_context, + trace_id, + name, + kind, + attributes, + links, + trace_state, + ) + + class TestTracerSampling(unittest.TestCase): def tearDown(self): reload(trace) @@ -219,6 +276,50 @@ def test_ratio_sampler_with_env(self): self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) self.assertEqual(tracer_provider.sampler._root.rate, 0.25) + @mock.patch.dict( + "os.environ", {OTEL_TRACES_SAMPLER: "non_existent_entry_point"} + ) + def test_sampler_with_env_non_existent_entry_point(self): + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) + # pylint: disable=protected-access + self.assertEqual(tracer_provider.sampler._root, sampling.ALWAYS_ON) + + @mock.patch("opentelemetry.sdk.trace.sampling._import_config_components") + @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler"}) + def test_custom_sampler_with_env( + self, mock_sampling_import_config_components + ): + mock_sampling_import_config_components.return_value = [ + ("custom_sampler", CustomSampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.assertIsInstance(tracer_provider.sampler, CustomSampler) + + @mock.patch("opentelemetry.sdk.trace.sampling._import_config_components") + @mock.patch.dict( + "os.environ", + { + OTEL_TRACES_SAMPLER: "custom_ratio_sampler", + OTEL_TRACES_SAMPLER_ARG: "0.5", + }, + ) + def test_custom_ratio_sampler_with_env( + self, mock_sampling_import_config_components + ): + mock_sampling_import_config_components.return_value = [ + ("custom_ratio_sampler", CustomRatioSampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.assertIsInstance(tracer_provider.sampler, CustomRatioSampler) + self.assertEqual(tracer_provider.sampler.ratio, 0.5) + class TestSpanCreation(unittest.TestCase): def test_start_span_invalid_spancontext(self): From b51464ef49e0ddd0e4b22ccb7e18bc7addc80273 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Thu, 13 Oct 2022 10:24:47 -0700 Subject: [PATCH 02/19] lint --- opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py | 2 +- opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index dcb87e8b7db..b0023a65bc9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -133,7 +133,6 @@ class CustomSampler(TraceIdRatioBased): from opentelemetry.trace.span import TraceState from opentelemetry.util.types import Attributes -# from opentelemetry.sdk._configuration import _import_config_components _logger = getLogger(__name__) @@ -380,6 +379,7 @@ class ParentBasedTraceIdRatio(ParentBased, TraceIdRatioBased): """ def __init__(self, rate: float): + # TODO: If TraceIdRatioBased inheritance is kept, change this root = TraceIdRatioBased(rate=rate) super().__init__(root=root) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py index 14fa28fd234..6d7f9d9bb59 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py @@ -14,8 +14,7 @@ import datetime import threading -from collections import OrderedDict, deque -from collections import abc +from collections import OrderedDict, abc, deque from typing import Optional, Sequence, Tuple from deprecated import deprecated From 3c4a6263068af24d9ce463d2a5361418403b1e3e Mon Sep 17 00:00:00 2001 From: jerevoss Date: Thu, 13 Oct 2022 16:08:28 -0700 Subject: [PATCH 03/19] Using factory method approach instread --- .../src/opentelemetry/sdk/trace/sampling.py | 94 ++++++----- opentelemetry-sdk/tests/trace/test_trace.py | 146 ++++++++++++++++-- 2 files changed, 188 insertions(+), 52 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index b0023a65bc9..c644346cb61 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -73,7 +73,10 @@ * parentbased_always_off - Sampler that respects its parent span's sampling decision, but otherwise never samples. * parentbased_traceidratio - Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on rate. -In order to configure a custom sampler via environment variables, create an entry point for the custom sampler class under the entry point group, ``opentelemtry_traces_sampler``. Then, set the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. For example, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and ``OTEL_TRACES_SAMPLER_ARG=0.5`` after creating the following entry point: +In order to configure a custom sampler via environment variables, create an entry point for the custom sampler factory method under the entry point group, ``opentelemtry_traces_sampler``. Then, set +the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. The custom sampler factory method must take a single string argument. This input will come from +``OTEL_TRACES_SAMPLER_ARG``. If ``OTEL_TRACES_SAMPLER_ARG`` is not configured, the input will be an empty string. For example, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and +``OTEL_TRACES_SAMPLER_ARG=0.5`` after creating the following entry point: .. code:: python @@ -82,15 +85,22 @@ entry_points={ ... "opentelemtry_traces_sampler": [ - "custom_sampler_name = path.to.sampler.module:CustomSampler" + "custom_sampler_name = path.to.sampler.factory.method:CustomSamplerFactory.get_sampler" ] } ) ... - class CustomSampler(TraceIdRatioBased): - ... + class CustomRatioSampler(Sampler): + def __init__(rate): + ... + ... + class CustomSamplerFactory: + get_sampler(sampler_argument_str): + rate = float(sampler_argument_str) + return CustomSampler(rate) -Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is a ``TraceIdRatioBased`` Sampler, such as ``traceidratio`` and ``parentbased_traceidratio``. When not provided rate will be set to 1.0 (maximum rate possible). +Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is a ``TraceIdRatioBased`` Sampler, such as ``traceidratio`` and ``parentbased_traceidratio``. When not provided rate +will be set to 1.0 (maximum rate possible). Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. @@ -372,60 +382,76 @@ def get_description(self): """Sampler that respects its parent span's sampling decision, but otherwise always samples.""" -class ParentBasedTraceIdRatio(ParentBased, TraceIdRatioBased): +class ParentBasedTraceIdRatio(ParentBased): """ Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on `rate`. """ def __init__(self, rate: float): - # TODO: If TraceIdRatioBased inheritance is kept, change this root = TraceIdRatioBased(rate=rate) super().__init__(root=root) -_KNOWN_INITIALIZED_SAMPLERS = { +_KNOWN_SAMPLERS = { "always_on": ALWAYS_ON, "always_off": ALWAYS_OFF, "parentbased_always_on": DEFAULT_ON, "parentbased_always_off": DEFAULT_OFF, -} - -_KNOWN_SAMPLER_CLASSES = { "traceidratio": TraceIdRatioBased, "parentbased_traceidratio": ParentBasedTraceIdRatio, } def _get_from_env_or_default() -> Sampler: - trace_sampler_name = os.getenv( + trace_sampler = os.getenv( OTEL_TRACES_SAMPLER, "parentbased_always_on" ).lower() + if trace_sampler not in _KNOWN_SAMPLERS: + _logger.warning("Couldn't recognize sampler %s.", trace_sampler) + trace_sampler = "parentbased_always_on" - if trace_sampler_name in _KNOWN_INITIALIZED_SAMPLERS: - return _KNOWN_INITIALIZED_SAMPLERS[trace_sampler_name] - - trace_sampler_impl = None - if trace_sampler_name in _KNOWN_SAMPLER_CLASSES: - trace_sampler_impl = _KNOWN_SAMPLER_CLASSES[trace_sampler_name] - else: - try: - trace_sampler_impl = _import_sampler(trace_sampler_name) - except RuntimeError as err: - _logger.warning( - "Unable to recognize sampler %s: %s", trace_sampler_name, err - ) - return _KNOWN_INITIALIZED_SAMPLERS["parentbased_always_on"] - - if issubclass(trace_sampler_impl, TraceIdRatioBased): + if trace_sampler in ("traceidratio", "parentbased_traceidratio"): try: rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) except ValueError: _logger.warning("Could not convert TRACES_SAMPLER_ARG to float.") rate = 1.0 - return trace_sampler_impl(rate) + return _KNOWN_SAMPLERS[trace_sampler](rate) - return trace_sampler_impl() + return _KNOWN_SAMPLERS[trace_sampler] + + +def _get_from_env_or_default() -> Sampler: + trace_sampler_name = os.getenv( + OTEL_TRACES_SAMPLER, "parentbased_always_on" + ).lower() + + if trace_sampler_name in _KNOWN_SAMPLERS: + if trace_sampler_name in ("traceidratio", "parentbased_traceidratio"): + try: + rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) + except ValueError: + _logger.warning("Could not convert TRACES_SAMPLER_ARG to float.") + rate = 1.0 + return _KNOWN_SAMPLERS[trace_sampler_name](rate) + return _KNOWN_SAMPLERS[trace_sampler_name] + else: + try: + trace_sampler_factory = _import_sampler_factory(trace_sampler_name) + sampler_arg = os.getenv(OTEL_TRACES_SAMPLER_ARG, "") + trace_sampler = trace_sampler_factory(sampler_arg) + _logger.warning("JEREVOSS: trace_sampler: %s" % trace_sampler) + if not issubclass(type(trace_sampler), Sampler): + message = "Output of traces sampler factory, %s, was not a Sampler object." % trace_sampler_factory + _logger.warning(message) + raise ValueError(message) + return trace_sampler + except Exception as exc: + _logger.warning( + "Failed to initialize custom sampler, %s: %s", trace_sampler_name, exc + ) + return _KNOWN_SAMPLERS["parentbased_always_on"] def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: @@ -435,13 +461,9 @@ def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: return parent_span_context.trace_state -def _import_sampler(sampler_name: str) -> Sampler: +def _import_sampler_factory(sampler_name: str) -> Sampler: # pylint: disable=unbalanced-tuple-unpacking [(sampler_name, sampler_impl)] = _import_config_components( [sampler_name.strip()], _OTEL_SAMPLER_ENTRY_POINT_GROUP ) - - if issubclass(sampler_impl, Sampler): - return sampler_impl - - raise RuntimeError(f"{sampler_name} is not an Sampler") + return sampler_impl diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index cb2cc25692f..623a648c37c 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -196,6 +196,26 @@ def should_sample( ) +class CustomSamplerFactory: + def get_custom_sampler(unused_sampler_arg): + return CustomSampler() + + def get_custom_ratio_sampler(sampler_arg): + return CustomRatioSampler(float(sampler_arg)) + + def empty_get_custom_sampler(sampler_arg): + return + + +class IterEntryPoint: + def __init__(self, name, class_type): + self.name = name + self.class_type = class_type + + def load(self): + return self.class_type + + class TestTracerSampling(unittest.TestCase): def tearDown(self): reload(trace) @@ -222,9 +242,7 @@ def test_default_sampler(self): def test_default_sampler_type(self): tracer_provider = trace.TracerProvider() - self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) - # pylint: disable=protected-access - self.assertEqual(tracer_provider.sampler._root, sampling.ALWAYS_ON) + self.verify_default_sampler(tracer_provider) def test_sampler_no_sampling(self): tracer_provider = trace.TracerProvider(sampling.ALWAYS_OFF) @@ -283,36 +301,69 @@ def test_sampler_with_env_non_existent_entry_point(self): # pylint: disable=protected-access reload(trace) tracer_provider = trace.TracerProvider() - self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) - # pylint: disable=protected-access - self.assertEqual(tracer_provider.sampler._root, sampling.ALWAYS_ON) + self.verify_default_sampler(tracer_provider) - @mock.patch("opentelemetry.sdk.trace.sampling._import_config_components") - @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler"}) + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"}) def test_custom_sampler_with_env( - self, mock_sampling_import_config_components + self, mock_iter_entry_points + ): + # mock_iter_entry_points.return_value = [ + # ("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) + # ] + mock_iter_entry_points.return_value=[ + IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.assertIsInstance(tracer_provider.sampler, CustomSampler) + + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"}) + def test_custom_sampler_with_env_bad_factory( + self, mock_iter_entry_points + ): + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.empty_get_custom_sampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.verify_default_sampler(tracer_provider) + + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict( + "os.environ", + { + OTEL_TRACES_SAMPLER: "custom_sampler_factory", + OTEL_TRACES_SAMPLER_ARG: "0.5", + }, + ) + def test_custom_sampler_with_env_unused_arg( + self, mock_iter_entry_points ): - mock_sampling_import_config_components.return_value = [ - ("custom_sampler", CustomSampler) + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) ] # pylint: disable=protected-access reload(trace) tracer_provider = trace.TracerProvider() self.assertIsInstance(tracer_provider.sampler, CustomSampler) - @mock.patch("opentelemetry.sdk.trace.sampling._import_config_components") + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") @mock.patch.dict( "os.environ", { - OTEL_TRACES_SAMPLER: "custom_ratio_sampler", + OTEL_TRACES_SAMPLER: "custom_ratio_sampler_factory", OTEL_TRACES_SAMPLER_ARG: "0.5", }, ) def test_custom_ratio_sampler_with_env( - self, mock_sampling_import_config_components + self, mock_iter_entry_points ): - mock_sampling_import_config_components.return_value = [ - ("custom_ratio_sampler", CustomRatioSampler) + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) ] # pylint: disable=protected-access reload(trace) @@ -320,6 +371,69 @@ def test_custom_ratio_sampler_with_env( self.assertIsInstance(tracer_provider.sampler, CustomRatioSampler) self.assertEqual(tracer_provider.sampler.ratio, 0.5) + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict( + "os.environ", + { + OTEL_TRACES_SAMPLER: "custom_ratio_sampler_factory", + OTEL_TRACES_SAMPLER_ARG: "foobar", + }, + ) + def test_custom_ratio_sampler_with_env_bad_arg( + self, mock_iter_entry_points + ): + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.verify_default_sampler(tracer_provider) + + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict( + "os.environ", + { + OTEL_TRACES_SAMPLER: "custom_ratio_sampler_factory", + }, + ) + def test_custom_ratio_sampler_with_env_no_arg( + self, mock_iter_entry_points + ): + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.verify_default_sampler(tracer_provider) + + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict( + "os.environ", + { + OTEL_TRACES_SAMPLER: "custom_sampler_factory", + OTEL_TRACES_SAMPLER_ARG: "0.5", + }, + ) + def test_custom_ratio_sampler_with_env_multiple_entry_points( + self, mock_iter_entry_points + ): + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler), + IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler), + IterEntryPoint("custom_z_sampler_factory", CustomSamplerFactory.empty_get_custom_sampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.assertIsInstance(tracer_provider.sampler, CustomSampler) + + def verify_default_sampler(self, tracer_provider): + self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) + # pylint: disable=protected-access + self.assertEqual(tracer_provider.sampler._root, sampling.ALWAYS_ON) + class TestSpanCreation(unittest.TestCase): def test_start_span_invalid_spancontext(self): From 6d9f9428dd46d5ff35636c1243ec325c6540126c Mon Sep 17 00:00:00 2001 From: jerevoss Date: Thu, 13 Oct 2022 16:22:30 -0700 Subject: [PATCH 04/19] lint --- .../src/opentelemetry/sdk/trace/sampling.py | 14 ++-- opentelemetry-sdk/tests/trace/test_trace.py | 71 ++++++++++++------- 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index c644346cb61..5b9218a668a 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -143,7 +143,6 @@ class CustomSamplerFactory: from opentelemetry.trace.span import TraceState from opentelemetry.util.types import Attributes - _logger = getLogger(__name__) @@ -432,7 +431,9 @@ def _get_from_env_or_default() -> Sampler: try: rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) except ValueError: - _logger.warning("Could not convert TRACES_SAMPLER_ARG to float.") + _logger.warning( + "Could not convert TRACES_SAMPLER_ARG to float." + ) rate = 1.0 return _KNOWN_SAMPLERS[trace_sampler_name](rate) return _KNOWN_SAMPLERS[trace_sampler_name] @@ -443,13 +444,18 @@ def _get_from_env_or_default() -> Sampler: trace_sampler = trace_sampler_factory(sampler_arg) _logger.warning("JEREVOSS: trace_sampler: %s" % trace_sampler) if not issubclass(type(trace_sampler), Sampler): - message = "Output of traces sampler factory, %s, was not a Sampler object." % trace_sampler_factory + message = ( + "Output of traces sampler factory, %s, was not a Sampler object." + % trace_sampler_factory + ) _logger.warning(message) raise ValueError(message) return trace_sampler except Exception as exc: _logger.warning( - "Failed to initialize custom sampler, %s: %s", trace_sampler_name, exc + "Failed to initialize custom sampler, %s: %s", + trace_sampler_name, + exc, ) return _KNOWN_SAMPLERS["parentbased_always_on"] diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 623a648c37c..c1d731b947a 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -304,15 +304,18 @@ def test_sampler_with_env_non_existent_entry_point(self): self.verify_default_sampler(tracer_provider) @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") - @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"}) - def test_custom_sampler_with_env( - self, mock_iter_entry_points - ): + @mock.patch.dict( + "os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"} + ) + def test_custom_sampler_with_env(self, mock_iter_entry_points): # mock_iter_entry_points.return_value = [ # ("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) # ] - mock_iter_entry_points.return_value=[ - IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) + mock_iter_entry_points.return_value = [ + IterEntryPoint( + "custom_sampler_factory", + CustomSamplerFactory.get_custom_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -320,12 +323,15 @@ def test_custom_sampler_with_env( self.assertIsInstance(tracer_provider.sampler, CustomSampler) @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") - @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"}) - def test_custom_sampler_with_env_bad_factory( - self, mock_iter_entry_points - ): + @mock.patch.dict( + "os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"} + ) + def test_custom_sampler_with_env_bad_factory(self, mock_iter_entry_points): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.empty_get_custom_sampler) + IterEntryPoint( + "custom_sampler_factory", + CustomSamplerFactory.empty_get_custom_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -340,11 +346,12 @@ def test_custom_sampler_with_env_bad_factory( OTEL_TRACES_SAMPLER_ARG: "0.5", }, ) - def test_custom_sampler_with_env_unused_arg( - self, mock_iter_entry_points - ): + def test_custom_sampler_with_env_unused_arg(self, mock_iter_entry_points): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) + IterEntryPoint( + "custom_sampler_factory", + CustomSamplerFactory.get_custom_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -359,11 +366,12 @@ def test_custom_sampler_with_env_unused_arg( OTEL_TRACES_SAMPLER_ARG: "0.5", }, ) - def test_custom_ratio_sampler_with_env( - self, mock_iter_entry_points - ): + def test_custom_ratio_sampler_with_env(self, mock_iter_entry_points): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) + IterEntryPoint( + "custom_ratio_sampler_factory", + CustomSamplerFactory.get_custom_ratio_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -383,7 +391,10 @@ def test_custom_ratio_sampler_with_env_bad_arg( self, mock_iter_entry_points ): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) + IterEntryPoint( + "custom_ratio_sampler_factory", + CustomSamplerFactory.get_custom_ratio_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -401,7 +412,10 @@ def test_custom_ratio_sampler_with_env_no_arg( self, mock_iter_entry_points ): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) + IterEntryPoint( + "custom_ratio_sampler_factory", + CustomSamplerFactory.get_custom_ratio_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -420,9 +434,18 @@ def test_custom_ratio_sampler_with_env_multiple_entry_points( self, mock_iter_entry_points ): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler), - IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler), - IterEntryPoint("custom_z_sampler_factory", CustomSamplerFactory.empty_get_custom_sampler) + IterEntryPoint( + "custom_ratio_sampler_factory", + CustomSamplerFactory.get_custom_ratio_sampler, + ), + IterEntryPoint( + "custom_sampler_factory", + CustomSamplerFactory.get_custom_sampler, + ), + IterEntryPoint( + "custom_z_sampler_factory", + CustomSamplerFactory.empty_get_custom_sampler, + ), ] # pylint: disable=protected-access reload(trace) From 49c3400a938fad0152969b3caae72b9c34bb22de Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 17 Oct 2022 15:06:44 -0700 Subject: [PATCH 05/19] Resolving comments --- .../sdk/_configuration/__init__.py | 5 +- .../src/opentelemetry/sdk/trace/sampling.py | 49 ++++++------------- .../src/opentelemetry/sdk/util/__init__.py | 4 +- opentelemetry-sdk/tests/trace/test_trace.py | 6 +-- 4 files changed, 23 insertions(+), 41 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py index 1194894d837..910ee4b4116 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py @@ -269,10 +269,9 @@ def _import_exporters( def _import_id_generator(id_generator_name: str) -> IdGenerator: - # pylint: disable=unbalanced-tuple-unpacking - [(id_generator_name, id_generator_impl)] = _import_config_components( + id_generator_name, id_generator_impl = _import_config_components( [id_generator_name.strip()], "opentelemetry_id_generator" - ) + )[0] if issubclass(id_generator_impl, IdGenerator): return id_generator_impl diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 5b9218a668a..3b0c30a65c8 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -73,10 +73,10 @@ * parentbased_always_off - Sampler that respects its parent span's sampling decision, but otherwise never samples. * parentbased_traceidratio - Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on rate. -In order to configure a custom sampler via environment variables, create an entry point for the custom sampler factory method under the entry point group, ``opentelemtry_traces_sampler``. Then, set -the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. The custom sampler factory method must take a single string argument. This input will come from -``OTEL_TRACES_SAMPLER_ARG``. If ``OTEL_TRACES_SAMPLER_ARG`` is not configured, the input will be an empty string. For example, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and -``OTEL_TRACES_SAMPLER_ARG=0.5`` after creating the following entry point: +Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is traceidratio or parentbased_traceidratio, when not provided rate will be set to 1.0 (maximum rate possible). + +In order to create a configurable custom sampler, create an entry point for the custom sampler factory method under the entry point group, ``opentelemtry_traces_sampler``. The custom sampler factory method must be of type ``Callable[[str], Sampler]``, taking a single string argument and returning a Sampler object. The single input will come from the string value of +the ``OTEL_TRACES_SAMPLER_ARG`` environment variable. If ``OTEL_TRACES_SAMPLER_ARG`` is not configured, the input will be an empty string. For example: .. code:: python @@ -95,12 +95,15 @@ def __init__(rate): ... ... class CustomSamplerFactory: + @staticmethod get_sampler(sampler_argument_str): - rate = float(sampler_argument_str) - return CustomSampler(rate) + try: + rate = float(sampler_argument_str) + return CustomSampler(rate) + except ValueError: # In case argument is empty string. + return CustomSampler(0.5) -Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is a ``TraceIdRatioBased`` Sampler, such as ``traceidratio`` and ``parentbased_traceidratio``. When not provided rate -will be set to 1.0 (maximum rate possible). +In order to configure you application with a custom sampler's entry point, set the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. For example, to configured the above sampler, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and ``OTEL_TRACES_SAMPLER_ARG=0.5``. Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. @@ -130,7 +133,7 @@ class CustomSamplerFactory: import os from logging import getLogger from types import MappingProxyType -from typing import Optional, Sequence +from typing import Callable, Optional, Sequence # pylint: disable=unused-import from opentelemetry.context import Context @@ -189,7 +192,7 @@ def __init__( self.trace_state = trace_state -_OTEL_SAMPLER_ENTRY_POINT_GROUP = "opentelemtry_traces_sampler" +_OTEL_SAMPLER_ENTRY_POINT_GROUP = "opentelemetry_traces_sampler" class Sampler(abc.ABC): @@ -402,25 +405,6 @@ def __init__(self, rate: float): } -def _get_from_env_or_default() -> Sampler: - trace_sampler = os.getenv( - OTEL_TRACES_SAMPLER, "parentbased_always_on" - ).lower() - if trace_sampler not in _KNOWN_SAMPLERS: - _logger.warning("Couldn't recognize sampler %s.", trace_sampler) - trace_sampler = "parentbased_always_on" - - if trace_sampler in ("traceidratio", "parentbased_traceidratio"): - try: - rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) - except ValueError: - _logger.warning("Could not convert TRACES_SAMPLER_ARG to float.") - rate = 1.0 - return _KNOWN_SAMPLERS[trace_sampler](rate) - - return _KNOWN_SAMPLERS[trace_sampler] - - def _get_from_env_or_default() -> Sampler: trace_sampler_name = os.getenv( OTEL_TRACES_SAMPLER, "parentbased_always_on" @@ -467,9 +451,8 @@ def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: return parent_span_context.trace_state -def _import_sampler_factory(sampler_name: str) -> Sampler: - # pylint: disable=unbalanced-tuple-unpacking - [(sampler_name, sampler_impl)] = _import_config_components( +def _import_sampler_factory(sampler_name: str) -> Callable[[str], Sampler]: + _, sampler_impl = _import_config_components( [sampler_name.strip()], _OTEL_SAMPLER_ENTRY_POINT_GROUP - ) + )[0] return sampler_impl diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py index 6d7f9d9bb59..52104243532 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py @@ -15,7 +15,7 @@ import datetime import threading from collections import OrderedDict, abc, deque -from typing import Optional, Sequence, Tuple +from typing import List, Optional, Sequence, Tuple from deprecated import deprecated from pkg_resources import iter_entry_points @@ -42,7 +42,7 @@ def get_dict_as_key(labels): def _import_config_components( - selected_components, entry_point_name + selected_components: List[str], entry_point_name: str ) -> Sequence[Tuple[str, object]]: component_entry_points = { ep.name: ep for ep in iter_entry_points(entry_point_name) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index c1d731b947a..13940ab8773 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -197,12 +197,15 @@ def should_sample( class CustomSamplerFactory: + @staticmethod def get_custom_sampler(unused_sampler_arg): return CustomSampler() + @staticmethod def get_custom_ratio_sampler(sampler_arg): return CustomRatioSampler(float(sampler_arg)) + @staticmethod def empty_get_custom_sampler(sampler_arg): return @@ -308,9 +311,6 @@ def test_sampler_with_env_non_existent_entry_point(self): "os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"} ) def test_custom_sampler_with_env(self, mock_iter_entry_points): - # mock_iter_entry_points.return_value = [ - # ("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) - # ] mock_iter_entry_points.return_value = [ IterEntryPoint( "custom_sampler_factory", From 873f402607c71583c2814745160acf37c4a35d80 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 17 Oct 2022 15:28:30 -0700 Subject: [PATCH 06/19] resolving comments --- .../src/opentelemetry/sdk/trace/sampling.py | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 3b0c30a65c8..28792aa2456 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -75,8 +75,31 @@ Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is traceidratio or parentbased_traceidratio, when not provided rate will be set to 1.0 (maximum rate possible). -In order to create a configurable custom sampler, create an entry point for the custom sampler factory method under the entry point group, ``opentelemtry_traces_sampler``. The custom sampler factory method must be of type ``Callable[[str], Sampler]``, taking a single string argument and returning a Sampler object. The single input will come from the string value of -the ``OTEL_TRACES_SAMPLER_ARG`` environment variable. If ``OTEL_TRACES_SAMPLER_ARG`` is not configured, the input will be an empty string. For example: +Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. + +.. code:: python + + from opentelemetry import trace + from opentelemetry.sdk.trace import TracerProvider + from opentelemetry.sdk.trace.export import ( + ConsoleSpanExporter, + SimpleSpanProcessor, + ) + + trace.set_tracer_provider(TracerProvider()) + + # set up an exporter for sampled spans + trace.get_tracer_provider().add_span_processor( + SimpleSpanProcessor(ConsoleSpanExporter()) + ) + + # created spans will now be sampled by the TraceIdRatioBased sampler with rate 1/1000. + with trace.get_tracer(__name__).start_as_current_span("Test Span"): + ... + +In order to create a configurable custom sampler, create an entry point for the custom sampler factory method under the entry point group, ``opentelemetry_traces_sampler``. The custom sampler factory +method must be of type ``Callable[[str], Sampler]``, taking a single string argument and returning a Sampler object. The single input will come from the string value of the +``OTEL_TRACES_SAMPLER_ARG`` environment variable. If ``OTEL_TRACES_SAMPLER_ARG`` is not configured, the input will be an empty string. For example: .. code:: python @@ -84,7 +107,7 @@ ... entry_points={ ... - "opentelemtry_traces_sampler": [ + "opentelemetry_traces_sampler": [ "custom_sampler_name = path.to.sampler.factory.method:CustomSamplerFactory.get_sampler" ] } @@ -103,30 +126,8 @@ class CustomSamplerFactory: except ValueError: # In case argument is empty string. return CustomSampler(0.5) -In order to configure you application with a custom sampler's entry point, set the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. For example, to configured the above sampler, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and ``OTEL_TRACES_SAMPLER_ARG=0.5``. - - -Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. - -.. code:: python - - from opentelemetry import trace - from opentelemetry.sdk.trace import TracerProvider - from opentelemetry.sdk.trace.export import ( - ConsoleSpanExporter, - SimpleSpanProcessor, - ) - - trace.set_tracer_provider(TracerProvider()) - - # set up an exporter for sampled spans - trace.get_tracer_provider().add_span_processor( - SimpleSpanProcessor(ConsoleSpanExporter()) - ) - - # created spans will now be sampled by the TraceIdRatioBased sampler with rate 1/1000. - with trace.get_tracer(__name__).start_as_current_span("Test Span"): - ... +In order to configure you application with a custom sampler's entry point, set the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. For example, to configured the +above sampler, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and ``OTEL_TRACES_SAMPLER_ARG=0.5``. """ import abc import enum @@ -426,8 +427,7 @@ def _get_from_env_or_default() -> Sampler: trace_sampler_factory = _import_sampler_factory(trace_sampler_name) sampler_arg = os.getenv(OTEL_TRACES_SAMPLER_ARG, "") trace_sampler = trace_sampler_factory(sampler_arg) - _logger.warning("JEREVOSS: trace_sampler: %s" % trace_sampler) - if not issubclass(type(trace_sampler), Sampler): + if not isinstance(trace_sampler, Sampler): message = ( "Output of traces sampler factory, %s, was not a Sampler object." % trace_sampler_factory From da7b9f94a99cec0650f6577ee0ec8d0691e25c7d Mon Sep 17 00:00:00 2001 From: jerevoss Date: Thu, 20 Oct 2022 14:53:34 -0700 Subject: [PATCH 07/19] Aaron's feedback --- .../src/opentelemetry/sdk/trace/sampling.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 28792aa2456..3644e8d1a85 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -73,7 +73,7 @@ * parentbased_always_off - Sampler that respects its parent span's sampling decision, but otherwise never samples. * parentbased_traceidratio - Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on rate. -Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is traceidratio or parentbased_traceidratio, when not provided rate will be set to 1.0 (maximum rate possible). +Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is traceidratio or parentbased_traceidratio. Rate must be in the range [0.0,1.0]. When not provided rate will be set to 1.0 (maximum rate possible). Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. @@ -112,16 +112,16 @@ ] } ) - ... + # ... class CustomRatioSampler(Sampler): def __init__(rate): - ... - ... + # ... + # ... class CustomSamplerFactory: @staticmethod - get_sampler(sampler_argument_str): + get_sampler(sampler_argument): try: - rate = float(sampler_argument_str) + rate = float(sampler_argument) return CustomSampler(rate) except ValueError: # In case argument is empty string. return CustomSampler(0.5) @@ -437,7 +437,7 @@ def _get_from_env_or_default() -> Sampler: return trace_sampler except Exception as exc: _logger.warning( - "Failed to initialize custom sampler, %s: %s", + "Using default sampler. Failed to initialize custom sampler, %s: %s", trace_sampler_name, exc, ) From c091d5682df85250b673386bf9867e3b816b8d84 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 24 Oct 2022 13:55:50 -0700 Subject: [PATCH 08/19] lint --- .../src/opentelemetry/sdk/trace/sampling.py | 36 +++--- opentelemetry-sdk/tests/trace/test_trace.py | 106 +++++++++--------- 2 files changed, 70 insertions(+), 72 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 3644e8d1a85..5bb69279721 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -422,26 +422,22 @@ def _get_from_env_or_default() -> Sampler: rate = 1.0 return _KNOWN_SAMPLERS[trace_sampler_name](rate) return _KNOWN_SAMPLERS[trace_sampler_name] - else: - try: - trace_sampler_factory = _import_sampler_factory(trace_sampler_name) - sampler_arg = os.getenv(OTEL_TRACES_SAMPLER_ARG, "") - trace_sampler = trace_sampler_factory(sampler_arg) - if not isinstance(trace_sampler, Sampler): - message = ( - "Output of traces sampler factory, %s, was not a Sampler object." - % trace_sampler_factory - ) - _logger.warning(message) - raise ValueError(message) - return trace_sampler - except Exception as exc: - _logger.warning( - "Using default sampler. Failed to initialize custom sampler, %s: %s", - trace_sampler_name, - exc, - ) - return _KNOWN_SAMPLERS["parentbased_always_on"] + try: + trace_sampler_factory = _import_sampler_factory(trace_sampler_name) + sampler_arg = os.getenv(OTEL_TRACES_SAMPLER_ARG, "") + trace_sampler = trace_sampler_factory(sampler_arg) + if not isinstance(trace_sampler, Sampler): + message = f"Output of traces sampler factory, {trace_sampler_factory}, was not a Sampler object." + _logger.warning(message) + raise ValueError(message) + return trace_sampler + except Exception as exc: # pylint: disable=broad-except + _logger.warning( + "Using default sampler. Failed to initialize custom sampler, %s: %s", + trace_sampler_name, + exc, + ) + return _KNOWN_SAMPLERS["parentbased_always_on"] def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 13940ab8773..3f4d0d0da1c 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -21,7 +21,7 @@ from logging import ERROR, WARNING from random import randint from time import time_ns -from typing import Optional +from typing import Optional, Sequence from unittest import mock from opentelemetry import trace as trace_api @@ -39,15 +39,27 @@ OTEL_TRACES_SAMPLER, OTEL_TRACES_SAMPLER_ARG, ) -from opentelemetry.sdk.trace import Resource, sampling +from opentelemetry.sdk.trace import Resource from opentelemetry.sdk.trace.id_generator import RandomIdGenerator +from opentelemetry.sdk.trace.sampling import ( + ALWAYS_OFF, + ALWAYS_ON, + Decision, + ParentBased, + Sampler, + SamplingResult, + StaticSampler, + TraceIdRatioBased, +) from opentelemetry.sdk.util import ns_to_iso_str from opentelemetry.sdk.util.instrumentation import InstrumentationInfo from opentelemetry.test.spantestutil import ( get_span_with_dropped_attributes_events_links, new_tracer, ) -from opentelemetry.trace import Status, StatusCode +from opentelemetry.trace import Link, SpanKind, Status, StatusCode +from opentelemetry.trace.span import TraceState +from opentelemetry.util.types import Attributes class TestTracer(unittest.TestCase): @@ -139,60 +151,52 @@ def test_tracer_provider_accepts_concurrent_multi_span_processor(self): ) -class CustomSampler(sampling.Sampler): +class CustomSampler(Sampler): def __init__(self) -> None: - super().__init__() + pass def get_description(self) -> str: - return super().get_description() + return "CustomSampler" def should_sample( self, - parent_context, - trace_id, - name, - kind, - attributes, - links, - trace_state, - ): - return super().should_sample( - parent_context, - trace_id, - name, - kind, - attributes, - links, - trace_state, - ) - - -class CustomRatioSampler(sampling.TraceIdRatioBased): + parent_context: Optional["Context"], + trace_id: int, + name: str, + kind: SpanKind = None, + attributes: Attributes = None, + links: Sequence[Link] = None, + trace_state: TraceState = None, + ) -> "SamplingResult": + return SamplingResult( + Decision.RECORD_AND_SAMPLE, + None, + None, + ) + + +class CustomRatioSampler(TraceIdRatioBased): def __init__(self, ratio): self.ratio = ratio super().__init__(ratio) def get_description(self) -> str: - return super().get_description() + return "CustomSampler" def should_sample( self, - parent_context, - trace_id, - name, - kind, - attributes, - links, - trace_state, - ): - return super().should_sample( - parent_context, - trace_id, - name, - kind, - attributes, - links, - trace_state, + parent_context: Optional["Context"], + trace_id: int, + name: str, + kind: SpanKind = None, + attributes: Attributes = None, + links: Sequence[Link] = None, + trace_state: TraceState = None, + ) -> "SamplingResult": + return SamplingResult( + Decision.RECORD_AND_SAMPLE, + None, + None, ) @@ -248,7 +252,7 @@ def test_default_sampler_type(self): self.verify_default_sampler(tracer_provider) def test_sampler_no_sampling(self): - tracer_provider = trace.TracerProvider(sampling.ALWAYS_OFF) + tracer_provider = trace.TracerProvider(ALWAYS_OFF) tracer = tracer_provider.get_tracer(__name__) # Check that the default tracer creates no-op spans if the sampler @@ -272,10 +276,8 @@ def test_sampler_with_env(self): # pylint: disable=protected-access reload(trace) tracer_provider = trace.TracerProvider() - self.assertIsInstance(tracer_provider.sampler, sampling.StaticSampler) - self.assertEqual( - tracer_provider.sampler._decision, sampling.Decision.DROP - ) + self.assertIsInstance(tracer_provider.sampler, StaticSampler) + self.assertEqual(tracer_provider.sampler._decision, Decision.DROP) tracer = tracer_provider.get_tracer(__name__) @@ -294,7 +296,7 @@ def test_ratio_sampler_with_env(self): # pylint: disable=protected-access reload(trace) tracer_provider = trace.TracerProvider() - self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) + self.assertIsInstance(tracer_provider.sampler, ParentBased) self.assertEqual(tracer_provider.sampler._root.rate, 0.25) @mock.patch.dict( @@ -453,9 +455,9 @@ def test_custom_ratio_sampler_with_env_multiple_entry_points( self.assertIsInstance(tracer_provider.sampler, CustomSampler) def verify_default_sampler(self, tracer_provider): - self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) + self.assertIsInstance(tracer_provider.sampler, ParentBased) # pylint: disable=protected-access - self.assertEqual(tracer_provider.sampler._root, sampling.ALWAYS_ON) + self.assertEqual(tracer_provider.sampler._root, ALWAYS_ON) class TestSpanCreation(unittest.TestCase): @@ -950,7 +952,7 @@ def test_sampling_attributes(self): "attr-in-both": "decision-attr", } tracer_provider = trace.TracerProvider( - sampling.StaticSampler(sampling.Decision.RECORD_AND_SAMPLE) + StaticSampler(Decision.RECORD_AND_SAMPLE) ) self.tracer = tracer_provider.get_tracer(__name__) From 34bd68e9071652005c943b585de8b33daf5f19c2 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 24 Oct 2022 15:58:22 -0700 Subject: [PATCH 09/19] renamed vars traces_sampler* for consistency --- .../src/opentelemetry/sdk/trace/sampling.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 5bb69279721..38a3338b02f 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -407,12 +407,12 @@ def __init__(self, rate: float): def _get_from_env_or_default() -> Sampler: - trace_sampler_name = os.getenv( + traces_sampler_name = os.getenv( OTEL_TRACES_SAMPLER, "parentbased_always_on" ).lower() - if trace_sampler_name in _KNOWN_SAMPLERS: - if trace_sampler_name in ("traceidratio", "parentbased_traceidratio"): + if traces_sampler_name in _KNOWN_SAMPLERS: + if traces_sampler_name in ("traceidratio", "parentbased_traceidratio"): try: rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) except ValueError: @@ -420,21 +420,21 @@ def _get_from_env_or_default() -> Sampler: "Could not convert TRACES_SAMPLER_ARG to float." ) rate = 1.0 - return _KNOWN_SAMPLERS[trace_sampler_name](rate) - return _KNOWN_SAMPLERS[trace_sampler_name] + return _KNOWN_SAMPLERS[traces_sampler_name](rate) + return _KNOWN_SAMPLERS[traces_sampler_name] try: - trace_sampler_factory = _import_sampler_factory(trace_sampler_name) + traces_sampler_factory = _import_sampler_factory(traces_sampler_name) sampler_arg = os.getenv(OTEL_TRACES_SAMPLER_ARG, "") - trace_sampler = trace_sampler_factory(sampler_arg) - if not isinstance(trace_sampler, Sampler): - message = f"Output of traces sampler factory, {trace_sampler_factory}, was not a Sampler object." + traces_sampler = traces_sampler_factory(sampler_arg) + if not isinstance(traces_sampler, Sampler): + message = f"Traces sampler factory, {traces_sampler_factory}, produced output, {traces_sampler}, which is not a Sampler object." _logger.warning(message) raise ValueError(message) - return trace_sampler + return traces_sampler except Exception as exc: # pylint: disable=broad-except _logger.warning( "Using default sampler. Failed to initialize custom sampler, %s: %s", - trace_sampler_name, + traces_sampler_name, exc, ) return _KNOWN_SAMPLERS["parentbased_always_on"] From 63250c9e14cc3916e2b8c9b455e958278346d1d8 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Tue, 11 Oct 2022 16:02:11 -0700 Subject: [PATCH 10/19] Enable custom sampler configuration via env vars --- CHANGELOG.md | 2 + .../sdk/_configuration/__init__.py | 22 +--- .../src/opentelemetry/sdk/trace/sampling.py | 72 +++++++++++-- .../src/opentelemetry/sdk/util/__init__.py | 29 ++++- opentelemetry-sdk/tests/test_configurator.py | 2 +- opentelemetry-sdk/tests/trace/test_trace.py | 101 ++++++++++++++++++ 6 files changed, 191 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cab9302a09..39e2c356653 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.13.0...HEAD) +- Enabled custom samplers via entry points + ([#2972](https://github.com/open-telemetry/opentelemetry-python/pull/2972)) - Update explicit histogram bucket boundaries ([#2947](https://github.com/open-telemetry/opentelemetry-python/pull/2947)) - `exporter-otlp-proto-http`: add user agent string diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py index 159d471900a..1194894d837 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py @@ -23,7 +23,6 @@ from os import environ from typing import Dict, Optional, Sequence, Tuple, Type -from pkg_resources import iter_entry_points from typing_extensions import Literal from opentelemetry.environment_variables import ( @@ -55,6 +54,7 @@ from opentelemetry.sdk.trace import TracerProvider from opentelemetry.sdk.trace.export import BatchSpanProcessor, SpanExporter from opentelemetry.sdk.trace.id_generator import IdGenerator +from opentelemetry.sdk.util import _import_config_components from opentelemetry.semconv.resource import ResourceAttributes from opentelemetry.trace import set_tracer_provider @@ -228,26 +228,6 @@ def _init_logging( logging.getLogger().addHandler(handler) -def _import_config_components( - selected_components, entry_point_name -) -> Sequence[Tuple[str, object]]: - component_entry_points = { - ep.name: ep for ep in iter_entry_points(entry_point_name) - } - component_impls = [] - for selected_component in selected_components: - entry_point = component_entry_points.get(selected_component, None) - if not entry_point: - raise RuntimeError( - f"Requested component '{selected_component}' not found in entry points for '{entry_point_name}'" - ) - - component_impl = entry_point.load() - component_impls.append((selected_component, component_impl)) - - return component_impls - - def _import_exporters( trace_exporter_names: Sequence[str], metric_exporter_names: Sequence[str], diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 3cdd34cfe8c..dcb87e8b7db 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -64,7 +64,7 @@ ... The tracer sampler can also be configured via environment variables ``OTEL_TRACES_SAMPLER`` and ``OTEL_TRACES_SAMPLER_ARG`` (only if applicable). -The list of known values for ``OTEL_TRACES_SAMPLER`` are: +The list of built-in values for ``OTEL_TRACES_SAMPLER`` are: * always_on - Sampler that always samples spans, regardless of the parent span's sampling decision. * always_off - Sampler that never samples spans, regardless of the parent span's sampling decision. @@ -73,7 +73,24 @@ * parentbased_always_off - Sampler that respects its parent span's sampling decision, but otherwise never samples. * parentbased_traceidratio - Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on rate. -Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is traceidratio or parentbased_traceidratio, when not provided rate will be set to 1.0 (maximum rate possible). +In order to configure a custom sampler via environment variables, create an entry point for the custom sampler class under the entry point group, ``opentelemtry_traces_sampler``. Then, set the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. For example, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and ``OTEL_TRACES_SAMPLER_ARG=0.5`` after creating the following entry point: + +.. code:: python + + setup( + ... + entry_points={ + ... + "opentelemtry_traces_sampler": [ + "custom_sampler_name = path.to.sampler.module:CustomSampler" + ] + } + ) + ... + class CustomSampler(TraceIdRatioBased): + ... + +Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is a ``TraceIdRatioBased`` Sampler, such as ``traceidratio`` and ``parentbased_traceidratio``. When not provided rate will be set to 1.0 (maximum rate possible). Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. @@ -111,10 +128,13 @@ OTEL_TRACES_SAMPLER, OTEL_TRACES_SAMPLER_ARG, ) +from opentelemetry.sdk.util import _import_config_components from opentelemetry.trace import Link, SpanKind, get_current_span from opentelemetry.trace.span import TraceState from opentelemetry.util.types import Attributes +# from opentelemetry.sdk._configuration import _import_config_components + _logger = getLogger(__name__) @@ -161,6 +181,9 @@ def __init__( self.trace_state = trace_state +_OTEL_SAMPLER_ENTRY_POINT_GROUP = "opentelemtry_traces_sampler" + + class Sampler(abc.ABC): @abc.abstractmethod def should_sample( @@ -350,7 +373,7 @@ def get_description(self): """Sampler that respects its parent span's sampling decision, but otherwise always samples.""" -class ParentBasedTraceIdRatio(ParentBased): +class ParentBasedTraceIdRatio(ParentBased, TraceIdRatioBased): """ Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on `rate`. @@ -361,33 +384,48 @@ def __init__(self, rate: float): super().__init__(root=root) -_KNOWN_SAMPLERS = { +_KNOWN_INITIALIZED_SAMPLERS = { "always_on": ALWAYS_ON, "always_off": ALWAYS_OFF, "parentbased_always_on": DEFAULT_ON, "parentbased_always_off": DEFAULT_OFF, +} + +_KNOWN_SAMPLER_CLASSES = { "traceidratio": TraceIdRatioBased, "parentbased_traceidratio": ParentBasedTraceIdRatio, } def _get_from_env_or_default() -> Sampler: - trace_sampler = os.getenv( + trace_sampler_name = os.getenv( OTEL_TRACES_SAMPLER, "parentbased_always_on" ).lower() - if trace_sampler not in _KNOWN_SAMPLERS: - _logger.warning("Couldn't recognize sampler %s.", trace_sampler) - trace_sampler = "parentbased_always_on" - if trace_sampler in ("traceidratio", "parentbased_traceidratio"): + if trace_sampler_name in _KNOWN_INITIALIZED_SAMPLERS: + return _KNOWN_INITIALIZED_SAMPLERS[trace_sampler_name] + + trace_sampler_impl = None + if trace_sampler_name in _KNOWN_SAMPLER_CLASSES: + trace_sampler_impl = _KNOWN_SAMPLER_CLASSES[trace_sampler_name] + else: + try: + trace_sampler_impl = _import_sampler(trace_sampler_name) + except RuntimeError as err: + _logger.warning( + "Unable to recognize sampler %s: %s", trace_sampler_name, err + ) + return _KNOWN_INITIALIZED_SAMPLERS["parentbased_always_on"] + + if issubclass(trace_sampler_impl, TraceIdRatioBased): try: rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) except ValueError: _logger.warning("Could not convert TRACES_SAMPLER_ARG to float.") rate = 1.0 - return _KNOWN_SAMPLERS[trace_sampler](rate) + return trace_sampler_impl(rate) - return _KNOWN_SAMPLERS[trace_sampler] + return trace_sampler_impl() def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: @@ -395,3 +433,15 @@ def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: if parent_span_context is None or not parent_span_context.is_valid: return None return parent_span_context.trace_state + + +def _import_sampler(sampler_name: str) -> Sampler: + # pylint: disable=unbalanced-tuple-unpacking + [(sampler_name, sampler_impl)] = _import_config_components( + [sampler_name.strip()], _OTEL_SAMPLER_ENTRY_POINT_GROUP + ) + + if issubclass(sampler_impl, Sampler): + return sampler_impl + + raise RuntimeError(f"{sampler_name} is not an Sampler") diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py index e1857d8e62d..14fa28fd234 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py @@ -15,10 +15,11 @@ import datetime import threading from collections import OrderedDict, deque -from collections.abc import MutableMapping, Sequence -from typing import Optional +from collections import abc +from typing import Optional, Sequence, Tuple from deprecated import deprecated +from pkg_resources import iter_entry_points def ns_to_iso_str(nanoseconds): @@ -41,7 +42,27 @@ def get_dict_as_key(labels): ) -class BoundedList(Sequence): +def _import_config_components( + selected_components, entry_point_name +) -> Sequence[Tuple[str, object]]: + component_entry_points = { + ep.name: ep for ep in iter_entry_points(entry_point_name) + } + component_impls = [] + for selected_component in selected_components: + entry_point = component_entry_points.get(selected_component, None) + if not entry_point: + raise RuntimeError( + f"Requested component '{selected_component}' not found in entry points for '{entry_point_name}'" + ) + + component_impl = entry_point.load() + component_impls.append((selected_component, component_impl)) + + return component_impls + + +class BoundedList(abc.Sequence): """An append only list with a fixed max size. Calls to `append` and `extend` will drop the oldest elements if there is @@ -92,7 +113,7 @@ def from_seq(cls, maxlen, seq): @deprecated(version="1.4.0") # type: ignore -class BoundedDict(MutableMapping): +class BoundedDict(abc.MutableMapping): """An ordered dict with a fixed max capacity. Oldest elements are dropped when the dict is full and a new element is diff --git a/opentelemetry-sdk/tests/test_configurator.py b/opentelemetry-sdk/tests/test_configurator.py index 4aae8aa53be..e64607e8acd 100644 --- a/opentelemetry-sdk/tests/test_configurator.py +++ b/opentelemetry-sdk/tests/test_configurator.py @@ -257,7 +257,7 @@ def test_trace_init_otlp(self): @patch.dict(environ, {OTEL_PYTHON_ID_GENERATOR: "custom_id_generator"}) @patch("opentelemetry.sdk._configuration.IdGenerator", new=IdGenerator) - @patch("opentelemetry.sdk._configuration.iter_entry_points") + @patch("opentelemetry.sdk.util.iter_entry_points") def test_trace_init_custom_id_generator(self, mock_iter_entry_points): mock_iter_entry_points.configure_mock( return_value=[ diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 8b8d33faa45..cb2cc25692f 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -139,6 +139,63 @@ def test_tracer_provider_accepts_concurrent_multi_span_processor(self): ) +class CustomSampler(sampling.Sampler): + def __init__(self) -> None: + super().__init__() + + def get_description(self) -> str: + return super().get_description() + + def should_sample( + self, + parent_context, + trace_id, + name, + kind, + attributes, + links, + trace_state, + ): + return super().should_sample( + parent_context, + trace_id, + name, + kind, + attributes, + links, + trace_state, + ) + + +class CustomRatioSampler(sampling.TraceIdRatioBased): + def __init__(self, ratio): + self.ratio = ratio + super().__init__(ratio) + + def get_description(self) -> str: + return super().get_description() + + def should_sample( + self, + parent_context, + trace_id, + name, + kind, + attributes, + links, + trace_state, + ): + return super().should_sample( + parent_context, + trace_id, + name, + kind, + attributes, + links, + trace_state, + ) + + class TestTracerSampling(unittest.TestCase): def tearDown(self): reload(trace) @@ -219,6 +276,50 @@ def test_ratio_sampler_with_env(self): self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) self.assertEqual(tracer_provider.sampler._root.rate, 0.25) + @mock.patch.dict( + "os.environ", {OTEL_TRACES_SAMPLER: "non_existent_entry_point"} + ) + def test_sampler_with_env_non_existent_entry_point(self): + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) + # pylint: disable=protected-access + self.assertEqual(tracer_provider.sampler._root, sampling.ALWAYS_ON) + + @mock.patch("opentelemetry.sdk.trace.sampling._import_config_components") + @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler"}) + def test_custom_sampler_with_env( + self, mock_sampling_import_config_components + ): + mock_sampling_import_config_components.return_value = [ + ("custom_sampler", CustomSampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.assertIsInstance(tracer_provider.sampler, CustomSampler) + + @mock.patch("opentelemetry.sdk.trace.sampling._import_config_components") + @mock.patch.dict( + "os.environ", + { + OTEL_TRACES_SAMPLER: "custom_ratio_sampler", + OTEL_TRACES_SAMPLER_ARG: "0.5", + }, + ) + def test_custom_ratio_sampler_with_env( + self, mock_sampling_import_config_components + ): + mock_sampling_import_config_components.return_value = [ + ("custom_ratio_sampler", CustomRatioSampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.assertIsInstance(tracer_provider.sampler, CustomRatioSampler) + self.assertEqual(tracer_provider.sampler.ratio, 0.5) + class TestSpanCreation(unittest.TestCase): def test_start_span_invalid_spancontext(self): From 5acf5b9f826480b78d5331dc3f719d5abc15f7e7 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Thu, 13 Oct 2022 10:24:47 -0700 Subject: [PATCH 11/19] lint --- opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py | 2 +- opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index dcb87e8b7db..b0023a65bc9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -133,7 +133,6 @@ class CustomSampler(TraceIdRatioBased): from opentelemetry.trace.span import TraceState from opentelemetry.util.types import Attributes -# from opentelemetry.sdk._configuration import _import_config_components _logger = getLogger(__name__) @@ -380,6 +379,7 @@ class ParentBasedTraceIdRatio(ParentBased, TraceIdRatioBased): """ def __init__(self, rate: float): + # TODO: If TraceIdRatioBased inheritance is kept, change this root = TraceIdRatioBased(rate=rate) super().__init__(root=root) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py index 14fa28fd234..6d7f9d9bb59 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py @@ -14,8 +14,7 @@ import datetime import threading -from collections import OrderedDict, deque -from collections import abc +from collections import OrderedDict, abc, deque from typing import Optional, Sequence, Tuple from deprecated import deprecated From 3c1d776b5caeb680650433db480352454bec1ecd Mon Sep 17 00:00:00 2001 From: jerevoss Date: Thu, 13 Oct 2022 16:08:28 -0700 Subject: [PATCH 12/19] Using factory method approach instread --- .../src/opentelemetry/sdk/trace/sampling.py | 94 ++++++----- opentelemetry-sdk/tests/trace/test_trace.py | 146 ++++++++++++++++-- 2 files changed, 188 insertions(+), 52 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index b0023a65bc9..c644346cb61 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -73,7 +73,10 @@ * parentbased_always_off - Sampler that respects its parent span's sampling decision, but otherwise never samples. * parentbased_traceidratio - Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on rate. -In order to configure a custom sampler via environment variables, create an entry point for the custom sampler class under the entry point group, ``opentelemtry_traces_sampler``. Then, set the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. For example, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and ``OTEL_TRACES_SAMPLER_ARG=0.5`` after creating the following entry point: +In order to configure a custom sampler via environment variables, create an entry point for the custom sampler factory method under the entry point group, ``opentelemtry_traces_sampler``. Then, set +the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. The custom sampler factory method must take a single string argument. This input will come from +``OTEL_TRACES_SAMPLER_ARG``. If ``OTEL_TRACES_SAMPLER_ARG`` is not configured, the input will be an empty string. For example, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and +``OTEL_TRACES_SAMPLER_ARG=0.5`` after creating the following entry point: .. code:: python @@ -82,15 +85,22 @@ entry_points={ ... "opentelemtry_traces_sampler": [ - "custom_sampler_name = path.to.sampler.module:CustomSampler" + "custom_sampler_name = path.to.sampler.factory.method:CustomSamplerFactory.get_sampler" ] } ) ... - class CustomSampler(TraceIdRatioBased): - ... + class CustomRatioSampler(Sampler): + def __init__(rate): + ... + ... + class CustomSamplerFactory: + get_sampler(sampler_argument_str): + rate = float(sampler_argument_str) + return CustomSampler(rate) -Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is a ``TraceIdRatioBased`` Sampler, such as ``traceidratio`` and ``parentbased_traceidratio``. When not provided rate will be set to 1.0 (maximum rate possible). +Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is a ``TraceIdRatioBased`` Sampler, such as ``traceidratio`` and ``parentbased_traceidratio``. When not provided rate +will be set to 1.0 (maximum rate possible). Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. @@ -372,60 +382,76 @@ def get_description(self): """Sampler that respects its parent span's sampling decision, but otherwise always samples.""" -class ParentBasedTraceIdRatio(ParentBased, TraceIdRatioBased): +class ParentBasedTraceIdRatio(ParentBased): """ Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on `rate`. """ def __init__(self, rate: float): - # TODO: If TraceIdRatioBased inheritance is kept, change this root = TraceIdRatioBased(rate=rate) super().__init__(root=root) -_KNOWN_INITIALIZED_SAMPLERS = { +_KNOWN_SAMPLERS = { "always_on": ALWAYS_ON, "always_off": ALWAYS_OFF, "parentbased_always_on": DEFAULT_ON, "parentbased_always_off": DEFAULT_OFF, -} - -_KNOWN_SAMPLER_CLASSES = { "traceidratio": TraceIdRatioBased, "parentbased_traceidratio": ParentBasedTraceIdRatio, } def _get_from_env_or_default() -> Sampler: - trace_sampler_name = os.getenv( + trace_sampler = os.getenv( OTEL_TRACES_SAMPLER, "parentbased_always_on" ).lower() + if trace_sampler not in _KNOWN_SAMPLERS: + _logger.warning("Couldn't recognize sampler %s.", trace_sampler) + trace_sampler = "parentbased_always_on" - if trace_sampler_name in _KNOWN_INITIALIZED_SAMPLERS: - return _KNOWN_INITIALIZED_SAMPLERS[trace_sampler_name] - - trace_sampler_impl = None - if trace_sampler_name in _KNOWN_SAMPLER_CLASSES: - trace_sampler_impl = _KNOWN_SAMPLER_CLASSES[trace_sampler_name] - else: - try: - trace_sampler_impl = _import_sampler(trace_sampler_name) - except RuntimeError as err: - _logger.warning( - "Unable to recognize sampler %s: %s", trace_sampler_name, err - ) - return _KNOWN_INITIALIZED_SAMPLERS["parentbased_always_on"] - - if issubclass(trace_sampler_impl, TraceIdRatioBased): + if trace_sampler in ("traceidratio", "parentbased_traceidratio"): try: rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) except ValueError: _logger.warning("Could not convert TRACES_SAMPLER_ARG to float.") rate = 1.0 - return trace_sampler_impl(rate) + return _KNOWN_SAMPLERS[trace_sampler](rate) - return trace_sampler_impl() + return _KNOWN_SAMPLERS[trace_sampler] + + +def _get_from_env_or_default() -> Sampler: + trace_sampler_name = os.getenv( + OTEL_TRACES_SAMPLER, "parentbased_always_on" + ).lower() + + if trace_sampler_name in _KNOWN_SAMPLERS: + if trace_sampler_name in ("traceidratio", "parentbased_traceidratio"): + try: + rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) + except ValueError: + _logger.warning("Could not convert TRACES_SAMPLER_ARG to float.") + rate = 1.0 + return _KNOWN_SAMPLERS[trace_sampler_name](rate) + return _KNOWN_SAMPLERS[trace_sampler_name] + else: + try: + trace_sampler_factory = _import_sampler_factory(trace_sampler_name) + sampler_arg = os.getenv(OTEL_TRACES_SAMPLER_ARG, "") + trace_sampler = trace_sampler_factory(sampler_arg) + _logger.warning("JEREVOSS: trace_sampler: %s" % trace_sampler) + if not issubclass(type(trace_sampler), Sampler): + message = "Output of traces sampler factory, %s, was not a Sampler object." % trace_sampler_factory + _logger.warning(message) + raise ValueError(message) + return trace_sampler + except Exception as exc: + _logger.warning( + "Failed to initialize custom sampler, %s: %s", trace_sampler_name, exc + ) + return _KNOWN_SAMPLERS["parentbased_always_on"] def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: @@ -435,13 +461,9 @@ def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: return parent_span_context.trace_state -def _import_sampler(sampler_name: str) -> Sampler: +def _import_sampler_factory(sampler_name: str) -> Sampler: # pylint: disable=unbalanced-tuple-unpacking [(sampler_name, sampler_impl)] = _import_config_components( [sampler_name.strip()], _OTEL_SAMPLER_ENTRY_POINT_GROUP ) - - if issubclass(sampler_impl, Sampler): - return sampler_impl - - raise RuntimeError(f"{sampler_name} is not an Sampler") + return sampler_impl diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index cb2cc25692f..623a648c37c 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -196,6 +196,26 @@ def should_sample( ) +class CustomSamplerFactory: + def get_custom_sampler(unused_sampler_arg): + return CustomSampler() + + def get_custom_ratio_sampler(sampler_arg): + return CustomRatioSampler(float(sampler_arg)) + + def empty_get_custom_sampler(sampler_arg): + return + + +class IterEntryPoint: + def __init__(self, name, class_type): + self.name = name + self.class_type = class_type + + def load(self): + return self.class_type + + class TestTracerSampling(unittest.TestCase): def tearDown(self): reload(trace) @@ -222,9 +242,7 @@ def test_default_sampler(self): def test_default_sampler_type(self): tracer_provider = trace.TracerProvider() - self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) - # pylint: disable=protected-access - self.assertEqual(tracer_provider.sampler._root, sampling.ALWAYS_ON) + self.verify_default_sampler(tracer_provider) def test_sampler_no_sampling(self): tracer_provider = trace.TracerProvider(sampling.ALWAYS_OFF) @@ -283,36 +301,69 @@ def test_sampler_with_env_non_existent_entry_point(self): # pylint: disable=protected-access reload(trace) tracer_provider = trace.TracerProvider() - self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) - # pylint: disable=protected-access - self.assertEqual(tracer_provider.sampler._root, sampling.ALWAYS_ON) + self.verify_default_sampler(tracer_provider) - @mock.patch("opentelemetry.sdk.trace.sampling._import_config_components") - @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler"}) + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"}) def test_custom_sampler_with_env( - self, mock_sampling_import_config_components + self, mock_iter_entry_points + ): + # mock_iter_entry_points.return_value = [ + # ("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) + # ] + mock_iter_entry_points.return_value=[ + IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.assertIsInstance(tracer_provider.sampler, CustomSampler) + + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"}) + def test_custom_sampler_with_env_bad_factory( + self, mock_iter_entry_points + ): + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.empty_get_custom_sampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.verify_default_sampler(tracer_provider) + + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict( + "os.environ", + { + OTEL_TRACES_SAMPLER: "custom_sampler_factory", + OTEL_TRACES_SAMPLER_ARG: "0.5", + }, + ) + def test_custom_sampler_with_env_unused_arg( + self, mock_iter_entry_points ): - mock_sampling_import_config_components.return_value = [ - ("custom_sampler", CustomSampler) + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) ] # pylint: disable=protected-access reload(trace) tracer_provider = trace.TracerProvider() self.assertIsInstance(tracer_provider.sampler, CustomSampler) - @mock.patch("opentelemetry.sdk.trace.sampling._import_config_components") + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") @mock.patch.dict( "os.environ", { - OTEL_TRACES_SAMPLER: "custom_ratio_sampler", + OTEL_TRACES_SAMPLER: "custom_ratio_sampler_factory", OTEL_TRACES_SAMPLER_ARG: "0.5", }, ) def test_custom_ratio_sampler_with_env( - self, mock_sampling_import_config_components + self, mock_iter_entry_points ): - mock_sampling_import_config_components.return_value = [ - ("custom_ratio_sampler", CustomRatioSampler) + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) ] # pylint: disable=protected-access reload(trace) @@ -320,6 +371,69 @@ def test_custom_ratio_sampler_with_env( self.assertIsInstance(tracer_provider.sampler, CustomRatioSampler) self.assertEqual(tracer_provider.sampler.ratio, 0.5) + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict( + "os.environ", + { + OTEL_TRACES_SAMPLER: "custom_ratio_sampler_factory", + OTEL_TRACES_SAMPLER_ARG: "foobar", + }, + ) + def test_custom_ratio_sampler_with_env_bad_arg( + self, mock_iter_entry_points + ): + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.verify_default_sampler(tracer_provider) + + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict( + "os.environ", + { + OTEL_TRACES_SAMPLER: "custom_ratio_sampler_factory", + }, + ) + def test_custom_ratio_sampler_with_env_no_arg( + self, mock_iter_entry_points + ): + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.verify_default_sampler(tracer_provider) + + @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") + @mock.patch.dict( + "os.environ", + { + OTEL_TRACES_SAMPLER: "custom_sampler_factory", + OTEL_TRACES_SAMPLER_ARG: "0.5", + }, + ) + def test_custom_ratio_sampler_with_env_multiple_entry_points( + self, mock_iter_entry_points + ): + mock_iter_entry_points.return_value = [ + IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler), + IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler), + IterEntryPoint("custom_z_sampler_factory", CustomSamplerFactory.empty_get_custom_sampler) + ] + # pylint: disable=protected-access + reload(trace) + tracer_provider = trace.TracerProvider() + self.assertIsInstance(tracer_provider.sampler, CustomSampler) + + def verify_default_sampler(self, tracer_provider): + self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) + # pylint: disable=protected-access + self.assertEqual(tracer_provider.sampler._root, sampling.ALWAYS_ON) + class TestSpanCreation(unittest.TestCase): def test_start_span_invalid_spancontext(self): From d67ba77ea1b6997a54c9c008a15b5c5111632858 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Thu, 13 Oct 2022 16:22:30 -0700 Subject: [PATCH 13/19] lint --- .../src/opentelemetry/sdk/trace/sampling.py | 14 ++-- opentelemetry-sdk/tests/trace/test_trace.py | 71 ++++++++++++------- 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index c644346cb61..5b9218a668a 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -143,7 +143,6 @@ class CustomSamplerFactory: from opentelemetry.trace.span import TraceState from opentelemetry.util.types import Attributes - _logger = getLogger(__name__) @@ -432,7 +431,9 @@ def _get_from_env_or_default() -> Sampler: try: rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) except ValueError: - _logger.warning("Could not convert TRACES_SAMPLER_ARG to float.") + _logger.warning( + "Could not convert TRACES_SAMPLER_ARG to float." + ) rate = 1.0 return _KNOWN_SAMPLERS[trace_sampler_name](rate) return _KNOWN_SAMPLERS[trace_sampler_name] @@ -443,13 +444,18 @@ def _get_from_env_or_default() -> Sampler: trace_sampler = trace_sampler_factory(sampler_arg) _logger.warning("JEREVOSS: trace_sampler: %s" % trace_sampler) if not issubclass(type(trace_sampler), Sampler): - message = "Output of traces sampler factory, %s, was not a Sampler object." % trace_sampler_factory + message = ( + "Output of traces sampler factory, %s, was not a Sampler object." + % trace_sampler_factory + ) _logger.warning(message) raise ValueError(message) return trace_sampler except Exception as exc: _logger.warning( - "Failed to initialize custom sampler, %s: %s", trace_sampler_name, exc + "Failed to initialize custom sampler, %s: %s", + trace_sampler_name, + exc, ) return _KNOWN_SAMPLERS["parentbased_always_on"] diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 623a648c37c..c1d731b947a 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -304,15 +304,18 @@ def test_sampler_with_env_non_existent_entry_point(self): self.verify_default_sampler(tracer_provider) @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") - @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"}) - def test_custom_sampler_with_env( - self, mock_iter_entry_points - ): + @mock.patch.dict( + "os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"} + ) + def test_custom_sampler_with_env(self, mock_iter_entry_points): # mock_iter_entry_points.return_value = [ # ("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) # ] - mock_iter_entry_points.return_value=[ - IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) + mock_iter_entry_points.return_value = [ + IterEntryPoint( + "custom_sampler_factory", + CustomSamplerFactory.get_custom_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -320,12 +323,15 @@ def test_custom_sampler_with_env( self.assertIsInstance(tracer_provider.sampler, CustomSampler) @mock.patch("opentelemetry.sdk.trace.util.iter_entry_points") - @mock.patch.dict("os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"}) - def test_custom_sampler_with_env_bad_factory( - self, mock_iter_entry_points - ): + @mock.patch.dict( + "os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"} + ) + def test_custom_sampler_with_env_bad_factory(self, mock_iter_entry_points): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.empty_get_custom_sampler) + IterEntryPoint( + "custom_sampler_factory", + CustomSamplerFactory.empty_get_custom_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -340,11 +346,12 @@ def test_custom_sampler_with_env_bad_factory( OTEL_TRACES_SAMPLER_ARG: "0.5", }, ) - def test_custom_sampler_with_env_unused_arg( - self, mock_iter_entry_points - ): + def test_custom_sampler_with_env_unused_arg(self, mock_iter_entry_points): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) + IterEntryPoint( + "custom_sampler_factory", + CustomSamplerFactory.get_custom_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -359,11 +366,12 @@ def test_custom_sampler_with_env_unused_arg( OTEL_TRACES_SAMPLER_ARG: "0.5", }, ) - def test_custom_ratio_sampler_with_env( - self, mock_iter_entry_points - ): + def test_custom_ratio_sampler_with_env(self, mock_iter_entry_points): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) + IterEntryPoint( + "custom_ratio_sampler_factory", + CustomSamplerFactory.get_custom_ratio_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -383,7 +391,10 @@ def test_custom_ratio_sampler_with_env_bad_arg( self, mock_iter_entry_points ): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) + IterEntryPoint( + "custom_ratio_sampler_factory", + CustomSamplerFactory.get_custom_ratio_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -401,7 +412,10 @@ def test_custom_ratio_sampler_with_env_no_arg( self, mock_iter_entry_points ): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler) + IterEntryPoint( + "custom_ratio_sampler_factory", + CustomSamplerFactory.get_custom_ratio_sampler, + ) ] # pylint: disable=protected-access reload(trace) @@ -420,9 +434,18 @@ def test_custom_ratio_sampler_with_env_multiple_entry_points( self, mock_iter_entry_points ): mock_iter_entry_points.return_value = [ - IterEntryPoint("custom_ratio_sampler_factory", CustomSamplerFactory.get_custom_ratio_sampler), - IterEntryPoint("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler), - IterEntryPoint("custom_z_sampler_factory", CustomSamplerFactory.empty_get_custom_sampler) + IterEntryPoint( + "custom_ratio_sampler_factory", + CustomSamplerFactory.get_custom_ratio_sampler, + ), + IterEntryPoint( + "custom_sampler_factory", + CustomSamplerFactory.get_custom_sampler, + ), + IterEntryPoint( + "custom_z_sampler_factory", + CustomSamplerFactory.empty_get_custom_sampler, + ), ] # pylint: disable=protected-access reload(trace) From fcddc03c0772e5711ff62718ad65e8bf00b2b225 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 17 Oct 2022 15:06:44 -0700 Subject: [PATCH 14/19] Resolving comments --- .../sdk/_configuration/__init__.py | 5 +- .../src/opentelemetry/sdk/trace/sampling.py | 49 ++++++------------- .../src/opentelemetry/sdk/util/__init__.py | 4 +- opentelemetry-sdk/tests/trace/test_trace.py | 6 +-- 4 files changed, 23 insertions(+), 41 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py index 1194894d837..910ee4b4116 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_configuration/__init__.py @@ -269,10 +269,9 @@ def _import_exporters( def _import_id_generator(id_generator_name: str) -> IdGenerator: - # pylint: disable=unbalanced-tuple-unpacking - [(id_generator_name, id_generator_impl)] = _import_config_components( + id_generator_name, id_generator_impl = _import_config_components( [id_generator_name.strip()], "opentelemetry_id_generator" - ) + )[0] if issubclass(id_generator_impl, IdGenerator): return id_generator_impl diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 5b9218a668a..3b0c30a65c8 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -73,10 +73,10 @@ * parentbased_always_off - Sampler that respects its parent span's sampling decision, but otherwise never samples. * parentbased_traceidratio - Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on rate. -In order to configure a custom sampler via environment variables, create an entry point for the custom sampler factory method under the entry point group, ``opentelemtry_traces_sampler``. Then, set -the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. The custom sampler factory method must take a single string argument. This input will come from -``OTEL_TRACES_SAMPLER_ARG``. If ``OTEL_TRACES_SAMPLER_ARG`` is not configured, the input will be an empty string. For example, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and -``OTEL_TRACES_SAMPLER_ARG=0.5`` after creating the following entry point: +Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is traceidratio or parentbased_traceidratio, when not provided rate will be set to 1.0 (maximum rate possible). + +In order to create a configurable custom sampler, create an entry point for the custom sampler factory method under the entry point group, ``opentelemtry_traces_sampler``. The custom sampler factory method must be of type ``Callable[[str], Sampler]``, taking a single string argument and returning a Sampler object. The single input will come from the string value of +the ``OTEL_TRACES_SAMPLER_ARG`` environment variable. If ``OTEL_TRACES_SAMPLER_ARG`` is not configured, the input will be an empty string. For example: .. code:: python @@ -95,12 +95,15 @@ def __init__(rate): ... ... class CustomSamplerFactory: + @staticmethod get_sampler(sampler_argument_str): - rate = float(sampler_argument_str) - return CustomSampler(rate) + try: + rate = float(sampler_argument_str) + return CustomSampler(rate) + except ValueError: # In case argument is empty string. + return CustomSampler(0.5) -Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is a ``TraceIdRatioBased`` Sampler, such as ``traceidratio`` and ``parentbased_traceidratio``. When not provided rate -will be set to 1.0 (maximum rate possible). +In order to configure you application with a custom sampler's entry point, set the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. For example, to configured the above sampler, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and ``OTEL_TRACES_SAMPLER_ARG=0.5``. Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. @@ -130,7 +133,7 @@ class CustomSamplerFactory: import os from logging import getLogger from types import MappingProxyType -from typing import Optional, Sequence +from typing import Callable, Optional, Sequence # pylint: disable=unused-import from opentelemetry.context import Context @@ -189,7 +192,7 @@ def __init__( self.trace_state = trace_state -_OTEL_SAMPLER_ENTRY_POINT_GROUP = "opentelemtry_traces_sampler" +_OTEL_SAMPLER_ENTRY_POINT_GROUP = "opentelemetry_traces_sampler" class Sampler(abc.ABC): @@ -402,25 +405,6 @@ def __init__(self, rate: float): } -def _get_from_env_or_default() -> Sampler: - trace_sampler = os.getenv( - OTEL_TRACES_SAMPLER, "parentbased_always_on" - ).lower() - if trace_sampler not in _KNOWN_SAMPLERS: - _logger.warning("Couldn't recognize sampler %s.", trace_sampler) - trace_sampler = "parentbased_always_on" - - if trace_sampler in ("traceidratio", "parentbased_traceidratio"): - try: - rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) - except ValueError: - _logger.warning("Could not convert TRACES_SAMPLER_ARG to float.") - rate = 1.0 - return _KNOWN_SAMPLERS[trace_sampler](rate) - - return _KNOWN_SAMPLERS[trace_sampler] - - def _get_from_env_or_default() -> Sampler: trace_sampler_name = os.getenv( OTEL_TRACES_SAMPLER, "parentbased_always_on" @@ -467,9 +451,8 @@ def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: return parent_span_context.trace_state -def _import_sampler_factory(sampler_name: str) -> Sampler: - # pylint: disable=unbalanced-tuple-unpacking - [(sampler_name, sampler_impl)] = _import_config_components( +def _import_sampler_factory(sampler_name: str) -> Callable[[str], Sampler]: + _, sampler_impl = _import_config_components( [sampler_name.strip()], _OTEL_SAMPLER_ENTRY_POINT_GROUP - ) + )[0] return sampler_impl diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py index 6d7f9d9bb59..52104243532 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py @@ -15,7 +15,7 @@ import datetime import threading from collections import OrderedDict, abc, deque -from typing import Optional, Sequence, Tuple +from typing import List, Optional, Sequence, Tuple from deprecated import deprecated from pkg_resources import iter_entry_points @@ -42,7 +42,7 @@ def get_dict_as_key(labels): def _import_config_components( - selected_components, entry_point_name + selected_components: List[str], entry_point_name: str ) -> Sequence[Tuple[str, object]]: component_entry_points = { ep.name: ep for ep in iter_entry_points(entry_point_name) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index c1d731b947a..13940ab8773 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -197,12 +197,15 @@ def should_sample( class CustomSamplerFactory: + @staticmethod def get_custom_sampler(unused_sampler_arg): return CustomSampler() + @staticmethod def get_custom_ratio_sampler(sampler_arg): return CustomRatioSampler(float(sampler_arg)) + @staticmethod def empty_get_custom_sampler(sampler_arg): return @@ -308,9 +311,6 @@ def test_sampler_with_env_non_existent_entry_point(self): "os.environ", {OTEL_TRACES_SAMPLER: "custom_sampler_factory"} ) def test_custom_sampler_with_env(self, mock_iter_entry_points): - # mock_iter_entry_points.return_value = [ - # ("custom_sampler_factory", CustomSamplerFactory.get_custom_sampler) - # ] mock_iter_entry_points.return_value = [ IterEntryPoint( "custom_sampler_factory", From e22c4dd97b0daf6c85ded50dcaa28191c400149c Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 17 Oct 2022 15:28:30 -0700 Subject: [PATCH 15/19] resolving comments --- .../src/opentelemetry/sdk/trace/sampling.py | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 3b0c30a65c8..28792aa2456 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -75,8 +75,31 @@ Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is traceidratio or parentbased_traceidratio, when not provided rate will be set to 1.0 (maximum rate possible). -In order to create a configurable custom sampler, create an entry point for the custom sampler factory method under the entry point group, ``opentelemtry_traces_sampler``. The custom sampler factory method must be of type ``Callable[[str], Sampler]``, taking a single string argument and returning a Sampler object. The single input will come from the string value of -the ``OTEL_TRACES_SAMPLER_ARG`` environment variable. If ``OTEL_TRACES_SAMPLER_ARG`` is not configured, the input will be an empty string. For example: +Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. + +.. code:: python + + from opentelemetry import trace + from opentelemetry.sdk.trace import TracerProvider + from opentelemetry.sdk.trace.export import ( + ConsoleSpanExporter, + SimpleSpanProcessor, + ) + + trace.set_tracer_provider(TracerProvider()) + + # set up an exporter for sampled spans + trace.get_tracer_provider().add_span_processor( + SimpleSpanProcessor(ConsoleSpanExporter()) + ) + + # created spans will now be sampled by the TraceIdRatioBased sampler with rate 1/1000. + with trace.get_tracer(__name__).start_as_current_span("Test Span"): + ... + +In order to create a configurable custom sampler, create an entry point for the custom sampler factory method under the entry point group, ``opentelemetry_traces_sampler``. The custom sampler factory +method must be of type ``Callable[[str], Sampler]``, taking a single string argument and returning a Sampler object. The single input will come from the string value of the +``OTEL_TRACES_SAMPLER_ARG`` environment variable. If ``OTEL_TRACES_SAMPLER_ARG`` is not configured, the input will be an empty string. For example: .. code:: python @@ -84,7 +107,7 @@ ... entry_points={ ... - "opentelemtry_traces_sampler": [ + "opentelemetry_traces_sampler": [ "custom_sampler_name = path.to.sampler.factory.method:CustomSamplerFactory.get_sampler" ] } @@ -103,30 +126,8 @@ class CustomSamplerFactory: except ValueError: # In case argument is empty string. return CustomSampler(0.5) -In order to configure you application with a custom sampler's entry point, set the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. For example, to configured the above sampler, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and ``OTEL_TRACES_SAMPLER_ARG=0.5``. - - -Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. - -.. code:: python - - from opentelemetry import trace - from opentelemetry.sdk.trace import TracerProvider - from opentelemetry.sdk.trace.export import ( - ConsoleSpanExporter, - SimpleSpanProcessor, - ) - - trace.set_tracer_provider(TracerProvider()) - - # set up an exporter for sampled spans - trace.get_tracer_provider().add_span_processor( - SimpleSpanProcessor(ConsoleSpanExporter()) - ) - - # created spans will now be sampled by the TraceIdRatioBased sampler with rate 1/1000. - with trace.get_tracer(__name__).start_as_current_span("Test Span"): - ... +In order to configure you application with a custom sampler's entry point, set the ``OTEL_TRACES_SAMPLER`` environment variable to the key name of the entry point. For example, to configured the +above sampler, set ``OTEL_TRACES_SAMPLER=custom_sampler_name`` and ``OTEL_TRACES_SAMPLER_ARG=0.5``. """ import abc import enum @@ -426,8 +427,7 @@ def _get_from_env_or_default() -> Sampler: trace_sampler_factory = _import_sampler_factory(trace_sampler_name) sampler_arg = os.getenv(OTEL_TRACES_SAMPLER_ARG, "") trace_sampler = trace_sampler_factory(sampler_arg) - _logger.warning("JEREVOSS: trace_sampler: %s" % trace_sampler) - if not issubclass(type(trace_sampler), Sampler): + if not isinstance(trace_sampler, Sampler): message = ( "Output of traces sampler factory, %s, was not a Sampler object." % trace_sampler_factory From 0ae3134d99fb6bc32cfae4ae6728b14f468e186f Mon Sep 17 00:00:00 2001 From: jerevoss Date: Thu, 20 Oct 2022 14:53:34 -0700 Subject: [PATCH 16/19] Aaron's feedback --- .../src/opentelemetry/sdk/trace/sampling.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 28792aa2456..3644e8d1a85 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -73,7 +73,7 @@ * parentbased_always_off - Sampler that respects its parent span's sampling decision, but otherwise never samples. * parentbased_traceidratio - Sampler that respects its parent span's sampling decision, but otherwise samples probabalistically based on rate. -Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is traceidratio or parentbased_traceidratio, when not provided rate will be set to 1.0 (maximum rate possible). +Sampling probability can be set with ``OTEL_TRACES_SAMPLER_ARG`` if the sampler is traceidratio or parentbased_traceidratio. Rate must be in the range [0.0,1.0]. When not provided rate will be set to 1.0 (maximum rate possible). Prev example but with environment variables. Please make sure to set the env ``OTEL_TRACES_SAMPLER=traceidratio`` and ``OTEL_TRACES_SAMPLER_ARG=0.001``. @@ -112,16 +112,16 @@ ] } ) - ... + # ... class CustomRatioSampler(Sampler): def __init__(rate): - ... - ... + # ... + # ... class CustomSamplerFactory: @staticmethod - get_sampler(sampler_argument_str): + get_sampler(sampler_argument): try: - rate = float(sampler_argument_str) + rate = float(sampler_argument) return CustomSampler(rate) except ValueError: # In case argument is empty string. return CustomSampler(0.5) @@ -437,7 +437,7 @@ def _get_from_env_or_default() -> Sampler: return trace_sampler except Exception as exc: _logger.warning( - "Failed to initialize custom sampler, %s: %s", + "Using default sampler. Failed to initialize custom sampler, %s: %s", trace_sampler_name, exc, ) From f927379d16e92b08669783019601dee0a1c7e096 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 24 Oct 2022 13:55:50 -0700 Subject: [PATCH 17/19] lint --- .../src/opentelemetry/sdk/trace/sampling.py | 36 +++--- opentelemetry-sdk/tests/trace/test_trace.py | 106 +++++++++--------- 2 files changed, 70 insertions(+), 72 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 3644e8d1a85..5bb69279721 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -422,26 +422,22 @@ def _get_from_env_or_default() -> Sampler: rate = 1.0 return _KNOWN_SAMPLERS[trace_sampler_name](rate) return _KNOWN_SAMPLERS[trace_sampler_name] - else: - try: - trace_sampler_factory = _import_sampler_factory(trace_sampler_name) - sampler_arg = os.getenv(OTEL_TRACES_SAMPLER_ARG, "") - trace_sampler = trace_sampler_factory(sampler_arg) - if not isinstance(trace_sampler, Sampler): - message = ( - "Output of traces sampler factory, %s, was not a Sampler object." - % trace_sampler_factory - ) - _logger.warning(message) - raise ValueError(message) - return trace_sampler - except Exception as exc: - _logger.warning( - "Using default sampler. Failed to initialize custom sampler, %s: %s", - trace_sampler_name, - exc, - ) - return _KNOWN_SAMPLERS["parentbased_always_on"] + try: + trace_sampler_factory = _import_sampler_factory(trace_sampler_name) + sampler_arg = os.getenv(OTEL_TRACES_SAMPLER_ARG, "") + trace_sampler = trace_sampler_factory(sampler_arg) + if not isinstance(trace_sampler, Sampler): + message = f"Output of traces sampler factory, {trace_sampler_factory}, was not a Sampler object." + _logger.warning(message) + raise ValueError(message) + return trace_sampler + except Exception as exc: # pylint: disable=broad-except + _logger.warning( + "Using default sampler. Failed to initialize custom sampler, %s: %s", + trace_sampler_name, + exc, + ) + return _KNOWN_SAMPLERS["parentbased_always_on"] def _get_parent_trace_state(parent_context) -> Optional["TraceState"]: diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 13940ab8773..3f4d0d0da1c 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -21,7 +21,7 @@ from logging import ERROR, WARNING from random import randint from time import time_ns -from typing import Optional +from typing import Optional, Sequence from unittest import mock from opentelemetry import trace as trace_api @@ -39,15 +39,27 @@ OTEL_TRACES_SAMPLER, OTEL_TRACES_SAMPLER_ARG, ) -from opentelemetry.sdk.trace import Resource, sampling +from opentelemetry.sdk.trace import Resource from opentelemetry.sdk.trace.id_generator import RandomIdGenerator +from opentelemetry.sdk.trace.sampling import ( + ALWAYS_OFF, + ALWAYS_ON, + Decision, + ParentBased, + Sampler, + SamplingResult, + StaticSampler, + TraceIdRatioBased, +) from opentelemetry.sdk.util import ns_to_iso_str from opentelemetry.sdk.util.instrumentation import InstrumentationInfo from opentelemetry.test.spantestutil import ( get_span_with_dropped_attributes_events_links, new_tracer, ) -from opentelemetry.trace import Status, StatusCode +from opentelemetry.trace import Link, SpanKind, Status, StatusCode +from opentelemetry.trace.span import TraceState +from opentelemetry.util.types import Attributes class TestTracer(unittest.TestCase): @@ -139,60 +151,52 @@ def test_tracer_provider_accepts_concurrent_multi_span_processor(self): ) -class CustomSampler(sampling.Sampler): +class CustomSampler(Sampler): def __init__(self) -> None: - super().__init__() + pass def get_description(self) -> str: - return super().get_description() + return "CustomSampler" def should_sample( self, - parent_context, - trace_id, - name, - kind, - attributes, - links, - trace_state, - ): - return super().should_sample( - parent_context, - trace_id, - name, - kind, - attributes, - links, - trace_state, - ) - - -class CustomRatioSampler(sampling.TraceIdRatioBased): + parent_context: Optional["Context"], + trace_id: int, + name: str, + kind: SpanKind = None, + attributes: Attributes = None, + links: Sequence[Link] = None, + trace_state: TraceState = None, + ) -> "SamplingResult": + return SamplingResult( + Decision.RECORD_AND_SAMPLE, + None, + None, + ) + + +class CustomRatioSampler(TraceIdRatioBased): def __init__(self, ratio): self.ratio = ratio super().__init__(ratio) def get_description(self) -> str: - return super().get_description() + return "CustomSampler" def should_sample( self, - parent_context, - trace_id, - name, - kind, - attributes, - links, - trace_state, - ): - return super().should_sample( - parent_context, - trace_id, - name, - kind, - attributes, - links, - trace_state, + parent_context: Optional["Context"], + trace_id: int, + name: str, + kind: SpanKind = None, + attributes: Attributes = None, + links: Sequence[Link] = None, + trace_state: TraceState = None, + ) -> "SamplingResult": + return SamplingResult( + Decision.RECORD_AND_SAMPLE, + None, + None, ) @@ -248,7 +252,7 @@ def test_default_sampler_type(self): self.verify_default_sampler(tracer_provider) def test_sampler_no_sampling(self): - tracer_provider = trace.TracerProvider(sampling.ALWAYS_OFF) + tracer_provider = trace.TracerProvider(ALWAYS_OFF) tracer = tracer_provider.get_tracer(__name__) # Check that the default tracer creates no-op spans if the sampler @@ -272,10 +276,8 @@ def test_sampler_with_env(self): # pylint: disable=protected-access reload(trace) tracer_provider = trace.TracerProvider() - self.assertIsInstance(tracer_provider.sampler, sampling.StaticSampler) - self.assertEqual( - tracer_provider.sampler._decision, sampling.Decision.DROP - ) + self.assertIsInstance(tracer_provider.sampler, StaticSampler) + self.assertEqual(tracer_provider.sampler._decision, Decision.DROP) tracer = tracer_provider.get_tracer(__name__) @@ -294,7 +296,7 @@ def test_ratio_sampler_with_env(self): # pylint: disable=protected-access reload(trace) tracer_provider = trace.TracerProvider() - self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) + self.assertIsInstance(tracer_provider.sampler, ParentBased) self.assertEqual(tracer_provider.sampler._root.rate, 0.25) @mock.patch.dict( @@ -453,9 +455,9 @@ def test_custom_ratio_sampler_with_env_multiple_entry_points( self.assertIsInstance(tracer_provider.sampler, CustomSampler) def verify_default_sampler(self, tracer_provider): - self.assertIsInstance(tracer_provider.sampler, sampling.ParentBased) + self.assertIsInstance(tracer_provider.sampler, ParentBased) # pylint: disable=protected-access - self.assertEqual(tracer_provider.sampler._root, sampling.ALWAYS_ON) + self.assertEqual(tracer_provider.sampler._root, ALWAYS_ON) class TestSpanCreation(unittest.TestCase): @@ -950,7 +952,7 @@ def test_sampling_attributes(self): "attr-in-both": "decision-attr", } tracer_provider = trace.TracerProvider( - sampling.StaticSampler(sampling.Decision.RECORD_AND_SAMPLE) + StaticSampler(Decision.RECORD_AND_SAMPLE) ) self.tracer = tracer_provider.get_tracer(__name__) From 708f1f9140005f6f2ed0405b99af75c7cadd6234 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 24 Oct 2022 15:58:22 -0700 Subject: [PATCH 18/19] renamed vars traces_sampler* for consistency --- .../src/opentelemetry/sdk/trace/sampling.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 5bb69279721..38a3338b02f 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -407,12 +407,12 @@ def __init__(self, rate: float): def _get_from_env_or_default() -> Sampler: - trace_sampler_name = os.getenv( + traces_sampler_name = os.getenv( OTEL_TRACES_SAMPLER, "parentbased_always_on" ).lower() - if trace_sampler_name in _KNOWN_SAMPLERS: - if trace_sampler_name in ("traceidratio", "parentbased_traceidratio"): + if traces_sampler_name in _KNOWN_SAMPLERS: + if traces_sampler_name in ("traceidratio", "parentbased_traceidratio"): try: rate = float(os.getenv(OTEL_TRACES_SAMPLER_ARG)) except ValueError: @@ -420,21 +420,21 @@ def _get_from_env_or_default() -> Sampler: "Could not convert TRACES_SAMPLER_ARG to float." ) rate = 1.0 - return _KNOWN_SAMPLERS[trace_sampler_name](rate) - return _KNOWN_SAMPLERS[trace_sampler_name] + return _KNOWN_SAMPLERS[traces_sampler_name](rate) + return _KNOWN_SAMPLERS[traces_sampler_name] try: - trace_sampler_factory = _import_sampler_factory(trace_sampler_name) + traces_sampler_factory = _import_sampler_factory(traces_sampler_name) sampler_arg = os.getenv(OTEL_TRACES_SAMPLER_ARG, "") - trace_sampler = trace_sampler_factory(sampler_arg) - if not isinstance(trace_sampler, Sampler): - message = f"Output of traces sampler factory, {trace_sampler_factory}, was not a Sampler object." + traces_sampler = traces_sampler_factory(sampler_arg) + if not isinstance(traces_sampler, Sampler): + message = f"Traces sampler factory, {traces_sampler_factory}, produced output, {traces_sampler}, which is not a Sampler object." _logger.warning(message) raise ValueError(message) - return trace_sampler + return traces_sampler except Exception as exc: # pylint: disable=broad-except _logger.warning( "Using default sampler. Failed to initialize custom sampler, %s: %s", - trace_sampler_name, + traces_sampler_name, exc, ) return _KNOWN_SAMPLERS["parentbased_always_on"] From a313046131e6e0e3ab54dc2c00045a6a03fbafaf Mon Sep 17 00:00:00 2001 From: jerevoss Date: Wed, 26 Oct 2022 14:01:42 -0700 Subject: [PATCH 19/19] retrigger checks