diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e082982a03a..7a7b9354dcb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,7 +10,7 @@ env: # Otherwise, set variable to the commit of your branch on # opentelemetry-python-contrib which is compatible with these Core repo # changes. - CONTRIB_REPO_SHA: e1d4eeb951694afebb1767b4ea8f593753d894e3 + CONTRIB_REPO_SHA: b37945bdeaf49822b240281d493d053995cc2b7b jobs: build: diff --git a/.gitignore b/.gitignore index 5378aadb363..d1687658908 100644 --- a/.gitignore +++ b/.gitignore @@ -16,7 +16,9 @@ var sdist develop-eggs .installed.cfg +pyvenv.cfg lib +share/ lib64 __pycache__ venv*/ diff --git a/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_metrics_exporter.py b/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_metrics_exporter.py index b48a4a5a33d..3b40b8d75ab 100644 --- a/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_metrics_exporter.py +++ b/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_metrics_exporter.py @@ -91,13 +91,13 @@ def test_get_collector_metric_type(self): def test_get_collector_point(self): aggregator = aggregate.SumAggregator() int_counter = self._meter.create_counter( - "testName", "testDescription", "unit", int, + "testNameIntCounter", "testDescription", "unit", int, ) float_counter = self._meter.create_counter( - "testName", "testDescription", "unit", float, + "testNameFloatCounter", "testDescription", "unit", float, ) valuerecorder = self._meter.create_valuerecorder( - "testName", "testDescription", "unit", float, + "testNameValueRecorder", "testDescription", "unit", float, ) result = metrics_exporter.get_collector_point( ExportRecord( @@ -168,7 +168,7 @@ def test_export(self): def test_translate_to_collector(self): test_metric = self._meter.create_counter( - "testname", "testdesc", "unit", int, self._labels.keys() + "testcollector", "testdesc", "unit", int, self._labels.keys() ) aggregator = aggregate.SumAggregator() aggregator.update(123) @@ -185,7 +185,9 @@ def test_translate_to_collector(self): ) self.assertEqual(len(output_metrics), 1) self.assertIsInstance(output_metrics[0], metrics_pb2.Metric) - self.assertEqual(output_metrics[0].metric_descriptor.name, "testname") + self.assertEqual( + output_metrics[0].metric_descriptor.name, "testcollector" + ) self.assertEqual( output_metrics[0].metric_descriptor.description, "testdesc" ) diff --git a/exporter/opentelemetry-exporter-otlp/CHANGELOG.md b/exporter/opentelemetry-exporter-otlp/CHANGELOG.md index 4528c215345..e3968d6825e 100644 --- a/exporter/opentelemetry-exporter-otlp/CHANGELOG.md +++ b/exporter/opentelemetry-exporter-otlp/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +## Version 0.16b1 + +Released 2020-11-26 + - Add meter reference to observers ([#1425](https://github.com/open-telemetry/opentelemetry-python/pull/1425)) diff --git a/exporter/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/exporter.py b/exporter/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/exporter.py index 54853f105cb..93a87b8cf76 100644 --- a/exporter/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/exporter.py +++ b/exporter/opentelemetry-exporter-otlp/src/opentelemetry/exporter/otlp/exporter.py @@ -208,11 +208,12 @@ def __init__( credentials is None and Configuration().EXPORTER_OTLP_CERTIFICATE is None ): - raise ValueError("No credentials set in secure mode.") - - credentials = credentials or _load_credential_from_file( - Configuration().EXPORTER_OTLP_CERTIFICATE - ) + # use the default location chosen by gRPC runtime + credentials = ssl_channel_credentials() + else: + credentials = credentials or _load_credential_from_file( + Configuration().EXPORTER_OTLP_CERTIFICATE + ) self._client = self._stub( secure_channel( endpoint, credentials, compression=compression_algorithm diff --git a/exporter/opentelemetry-exporter-otlp/tests/test_otlp_metric_exporter.py b/exporter/opentelemetry-exporter-otlp/tests/test_otlp_metric_exporter.py index 6d45e30d440..c8664f94fa7 100644 --- a/exporter/opentelemetry-exporter-otlp/tests/test_otlp_metric_exporter.py +++ b/exporter/opentelemetry-exporter-otlp/tests/test_otlp_metric_exporter.py @@ -90,9 +90,17 @@ def test_env_variables(self, mock_exporter_mixin): self.assertIsNotNone(kwargs["credentials"]) self.assertIsInstance(kwargs["credentials"], ChannelCredentials) - def test_no_credentials_error(self): - with self.assertRaises(ValueError): - OTLPMetricsExporter() + @patch("opentelemetry.exporter.otlp.exporter.ssl_channel_credentials") + @patch("opentelemetry.exporter.otlp.exporter.secure_channel") + @patch( + "opentelemetry.exporter.otlp.metrics_exporter.OTLPMetricsExporter._stub" + ) + # pylint: disable=unused-argument + def test_no_credentials_error( + self, mock_ssl_channel, mock_secure, mock_stub + ): + OTLPMetricsExporter(insecure=False) + self.assertTrue(mock_ssl_channel.called) @patch("opentelemetry.sdk.metrics.export.aggregate.time_ns") def test_translate_counter_export_record(self, mock_time_ns): diff --git a/exporter/opentelemetry-exporter-otlp/tests/test_otlp_trace_exporter.py b/exporter/opentelemetry-exporter-otlp/tests/test_otlp_trace_exporter.py index 4445873aaf6..b691e85849c 100644 --- a/exporter/opentelemetry-exporter-otlp/tests/test_otlp_trace_exporter.py +++ b/exporter/opentelemetry-exporter-otlp/tests/test_otlp_trace_exporter.py @@ -187,9 +187,15 @@ def test_env_variables(self, mock_exporter_mixin): self.assertIsNotNone(kwargs["credentials"]) self.assertIsInstance(kwargs["credentials"], ChannelCredentials) - def test_no_credentials_error(self): - with self.assertRaises(ValueError): - OTLPSpanExporter() + @patch("opentelemetry.exporter.otlp.exporter.ssl_channel_credentials") + @patch("opentelemetry.exporter.otlp.exporter.secure_channel") + @patch("opentelemetry.exporter.otlp.trace_exporter.OTLPSpanExporter._stub") + # pylint: disable=unused-argument + def test_no_credentials_error( + self, mock_ssl_channel, mock_secure, mock_stub + ): + OTLPSpanExporter(insecure=False) + self.assertTrue(mock_ssl_channel.called) @patch("opentelemetry.exporter.otlp.exporter.expo") @patch("opentelemetry.exporter.otlp.exporter.sleep") diff --git a/opentelemetry-api/src/opentelemetry/configuration/__init__.py b/opentelemetry-api/src/opentelemetry/configuration/__init__.py index bf641ca836d..5e3d3667dc1 100644 --- a/opentelemetry-api/src/opentelemetry/configuration/__init__.py +++ b/opentelemetry-api/src/opentelemetry/configuration/__init__.py @@ -93,14 +93,26 @@ to override this value instead of changing it. """ +import re from os import environ -from re import fullmatch -from typing import ClassVar, Dict, Optional, TypeVar, Union +from typing import ClassVar, Dict, List, Optional, Sequence, TypeVar, Union ConfigValue = Union[str, bool, int, float] _T = TypeVar("_T", ConfigValue, Optional[ConfigValue]) +class ExcludeList: + """Class to exclude certain paths (given as a list of regexes) from tracing requests""" + + def __init__(self, excluded_urls: Sequence[str]): + self._non_empty = len(excluded_urls) > 0 + if self._non_empty: + self._regex = re.compile("|".join(excluded_urls)) + + def url_disabled(self, url: str) -> bool: + return bool(self._non_empty and re.search(self._regex, url)) + + class Configuration: _instance = None # type: ClassVar[Optional[Configuration]] _config_map = {} # type: ClassVar[Dict[str, ConfigValue]] @@ -113,7 +125,7 @@ def __new__(cls) -> "Configuration": instance = super().__new__(cls) for key, value_str in environ.items(): - match = fullmatch(r"OTEL_(PYTHON_)?([A-Za-z_][\w_]*)", key) + match = re.fullmatch(r"OTEL_(PYTHON_)?([A-Za-z_][\w_]*)", key) if match is not None: @@ -167,3 +179,20 @@ def _reset(cls) -> None: if cls._instance: cls._instance._config_map.clear() # pylint: disable=protected-access cls._instance = None + + def _traced_request_attrs(self, instrumentation: str) -> List[str]: + """Returns list of traced request attributes for instrumentation.""" + key = "{}_TRACED_REQUEST_ATTRS".format(instrumentation.upper()) + value = self._config_map.get(key, "") + + request_attrs = ( + [attr.strip() for attr in str.split(value, ",")] if value else [] # type: ignore + ) + return request_attrs + + def _excluded_urls(self, instrumentation: str) -> ExcludeList: + key = "{}_EXCLUDED_URLS".format(instrumentation.upper()) + value = self._config_map.get(key, "") + + urls = str.split(value, ",") if value else [] # type: ignore + return ExcludeList(urls) diff --git a/opentelemetry-api/src/opentelemetry/trace/span.py b/opentelemetry-api/src/opentelemetry/trace/span.py index 2d0e34996c7..d41a0c3fbd4 100644 --- a/opentelemetry-api/src/opentelemetry/trace/span.py +++ b/opentelemetry-api/src/opentelemetry/trace/span.py @@ -77,7 +77,7 @@ def is_recording(self) -> bool: @abc.abstractmethod def set_status(self, status: Status) -> None: """Sets the Status of the Span. If used, this will override the default - Span status, which is OK. + Span status. """ @abc.abstractmethod diff --git a/opentelemetry-api/src/opentelemetry/util/__init__.py b/opentelemetry-api/src/opentelemetry/util/__init__.py index 5d98ba96bf6..58c297269e0 100644 --- a/opentelemetry-api/src/opentelemetry/util/__init__.py +++ b/opentelemetry-api/src/opentelemetry/util/__init__.py @@ -65,15 +65,3 @@ def _load_meter_provider(provider: str) -> "MeterProvider": def _load_trace_provider(provider: str) -> "TracerProvider": return cast("TracerProvider", _load_provider(provider)) - - -class ExcludeList: - """Class to exclude certain paths (given as a list of regexes) from tracing requests""" - - def __init__(self, excluded_urls: Sequence[str]): - self._non_empty = len(excluded_urls) > 0 - if self._non_empty: - self._regex = re.compile("|".join(excluded_urls)) - - def url_disabled(self, url: str) -> bool: - return bool(self._non_empty and re.search(self._regex, url)) diff --git a/opentelemetry-api/tests/configuration/test_configuration.py b/opentelemetry-api/tests/configuration/test_configuration.py index 608a96c96d1..de7bb16de6e 100644 --- a/opentelemetry-api/tests/configuration/test_configuration.py +++ b/opentelemetry-api/tests/configuration/test_configuration.py @@ -145,3 +145,32 @@ def test_float(self) -> None: self.assertEqual( Configuration().NON_FLOAT, "-12z3.123" ) # pylint: disable=no-member + + @patch.dict( + "os.environ", # type: ignore + { + "OTEL_PYTHON_WEBFRAMEWORK_TRACED_REQUEST_ATTRS": "content_type,keep_alive", + }, + ) + def test_traced_request_attrs(self) -> None: + cfg = Configuration() + request_attrs = cfg._traced_request_attrs("webframework") + self.assertEqual(len(request_attrs), 2) + self.assertIn("content_type", request_attrs) + self.assertIn("keep_alive", request_attrs) + self.assertNotIn("authorization", request_attrs) + + @patch.dict( + "os.environ", # type: ignore + { + "OTEL_PYTHON_WEBFRAMEWORK_EXCLUDED_URLS": "/healthzz,path,/issues/.*/view", + }, + ) + def test_excluded_urls(self) -> None: + cfg = Configuration() + excluded_urls = cfg._excluded_urls("webframework") + self.assertTrue(excluded_urls.url_disabled("/healthzz")) + self.assertTrue(excluded_urls.url_disabled("/path")) + self.assertTrue(excluded_urls.url_disabled("/issues/123/view")) + self.assertFalse(excluded_urls.url_disabled("/issues")) + self.assertFalse(excluded_urls.url_disabled("/hello")) diff --git a/opentelemetry-api/tests/util/test_exclude_list.py b/opentelemetry-api/tests/configuration/test_exclude_list.py similarity index 97% rename from opentelemetry-api/tests/util/test_exclude_list.py rename to opentelemetry-api/tests/configuration/test_exclude_list.py index da51478de3e..c7baf842ed7 100644 --- a/opentelemetry-api/tests/util/test_exclude_list.py +++ b/opentelemetry-api/tests/configuration/test_exclude_list.py @@ -13,9 +13,8 @@ # limitations under the License. import unittest -from unittest import mock -from opentelemetry.util import ExcludeList +from opentelemetry.configuration import ExcludeList class TestExcludeList(unittest.TestCase): diff --git a/opentelemetry-instrumentation/tests/test_metric.py b/opentelemetry-instrumentation/tests/test_metric.py index 8e676c737e2..c0bdcca15ad 100644 --- a/opentelemetry-instrumentation/tests/test_metric.py +++ b/opentelemetry-instrumentation/tests/test_metric.py @@ -72,7 +72,7 @@ def test_ctor(self): "measures the duration of the outbound HTTP request", ) - def test_ctor_types(self): + def test_ctor_type_client(self): meter = metrics_api.get_meter(__name__) recorder = HTTPMetricRecorder(meter, HTTPMetricType.CLIENT) self.assertEqual(recorder._http_type, HTTPMetricType.CLIENT) @@ -81,6 +81,8 @@ def test_ctor_types(self): ) self.assertIsNone(recorder._server_duration) + def test_ctor_type_server(self): + meter = metrics_api.get_meter(__name__) recorder = HTTPMetricRecorder(meter, HTTPMetricType.SERVER) self.assertEqual(recorder._http_type, HTTPMetricType.SERVER) self.assertTrue( @@ -88,6 +90,8 @@ def test_ctor_types(self): ) self.assertIsNone(recorder._client_duration) + def test_ctor_type_both(self): + meter = metrics_api.get_meter(__name__) recorder = HTTPMetricRecorder(meter, HTTPMetricType.BOTH) self.assertEqual(recorder._http_type, HTTPMetricType.BOTH) self.assertTrue( diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 5014e1c19b6..9f94dff63c0 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -4,6 +4,8 @@ - Add meter reference to observers ([#1425](https://github.com/open-telemetry/opentelemetry-python/pull/1425)) +- Add local/remote samplers to parent based sampler + ([#1440](https://github.com/open-telemetry/opentelemetry-python/pull/1440)) - Add `fields` to propagators ([#1374](https://github.com/open-telemetry/opentelemetry-python/pull/1374)) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index a09047e5231..ca0ec2f967c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -15,7 +15,7 @@ import atexit import logging import threading -from typing import Dict, Sequence, Tuple, Type, TypeVar +from typing import Dict, Sequence, Tuple, Type, TypeVar, Union from opentelemetry import metrics as metrics_api from opentelemetry.sdk.metrics.export import ( @@ -356,12 +356,23 @@ def __init__( ): self.instrumentation_info = instrumentation_info self.processor = Processor(source.stateful, source.resource) - self.metrics = set() - self.observers = set() - self.metrics_lock = threading.Lock() - self.observers_lock = threading.Lock() + self.instruments = {} + self.instruments_lock = threading.Lock() self.view_manager = ViewManager() + def _register_instrument( + self, instrument: Union[metrics_api.Metric, metrics_api.Observer] + ): + name = instrument.name.strip().lower() + with self.instruments_lock: + if name in self.instruments: + raise ValueError( + "Multiple instruments can't be registered by the same name: ({})".format( + name + ) + ) + self.instruments[name] = instrument + def collect(self) -> None: """Collects all the metrics created with this `Meter` for export. @@ -369,46 +380,41 @@ def collect(self) -> None: each aggregator belonging to the metrics that were created with this meter instance. """ - - self._collect_metrics() - self._collect_observers() - - def _collect_metrics(self) -> None: - for metric in self.metrics: - if not metric.enabled: - continue - to_remove = [] - with metric.bound_instruments_lock: - for ( - labels, - bound_instrument, - ) in metric.bound_instruments.items(): - for view_data in bound_instrument.view_datas: + with self.instruments_lock: + for instrument in self.instruments.values(): + if not instrument.enabled: + continue + if isinstance(instrument, metrics_api.Metric): + to_remove = [] + with instrument.bound_instruments_lock: + for ( + labels, + bound_instrument, + ) in instrument.bound_instruments.items(): + for view_data in bound_instrument.view_datas: + accumulation = Accumulation( + instrument, + view_data.labels, + view_data.aggregator, + ) + self.processor.process(accumulation) + + if bound_instrument.ref_count() == 0: + to_remove.append(labels) + + # Remove handles that were released + for labels in to_remove: + del instrument.bound_instruments[labels] + elif isinstance(instrument, metrics_api.Observer): + if not instrument.run(): + continue + + for labels, aggregator in instrument.aggregators.items(): accumulation = Accumulation( - metric, view_data.labels, view_data.aggregator + instrument, labels, aggregator ) self.processor.process(accumulation) - if bound_instrument.ref_count() == 0: - to_remove.append(labels) - - # Remove handles that were released - for labels in to_remove: - del metric.bound_instruments[labels] - - def _collect_observers(self) -> None: - with self.observers_lock: - for observer in self.observers: - if not observer.enabled: - continue - - if not observer.run(): - continue - - for labels, aggregator in observer.aggregators.items(): - accumulation = Accumulation(observer, labels, aggregator) - self.processor.process(accumulation) - def record_batch( self, labels: Dict[str, str], @@ -432,8 +438,7 @@ def create_counter( counter = Counter( name, description, unit, value_type, self, enabled=enabled ) - with self.metrics_lock: - self.metrics.add(counter) + self._register_instrument(counter) return counter def create_updowncounter( @@ -448,8 +453,7 @@ def create_updowncounter( counter = UpDownCounter( name, description, unit, value_type, self, enabled=enabled ) - with self.metrics_lock: - self.metrics.add(counter) + self._register_instrument(counter) return counter def create_valuerecorder( @@ -464,8 +468,7 @@ def create_valuerecorder( recorder = ValueRecorder( name, description, unit, value_type, self, enabled=enabled ) - with self.metrics_lock: - self.metrics.add(recorder) + self._register_instrument(recorder) return recorder def register_sumobserver( @@ -488,8 +491,7 @@ def register_sumobserver( label_keys, enabled, ) - with self.observers_lock: - self.observers.add(ob) + self._register_instrument(ob) return ob def register_updownsumobserver( @@ -512,8 +514,7 @@ def register_updownsumobserver( label_keys, enabled, ) - with self.observers_lock: - self.observers.add(ob) + self._register_instrument(ob) return ob def register_valueobserver( @@ -536,13 +537,13 @@ def register_valueobserver( label_keys, enabled, ) - with self.observers_lock: - self.observers.add(ob) + self._register_instrument(ob) return ob def unregister_observer(self, observer: metrics_api.Observer) -> None: - with self.observers_lock: - self.observers.remove(observer) + name = observer.name.strip().lower() + with self.instruments_lock: + self.instruments.pop(name) def register_view(self, view): self.view_manager.register_view(view) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py index 82d2cebaa51..d1e02b3109d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py @@ -27,7 +27,7 @@ A `TraceIdRatioBased` sampler makes a random sampling result based on the sampling probability given. -If the span being sampled has a parent, `ParentBased` will respect the parent span's sampling result. Otherwise, it returns the sampling result from the given delegate sampler. +If the span being sampled has a parent, `ParentBased` will respect the parent delegate sampler. Otherwise, it returns the sampling result from the given root sampler. Currently, sampling results are always made during the creation of the span. However, this might not always be the case in the future (see `OTEP #115 `_). @@ -160,6 +160,13 @@ def get_description(self) -> str: return "AlwaysOnSampler" +ALWAYS_OFF = StaticSampler(Decision.DROP) +"""Sampler that never samples spans, regardless of the parent span's sampling decision.""" + +ALWAYS_ON = StaticSampler(Decision.RECORD_AND_SAMPLE) +"""Sampler that always samples spans, regardless of the parent span's sampling decision.""" + + class TraceIdRatioBased(Sampler): """ Sampler that makes sampling decisions probabalistically based on `rate`, @@ -218,16 +225,33 @@ def get_description(self) -> str: class ParentBased(Sampler): """ - If a parent is set, follows the same sampling decision as the parent. - Otherwise, uses the delegate provided at initialization to make a + If a parent is set, applies the respective delegate sampler. + Otherwise, uses the root provided at initialization to make a decision. Args: - delegate: The delegate sampler to use if parent is not set. + root: Sampler called for spans with no parent (root spans). + remote_parent_sampled: Sampler called for a remote sampled parent. + remote_parent_not_sampled: Sampler called for a remote parent that is + not sampled. + local_parent_sampled: Sampler called for a local sampled parent. + local_parent_not_sampled: Sampler called for a local parent that is + not sampled. """ - def __init__(self, delegate: Sampler): - self._delegate = delegate + def __init__( + self, + root: Sampler, + remote_parent_sampled: Sampler = ALWAYS_ON, + remote_parent_not_sampled: Sampler = ALWAYS_OFF, + local_parent_sampled: Sampler = ALWAYS_ON, + local_parent_not_sampled: Sampler = ALWAYS_OFF, + ): + self._root = root + self._remote_parent_sampled = remote_parent_sampled + self._remote_parent_not_sampled = remote_parent_not_sampled + self._local_parent_sampled = local_parent_sampled + self._local_parent_not_sampled = local_parent_not_sampled def should_sample( self, @@ -241,15 +265,22 @@ def should_sample( parent_span_context = get_current_span( parent_context ).get_span_context() - # respect the sampling flag of the parent if present + # default to the root sampler + sampler = self._root + # respect the sampling and remote flag of the parent if present if parent_span_context is not None and parent_span_context.is_valid: - decision = Decision.RECORD_AND_SAMPLE - if not parent_span_context.trace_flags.sampled: - decision = Decision.DROP - attributes = None - return SamplingResult(decision, attributes, trace_state) - - return self._delegate.should_sample( + if parent_span_context.is_remote: + if parent_span_context.trace_flags.sampled: + sampler = self._remote_parent_sampled + else: + sampler = self._remote_parent_not_sampled + else: + if parent_span_context.trace_flags.sampled: + sampler = self._local_parent_sampled + else: + sampler = self._local_parent_not_sampled + + return sampler.should_sample( parent_context=parent_context, trace_id=trace_id, name=name, @@ -259,14 +290,17 @@ def should_sample( ) def get_description(self): - return "ParentBased{{{}}}".format(self._delegate.get_description()) - - -ALWAYS_OFF = StaticSampler(Decision.DROP) -"""Sampler that never samples spans, regardless of the parent span's sampling decision.""" + return ( + "ParentBased{{root:{},remoteParentSampled:{},remoteParentNotSampled:{}," + "localParentSampled:{},localParentNotSampled:{}}}".format( + self._root.get_description(), + self._remote_parent_sampled.get_description(), + self._remote_parent_not_sampled.get_description(), + self._local_parent_sampled.get_description(), + self._local_parent_not_sampled.get_description(), + ) + ) -ALWAYS_ON = StaticSampler(Decision.RECORD_AND_SAMPLE) -"""Sampler that always samples spans, regardless of the parent span's sampling decision.""" DEFAULT_OFF = ParentBased(ALWAYS_OFF) """Sampler that respects its parent span's sampling decision, but otherwise never samples.""" diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 1697d8e6c85..32c22c8c6bb 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -118,11 +118,9 @@ def callback(observer): self.assertIsInstance(observer, metrics_api.Observer) observer.observe(45, {}) - observer = metrics.ValueObserver( + meter.register_valueobserver( callback, "name", "desc", "unit", int, (), True ) - - meter.observers.add(observer) meter.collect() self.assertTrue(processor_mock.process.called) @@ -164,6 +162,23 @@ def test_create_counter(self): self.assertIs(meter_provider.resource, resource) self.assertEqual(counter.meter, meter) + def test_instrument_same_name_error(self): + resource = Mock(spec=resources.Resource) + meter_provider = metrics.MeterProvider(resource=resource) + meter = meter_provider.get_meter(__name__) + counter = meter.create_counter("name", "desc", "unit", int,) + self.assertIsInstance(counter, metrics.Counter) + self.assertEqual(counter.value_type, int) + self.assertEqual(counter.name, "name") + self.assertIs(meter_provider.resource, resource) + self.assertEqual(counter.meter, meter) + with self.assertRaises(ValueError) as ctx: + _ = meter.create_counter("naME", "desc", "unit", int,) + self.assertTrue( + "Multiple instruments can't be registered by the same name: (name)" + in str(ctx.exception) + ) + def test_create_updowncounter(self): meter = metrics.MeterProvider().get_meter(__name__) updowncounter = meter.create_updowncounter( @@ -193,7 +208,7 @@ def test_register_sumobserver(self): ) self.assertIsInstance(observer, metrics.SumObserver) - self.assertEqual(len(meter.observers), 1) + self.assertEqual(len(meter.instruments), 1) self.assertEqual(observer.callback, callback) self.assertEqual(observer.name, "name") @@ -213,7 +228,7 @@ def test_register_updownsumobserver(self): ) self.assertIsInstance(observer, metrics.UpDownSumObserver) - self.assertEqual(len(meter.observers), 1) + self.assertEqual(len(meter.instruments), 1) self.assertEqual(observer.callback, callback) self.assertEqual(observer.name, "name") @@ -233,7 +248,7 @@ def test_register_valueobserver(self): ) self.assertIsInstance(observer, metrics.ValueObserver) - self.assertEqual(len(meter.observers), 1) + self.assertEqual(len(meter.instruments), 1) self.assertEqual(observer.callback, callback) self.assertEqual(observer.name, "name") @@ -253,7 +268,27 @@ def test_unregister_observer(self): ) meter.unregister_observer(observer) - self.assertEqual(len(meter.observers), 0) + self.assertEqual(len(meter.instruments), 0) + + def test_unregister_and_reregister_observer(self): + meter = metrics.MeterProvider().get_meter(__name__) + + callback = Mock() + + observer = meter.register_valueobserver( + callback, + "nameCaSEinSENsitive", + "desc", + "unit", + int, + metrics.ValueObserver, + ) + + meter.unregister_observer(observer) + self.assertEqual(len(meter.instruments), 0) + observer = meter.register_valueobserver( + callback, "name", "desc", "unit", int, metrics.ValueObserver + ) class TestMetric(unittest.TestCase): diff --git a/opentelemetry-sdk/tests/trace/test_sampling.py b/opentelemetry-sdk/tests/trace/test_sampling.py index f6d77ab04ba..0d026de01df 100644 --- a/opentelemetry-sdk/tests/trace/test_sampling.py +++ b/opentelemetry-sdk/tests/trace/test_sampling.py @@ -311,13 +311,15 @@ def test_probability_sampler_limits(self): almost_almost_always_on.bound, 0xFFFFFFFFFFFFFFFF, ) + # pylint:disable=too-many-statements def exec_parent_based(self, parent_sampling_context): trace_state = trace.TraceState({"key": "value"}) sampler = sampling.ParentBased(sampling.ALWAYS_ON) + # Check that the sampling decision matches the parent context if given with parent_sampling_context( self._create_parent_span(trace_flags=TO_DEFAULT) ) as context: - # Check that the sampling decision matches the parent context if given + # local, not sampled not_sampled_result = sampler.should_sample( context, 0x7FFFFFFFFFFFFFFF, @@ -329,11 +331,101 @@ def exec_parent_based(self, parent_sampling_context): self.assertEqual(not_sampled_result.attributes, {}) self.assertEqual(not_sampled_result.trace_state, trace_state) + with parent_sampling_context( + self._create_parent_span(trace_flags=TO_DEFAULT) + ) as context: + sampler = sampling.ParentBased( + root=sampling.ALWAYS_OFF, + local_parent_not_sampled=sampling.ALWAYS_ON, + ) + # local, not sampled -> opposite sampler + sampled_result = sampler.should_sample( + context, + 0x7FFFFFFFFFFFFFFF, + "unsampled parent, sampling on", + attributes={"sampled": "false"}, + trace_state=trace_state, + ) + self.assertTrue(sampled_result.decision.is_sampled()) + self.assertEqual(sampled_result.attributes, {"sampled": "false"}) + self.assertEqual(sampled_result.trace_state, trace_state) + + with parent_sampling_context( + self._create_parent_span(trace_flags=TO_SAMPLED) + ) as context: + sampler = sampling.ParentBased(sampling.ALWAYS_OFF) + # local, sampled + sampled_result = sampler.should_sample( + context, + 0x8000000000000000, + "sampled parent, sampling off", + attributes={"sampled": "true"}, + trace_state=trace_state, + ) + self.assertTrue(sampled_result.decision.is_sampled()) + self.assertEqual(sampled_result.attributes, {"sampled": "true"}) + self.assertEqual(sampled_result.trace_state, trace_state) + with parent_sampling_context( self._create_parent_span(trace_flags=TO_SAMPLED) ) as context: - sampler2 = sampling.ParentBased(sampling.ALWAYS_OFF) - sampled_result = sampler2.should_sample( + sampler = sampling.ParentBased( + root=sampling.ALWAYS_ON, + local_parent_sampled=sampling.ALWAYS_OFF, + ) + # local, sampled -> opposite sampler + not_sampled_result = sampler.should_sample( + context, + 0x7FFFFFFFFFFFFFFF, + "unsampled parent, sampling on", + attributes={"sampled": "false"}, + trace_state=trace_state, + ) + self.assertFalse(not_sampled_result.decision.is_sampled()) + self.assertEqual(not_sampled_result.attributes, {}) + self.assertEqual(not_sampled_result.trace_state, trace_state) + + with parent_sampling_context( + self._create_parent_span(trace_flags=TO_DEFAULT, is_remote=True) + ) as context: + sampler = sampling.ParentBased(sampling.ALWAYS_ON) + # remote, not sampled + not_sampled_result = sampler.should_sample( + context, + 0x7FFFFFFFFFFFFFFF, + "unsampled parent, sampling on", + attributes={"sampled": "false"}, + trace_state=trace_state, + ) + self.assertFalse(not_sampled_result.decision.is_sampled()) + self.assertEqual(not_sampled_result.attributes, {}) + self.assertEqual(not_sampled_result.trace_state, trace_state) + + with parent_sampling_context( + self._create_parent_span(trace_flags=TO_DEFAULT, is_remote=True) + ) as context: + sampler = sampling.ParentBased( + root=sampling.ALWAYS_OFF, + remote_parent_not_sampled=sampling.ALWAYS_ON, + ) + # remote, not sampled -> opposite sampler + sampled_result = sampler.should_sample( + context, + 0x7FFFFFFFFFFFFFFF, + "unsampled parent, sampling on", + attributes={"sampled": "false"}, + trace_state=trace_state, + ) + self.assertTrue(sampled_result.decision.is_sampled()) + self.assertEqual(sampled_result.attributes, {"sampled": "false"}) + self.assertEqual(sampled_result.trace_state, trace_state) + + with parent_sampling_context( + self._create_parent_span(trace_flags=TO_SAMPLED, is_remote=True) + ) as context: + sampler = sampling.ParentBased(sampling.ALWAYS_OFF) + # remote, sampled + sampled_result = sampler.should_sample( context, 0x8000000000000000, "sampled parent, sampling off", @@ -344,10 +436,29 @@ def exec_parent_based(self, parent_sampling_context): self.assertEqual(sampled_result.attributes, {"sampled": "true"}) self.assertEqual(sampled_result.trace_state, trace_state) - # for root span follow decision of delegate sampler + with parent_sampling_context( + self._create_parent_span(trace_flags=TO_SAMPLED, is_remote=True) + ) as context: + sampler = sampling.ParentBased( + root=sampling.ALWAYS_ON, + remote_parent_sampled=sampling.ALWAYS_OFF, + ) + # remote, sampled -> opposite sampler + not_sampled_result = sampler.should_sample( + context, + 0x7FFFFFFFFFFFFFFF, + "unsampled parent, sampling on", + attributes={"sampled": "false"}, + trace_state=trace_state, + ) + self.assertFalse(not_sampled_result.decision.is_sampled()) + self.assertEqual(not_sampled_result.attributes, {}) + self.assertEqual(not_sampled_result.trace_state, trace_state) + + # for root span follow decision of root sampler with parent_sampling_context(trace.INVALID_SPAN) as context: - sampler3 = sampling.ParentBased(sampling.ALWAYS_OFF) - not_sampled_result = sampler3.should_sample( + sampler = sampling.ParentBased(sampling.ALWAYS_OFF) + not_sampled_result = sampler.should_sample( context, 0x8000000000000000, "parent, sampling off", @@ -359,8 +470,8 @@ def exec_parent_based(self, parent_sampling_context): self.assertEqual(not_sampled_result.trace_state, trace_state) with parent_sampling_context(trace.INVALID_SPAN) as context: - sampler4 = sampling.ParentBased(sampling.ALWAYS_ON) - sampled_result = sampler4.should_sample( + sampler = sampling.ParentBased(sampling.ALWAYS_ON) + sampled_result = sampler.should_sample( context, 0x8000000000000000, "no parent, sampling on",