From cd63eb6e52c6f66c9170a2928d2b4dc8918a41de Mon Sep 17 00:00:00 2001 From: "(Eliseo) Nathaniel Ruiz Nowell" Date: Mon, 14 Dec 2020 16:12:51 +0000 Subject: [PATCH 1/4] Fix Benchmarks grouping name (#1473) --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4c7dbc0478b..23c78227d6b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -74,7 +74,7 @@ jobs: if: matrix.package == 'core' uses: rhysd/github-action-benchmark@v1 with: - name: OpenTelemetry Python Benchmarks - Python ${{ env[matrix.python-version ]}} - ${{ matrix.package-group }} + name: OpenTelemetry Python Benchmarks - Python ${{ env[matrix.python-version ]}} - ${{ matrix.package }} tool: pytest output-file-path: output.json github-token: ${{ secrets.GITHUB_TOKEN }} From 47d6e95057c3a44faaea8562e2875e6d52f3fdab Mon Sep 17 00:00:00 2001 From: "(Eliseo) Nathaniel Ruiz Nowell" Date: Mon, 14 Dec 2020 21:28:12 +0000 Subject: [PATCH 2/4] Do not try to comment on PR after benchmarks (#1478) --- .github/workflows/test.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 23c78227d6b..672e14e1e7d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -80,7 +80,6 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} # Alert with a commit comment on possible performance regression alert-threshold: 200% - comment-always: true fail-on-alert: true # Make a commit on `gh-pages` with benchmarks from previous step auto-push: ${{ github.ref == 'refs/heads/master' }} From 33fa7b99274d1a7d7d7cb52fb273b18cbb5107a0 Mon Sep 17 00:00:00 2001 From: "(Eliseo) Nathaniel Ruiz Nowell" Date: Mon, 14 Dec 2020 22:37:44 +0000 Subject: [PATCH 3/4] Remove unnecessary contrib pkgs from docs install (#1470) --- docs-requirements.txt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs-requirements.txt b/docs-requirements.txt index 64fd71c5a99..2ce3b654c84 100644 --- a/docs-requirements.txt +++ b/docs-requirements.txt @@ -6,13 +6,7 @@ sphinx-autodoc-typehints~=1.10.2 # doesn't work for pkg_resources. ./opentelemetry-api ./opentelemetry-sdk --e "git+https://github.com/open-telemetry/opentelemetry-python-contrib.git#egg=opentelemetry-exporter-datadog&subdirectory=exporter/opentelemetry-exporter-datadog" --e "git+https://github.com/open-telemetry/opentelemetry-python-contrib.git#egg=opentelemetry-instrumentation-grpc&subdirectory=instrumentation/opentelemetry-instrumentation-grpc" ./opentelemetry-instrumentation --e "git+https://github.com/open-telemetry/opentelemetry-python-contrib.git#egg=opentelemetry-instrumentation-requests&subdirectory=instrumentation/opentelemetry-instrumentation-requests" --e "git+https://github.com/open-telemetry/opentelemetry-python-contrib.git#egg=opentelemetry-instrumentation-wsgi&subdirectory=instrumentation/opentelemetry-instrumentation-wsgi" --e "git+https://github.com/open-telemetry/opentelemetry-python-contrib.git#egg=opentelemetry-instrumentation-django&subdirectory=instrumentation/opentelemetry-instrumentation-django" --e "git+https://github.com/open-telemetry/opentelemetry-python-contrib.git#egg=opentelemetry-instrumentation-flask&subdirectory=instrumentation/opentelemetry-instrumentation-flask" # Required by ext packages asgiref~=3.0 From 001163739d7cc09a26592b688f9880da02baf208 Mon Sep 17 00:00:00 2001 From: alrex Date: Mon, 14 Dec 2020 15:03:25 -0800 Subject: [PATCH 4/4] Remove SDK dependency from auto-instrumentation (#1420) --- CHANGELOG.md | 4 ++ opentelemetry-instrumentation/setup.cfg | 1 - .../auto_instrumentation/sitecustomize.py | 34 +++++++++--- .../instrumentation/configurator.py | 53 +++++++++++++++++++ opentelemetry-sdk/setup.cfg | 3 ++ .../sdk/configuration/__init__.py | 42 ++++++++------- .../tests/configuration/test_configurator.py | 28 +++++----- tox.ini | 7 +-- 8 files changed, 129 insertions(+), 43 deletions(-) create mode 100644 opentelemetry-instrumentation/src/opentelemetry/instrumentation/configurator.py rename opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py => opentelemetry-sdk/src/opentelemetry/sdk/configuration/__init__.py (82%) rename opentelemetry-instrumentation/tests/test_auto_tracing.py => opentelemetry-sdk/tests/configuration/test_configurator.py (83%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73ec8a070c7..078f02791f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1449](https://github.com/open-telemetry/opentelemetry-python/pull/1449)) - Added support for Jaeger propagator ([#1219](https://github.com/open-telemetry/opentelemetry-python/pull/1219)) +- Remove dependency on SDK from `opentelemetry-instrumentation` package. The + `opentelemetry-sdk` package now registers an entrypoint `opentelemetry_configurator` + to allow `opentelemetry-instrument` to load the configuration for the SDK + ([#1420](https://github.com/open-telemetry/opentelemetry-python/pull/1420)) ## [0.16b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v0.16b1) - 2020-11-26 ### Added diff --git a/opentelemetry-instrumentation/setup.cfg b/opentelemetry-instrumentation/setup.cfg index e38e80d1bef..265cdec80b2 100644 --- a/opentelemetry-instrumentation/setup.cfg +++ b/opentelemetry-instrumentation/setup.cfg @@ -43,7 +43,6 @@ zip_safe = False include_package_data = True install_requires = opentelemetry-api == 0.17.dev0 - opentelemetry-sdk == 0.17.dev0 wrapt >= 1.0.0, < 2.0.0 [options.packages.find] diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py index c56d86c7f1e..2a51d0b8034 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py @@ -18,14 +18,15 @@ from pkg_resources import iter_entry_points -from opentelemetry.instrumentation.auto_instrumentation.components import ( - initialize_components, -) - logger = getLogger(__file__) -def auto_instrument(): +def _load_distros(): + # will be implemented in a subsequent PR + pass + + +def _load_instrumentors(): for entry_point in iter_entry_points("opentelemetry_instrumentor"): try: entry_point.load()().instrument() # type: ignore @@ -35,10 +36,29 @@ def auto_instrument(): raise exc +def _load_configurators(): + configured = None + for entry_point in iter_entry_points("opentelemetry_configurator"): + if configured is not None: + logger.warning( + "Configuration of %s not loaded, %s already loaded", + entry_point.name, + configured, + ) + continue + try: + entry_point.load()().configure() # type: ignore + configured = entry_point.name + except Exception as exc: # pylint: disable=broad-except + logger.exception("Configuration of %s failed", entry_point.name) + raise exc + + def initialize(): try: - initialize_components() - auto_instrument() + _load_distros() + _load_configurators() + _load_instrumentors() except Exception: # pylint: disable=broad-except logger.exception("Failed to auto initialize opentelemetry") diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/configurator.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/configurator.py new file mode 100644 index 00000000000..3efa71e89e9 --- /dev/null +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/configurator.py @@ -0,0 +1,53 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# type: ignore + +""" +OpenTelemetry Base Configurator +""" + +from abc import ABC, abstractmethod +from logging import getLogger + +_LOG = getLogger(__name__) + + +class BaseConfigurator(ABC): + """An ABC for configurators + + Configurators are used to configure + SDKs (i.e. TracerProvider, MeterProvider, Processors...) + to reduce the amount of manual configuration required. + """ + + _instance = None + _is_instrumented = False + + def __new__(cls, *args, **kwargs): + + if cls._instance is None: + cls._instance = object.__new__(cls, *args, **kwargs) + + return cls._instance + + @abstractmethod + def _configure(self, **kwargs): + """Configure the SDK""" + + def configure(self, **kwargs): + """Configure the SDK""" + self._configure(**kwargs) + + +__all__ = ["BaseConfigurator"] diff --git a/opentelemetry-sdk/setup.cfg b/opentelemetry-sdk/setup.cfg index 94dfbd18bc8..765d3891ec8 100644 --- a/opentelemetry-sdk/setup.cfg +++ b/opentelemetry-sdk/setup.cfg @@ -43,6 +43,7 @@ zip_safe = False include_package_data = True install_requires = opentelemetry-api == 0.17.dev0 + opentelemetry-instrumentation == 0.17.dev0 [options.packages.find] where = src @@ -57,6 +58,8 @@ opentelemetry_propagator = opentelemetry_exporter = console_span = opentelemetry.sdk.trace.export:ConsoleSpanExporter console_metrics = opentelemetry.sdk.metrics.export:ConsoleMetricsExporter +opentelemetry_configurator = + sdk_configurator = opentelemetry.sdk.configuration:Configurator [options.extras_require] test = diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py b/opentelemetry-sdk/src/opentelemetry/sdk/configuration/__init__.py similarity index 82% rename from opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py rename to opentelemetry-sdk/src/opentelemetry/sdk/configuration/__init__.py index 8b4ef4f3e8b..477aa5cf62e 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/configuration/__init__.py @@ -19,6 +19,7 @@ from opentelemetry import trace from opentelemetry.configuration import Configuration +from opentelemetry.instrumentation.configurator import BaseConfigurator from opentelemetry.sdk.metrics.export import MetricsExporter from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.trace import TracerProvider @@ -38,15 +39,15 @@ _DEFAULT_IDS_GENERATOR = RANDOM_IDS_GENERATOR -def get_ids_generator() -> str: +def _get_ids_generator() -> str: return Configuration().IDS_GENERATOR or _DEFAULT_IDS_GENERATOR -def get_service_name() -> str: +def _get_service_name() -> str: return Configuration().SERVICE_NAME or "" -def get_exporter_names() -> Sequence[str]: +def _get_exporter_names() -> Sequence[str]: exporter = Configuration().EXPORTER or _DEFAULT_EXPORTER if exporter.lower().strip() == "none": return [] @@ -62,10 +63,10 @@ def get_exporter_names() -> Sequence[str]: return names -def init_tracing( +def _init_tracing( exporters: Sequence[SpanExporter], ids_generator: trace.IdsGenerator ): - service_name = get_service_name() + service_name = _get_service_name() provider = TracerProvider( resource=Resource.create({"service.name": service_name}), ids_generator=ids_generator(), @@ -85,12 +86,12 @@ def init_tracing( ) -def init_metrics(exporters: Sequence[MetricsExporter]): +def _init_metrics(exporters: Sequence[MetricsExporter]): if exporters: logger.warning("automatic metric initialization is not supported yet.") -def import_tracer_provider_config_components( +def _import_tracer_provider_config_components( selected_components, entry_point_name ) -> Sequence[Tuple[str, object]]: component_entry_points = { @@ -112,7 +113,7 @@ def import_tracer_provider_config_components( return component_impls -def import_exporters( +def _import_exporters( exporter_names: Sequence[str], ) -> Tuple[Sequence[SpanExporter], Sequence[MetricsExporter]]: trace_exporters, metric_exporters = {}, {} @@ -120,7 +121,7 @@ def import_exporters( for ( exporter_name, exporter_impl, - ) in import_tracer_provider_config_components( + ) in _import_tracer_provider_config_components( exporter_names, "opentelemetry_exporter" ): if issubclass(exporter_impl, SpanExporter): @@ -136,11 +137,11 @@ def import_exporters( return trace_exporters, metric_exporters -def import_ids_generator(ids_generator_name: str) -> trace.IdsGenerator: +def _import_ids_generator(ids_generator_name: str) -> trace.IdsGenerator: # pylint: disable=unbalanced-tuple-unpacking [ (ids_generator_name, ids_generator_impl) - ] = import_tracer_provider_config_components( + ] = _import_tracer_provider_config_components( [ids_generator_name.strip()], "opentelemetry_ids_generator" ) @@ -150,14 +151,19 @@ def import_ids_generator(ids_generator_name: str) -> trace.IdsGenerator: raise RuntimeError("{0} is not an IdsGenerator".format(ids_generator_name)) -def initialize_components(): - exporter_names = get_exporter_names() - trace_exporters, metric_exporters = import_exporters(exporter_names) - ids_generator_name = get_ids_generator() - ids_generator = import_ids_generator(ids_generator_name) - init_tracing(trace_exporters, ids_generator) +def _initialize_components(): + exporter_names = _get_exporter_names() + trace_exporters, metric_exporters = _import_exporters(exporter_names) + ids_generator_name = _get_ids_generator() + ids_generator = _import_ids_generator(ids_generator_name) + _init_tracing(trace_exporters, ids_generator) # We don't support automatic initialization for metric yet but have added # some boilerplate in order to make sure current implementation does not # lock us out of supporting metrics later without major surgery. - init_metrics(metric_exporters) + _init_metrics(metric_exporters) + + +class Configurator(BaseConfigurator): + def _configure(self, **kwargs): + _initialize_components() diff --git a/opentelemetry-instrumentation/tests/test_auto_tracing.py b/opentelemetry-sdk/tests/configuration/test_configurator.py similarity index 83% rename from opentelemetry-instrumentation/tests/test_auto_tracing.py rename to opentelemetry-sdk/tests/configuration/test_configurator.py index 9ffe421a25d..c8851321f1e 100644 --- a/opentelemetry-instrumentation/tests/test_auto_tracing.py +++ b/opentelemetry-sdk/tests/configuration/test_configurator.py @@ -18,7 +18,11 @@ from unittest.mock import patch from opentelemetry.configuration import Configuration -from opentelemetry.instrumentation.auto_instrumentation import components +from opentelemetry.sdk.configuration import ( + _get_ids_generator, + _import_ids_generator, + _init_tracing, +) from opentelemetry.sdk.resources import Resource from opentelemetry.trace.ids_generator import RandomIdsGenerator @@ -71,11 +75,10 @@ class TestTraceInit(TestCase): def setUp(self): super() self.get_provider_patcher = patch( - "opentelemetry.instrumentation.auto_instrumentation.components.TracerProvider", - Provider, + "opentelemetry.sdk.configuration.TracerProvider", Provider, ) self.get_processor_patcher = patch( - "opentelemetry.instrumentation.auto_instrumentation.components.BatchExportSpanProcessor", + "opentelemetry.sdk.configuration.BatchExportSpanProcessor", Processor, ) self.set_provider_patcher = patch( @@ -96,7 +99,7 @@ def tearDown(self): def test_trace_init_default(self): environ["OTEL_SERVICE_NAME"] = "my-test-service" Configuration._reset() - components.init_tracing({"zipkin": Exporter}, RandomIdsGenerator) + _init_tracing({"zipkin": Exporter}, RandomIdsGenerator) self.assertEqual(self.set_provider_mock.call_count, 1) provider = self.set_provider_mock.call_args[0][0] @@ -111,7 +114,7 @@ def test_trace_init_default(self): def test_trace_init_otlp(self): environ["OTEL_SERVICE_NAME"] = "my-otlp-test-service" Configuration._reset() - components.init_tracing({"otlp": OTLPExporter}, RandomIdsGenerator) + _init_tracing({"otlp": OTLPExporter}, RandomIdsGenerator) self.assertEqual(self.set_provider_mock.call_count, 1) provider = self.set_provider_mock.call_args[0][0] @@ -128,12 +131,9 @@ def test_trace_init_otlp(self): @patch.dict(environ, {"OTEL_IDS_GENERATOR": "custom_ids_generator"}) @patch( - "opentelemetry.instrumentation.auto_instrumentation.components.trace.IdsGenerator", - new=IdsGenerator, - ) - @patch( - "opentelemetry.instrumentation.auto_instrumentation.components.iter_entry_points" + "opentelemetry.sdk.configuration.trace.IdsGenerator", new=IdsGenerator, ) + @patch("opentelemetry.sdk.configuration.iter_entry_points") def test_trace_init_custom_ids_generator(self, mock_iter_entry_points): mock_iter_entry_points.configure_mock( return_value=[ @@ -141,8 +141,8 @@ def test_trace_init_custom_ids_generator(self, mock_iter_entry_points): ] ) Configuration._reset() - ids_generator_name = components.get_ids_generator() - ids_generator = components.import_ids_generator(ids_generator_name) - components.init_tracing({}, ids_generator) + ids_generator_name = _get_ids_generator() + ids_generator = _import_ids_generator(ids_generator_name) + _init_tracing({}, ids_generator) provider = self.set_provider_mock.call_args[0][0] self.assertIsInstance(provider.ids_generator, CustomIdsGenerator) diff --git a/tox.ini b/tox.ini index c5044993b81..9a7c8ce3f85 100644 --- a/tox.ini +++ b/tox.ini @@ -84,7 +84,7 @@ commands_pre = py3{5,6,7,8,9}: python -m pip install -U pip setuptools wheel ; Install common packages for all the tests. These are not needed in all the ; cases but it saves a lot of boilerplate in this file. - test: pip install {toxinidir}/opentelemetry-api {toxinidir}/opentelemetry-sdk {toxinidir}/tests/util + test: pip install {toxinidir}/opentelemetry-api {toxinidir}/opentelemetry-instrumentation {toxinidir}/opentelemetry-sdk {toxinidir}/tests/util test-core-proto: pip install {toxinidir}/opentelemetry-proto instrumentation: pip install {toxinidir}/opentelemetry-instrumentation @@ -141,8 +141,8 @@ deps = commands_pre = python -m pip install -e {toxinidir}/opentelemetry-api[test] - python -m pip install -e {toxinidir}/opentelemetry-sdk[test] python -m pip install -e {toxinidir}/opentelemetry-instrumentation[test] + python -m pip install -e {toxinidir}/opentelemetry-sdk[test] python -m pip install -e {toxinidir}/opentelemetry-proto[test] python -m pip install -e {toxinidir}/tests/util[test] python -m pip install -e {toxinidir}/instrumentation/opentelemetry-instrumentation-opentracing-shim[test] @@ -176,8 +176,8 @@ deps = commands_pre = pip install -e {toxinidir}/opentelemetry-api \ - -e {toxinidir}/opentelemetry-sdk \ -e {toxinidir}/opentelemetry-instrumentation \ + -e {toxinidir}/opentelemetry-sdk \ -e {toxinidir}/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-requests \ -e {toxinidir}/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-wsgi @@ -194,6 +194,7 @@ changedir = commands_pre = pip install -e {toxinidir}/opentelemetry-api \ + -e {toxinidir}/opentelemetry-instrumentation \ -e {toxinidir}/opentelemetry-sdk \ -e {toxinidir}/tests/util \ -e {toxinidir}/exporter/opentelemetry-exporter-opencensus