Merge branch 'master' into recreate-span-on-every-run

alrex · web-flow · commit e911e4f49c5b · 2020-12-15T20:15:45.000-08:00
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -74,13 +74,12 @@ 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 }}
           # 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' }}
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,7 +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))
-### Changed
+- 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))
 - Recreate span on every run of a `start_as_current_span`-decorated function
   ([#1451](https://github.com/open-telemetry/opentelemetry-python/pull/1451))
 
diff --git 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
diff --git 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
@@ -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
@@ -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
@@ -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-sdk/src/opentelemetry/sdk/configuration/__init__.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,15 +113,15 @@ 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 = {}, {}
 
     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-sdk/tests/configuration/test_configurator.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,21 +131,18 @@ 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=[
                 IterEntryPoint("custom_ids_generator", CustomIdsGenerator)
             ]
         )
         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
@@ -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
