refactor: Typing fixes open-telemetry#768, upgrade mypy to 0.770 (open-telemetry#769)

Author: aabmass

dev-requirements.txt

@@ -2,7 +2,7 @@ pylint==2.4.4
 flake8==3.7.9
 isort~=4.3
 black>=19.3b0,==19.*
-mypy==0.740
+mypy==0.770
 sphinx~=2.1
 sphinx-rtd-theme~=0.4
 sphinx-autodoc-typehints~=1.10.2

opentelemetry-api/src/opentelemetry/__init__.pyi

@@ -12,9 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# FIXME find a better way to avoid all those "Expression has type "Any"" errors
-# type: ignore
-
 """
 Simple configuration manager
 
@@ -95,17 +92,23 @@
 
 from os import environ
 from re import fullmatch
+from typing import ClassVar, Dict, Optional, TypeVar, Union
 
+ConfigValue = Union[str, bool, int, float]
+_T = TypeVar("_T", ConfigValue, Optional[ConfigValue])
 
-class Configuration:
-    _instance = None
 
-    __slots__ = []
+class Configuration:
+    _instance = None  # type: ClassVar[Optional[Configuration]]
+    _config_map = {}  # type: ClassVar[Dict[str, ConfigValue]]
 
     def __new__(cls) -> "Configuration":
-        if Configuration._instance is None:
+        if cls._instance is not None:
+            instance = cls._instance
+        else:
 
-            for key, value in environ.items():
+            instance = super().__new__(cls)
+            for key, value_str in environ.items():
 
                 match = fullmatch(
                     r"OPENTELEMETRY_PYTHON_([A-Za-z_][\w_]*)", key
@@ -114,56 +117,54 @@ def __new__(cls) -> "Configuration":
                 if match is not None:
 
                     key = match.group(1)
+                    value = value_str  # type: ConfigValue
 
-                    if value == "True":
+                    if value_str == "True":
                         value = True
-                    elif value == "False":
+                    elif value_str == "False":
                         value = False
                     else:
                         try:
-                            value = int(value)
+                            value = int(value_str)
                         except ValueError:
                             pass
                         try:
-                            value = float(value)
+                            value = float(value_str)
                         except ValueError:
                             pass
 
-                    setattr(Configuration, "_{}".format(key), value)
-                    setattr(
-                        Configuration,
-                        key,
-                        property(
-                            fget=lambda cls, key=key: getattr(
-                                cls, "_{}".format(key)
-                            )
-                        ),
-                    )
+                    instance._config_map[key] = value
 
-                Configuration.__slots__.append(key)
+            cls._instance = instance
 
-            Configuration.__slots__ = tuple(Configuration.__slots__)
+        return instance
 
-            Configuration._instance = object.__new__(cls)
+    def __getattr__(self, name: str) -> Optional[ConfigValue]:
+        return self._config_map.get(name)
 
-        return cls._instance
+    def __setattr__(self, key: str, val: ConfigValue) -> None:
+        if key == "_config_map":
+            super().__setattr__(key, val)
+        else:
+            raise AttributeError(key)
 
-    def __getattr__(self, name):
-        return None
+    def get(self, name: str, default: _T) -> _T:
+        """Use this typed method for dynamic access instead of `getattr`
+
+        :rtype: str or bool or int or float or None
+        """
+        val = self._config_map.get(name, default)
+        return val
 
     @classmethod
-    def _reset(cls):
+    def _reset(cls) -> None:
         """
         This method "resets" the global configuration attributes
 
         It is not intended to be used by production code but by testing code
         only.
         """
 
-        for slot in cls.__slots__:
-            if slot in cls.__dict__.keys():
-                delattr(cls, slot)
-                delattr(cls, "_{}".format(slot))
-
-        cls.__slots__ = []
-        cls._instance = None
+        if cls._instance:
+            cls._instance._config_map.clear()  # pylint: disable=protected-access
+            cls._instance = None

opentelemetry-api/src/opentelemetry/configuration/__init__.py

@@ -31,7 +31,7 @@
 from logging import getLogger
 from typing import Callable, Dict, Optional, Sequence, Tuple, Type, TypeVar
 
-from opentelemetry.util import _load_provider
+from opentelemetry.util import _load_meter_provider
 
 logger = getLogger(__name__)
 ValueT = TypeVar("ValueT", int, float)
@@ -395,6 +395,6 @@ def get_meter_provider() -> MeterProvider:
     global _METER_PROVIDER  # pylint: disable=global-statement
 
     if _METER_PROVIDER is None:
-        _METER_PROVIDER = _load_provider("meter_provider")
+        _METER_PROVIDER = _load_meter_provider("meter_provider")
 
     return _METER_PROVIDER

opentelemetry-api/src/opentelemetry/metrics/__init__.py

@@ -78,7 +78,7 @@
 from logging import getLogger
 
 from opentelemetry.trace.status import Status
-from opentelemetry.util import _load_provider, types
+from opentelemetry.util import _load_trace_provider, types
 
 logger = getLogger(__name__)
 
@@ -706,6 +706,6 @@ def get_tracer_provider() -> TracerProvider:
     global _TRACER_PROVIDER  # pylint: disable=global-statement
 
     if _TRACER_PROVIDER is None:
-        _TRACER_PROVIDER = _load_provider("tracer_provider")
+        _TRACER_PROVIDER = _load_trace_provider("tracer_provider")
 
     return _TRACER_PROVIDER

opentelemetry-api/src/opentelemetry/trace/__init__.py

@@ -14,11 +14,17 @@
 import re
 import time
 from logging import getLogger
-from typing import Sequence, Union
+from typing import TYPE_CHECKING, Sequence, Union, cast
 
 from pkg_resources import iter_entry_points
 
-from opentelemetry.configuration import Configuration  # type: ignore
+from opentelemetry.configuration import Configuration
+
+if TYPE_CHECKING:
+    from opentelemetry.trace import TracerProvider
+    from opentelemetry.metrics import MeterProvider
+
+Provider = Union["TracerProvider", "MeterProvider"]
 
 logger = getLogger(__name__)
 
@@ -34,25 +40,33 @@ def time_ns() -> int:
         return int(time.time() * 1e9)
 
 
-def _load_provider(
-    provider: str,
-) -> Union["TracerProvider", "MeterProvider"]:  # type: ignore
+def _load_provider(provider: str) -> Provider:
     try:
-        return next(  # type: ignore
+        entry_point = next(
             iter_entry_points(
                 "opentelemetry_{}".format(provider),
-                name=getattr(
-                    Configuration(),  # type: ignore
-                    provider,
-                    "default_{}".format(provider),
+                name=cast(
+                    str,
+                    Configuration().get(
+                        provider, "default_{}".format(provider),
+                    ),
                 ),
             )
-        ).load()()
+        )
+        return cast(Provider, entry_point.load()(),)
     except Exception:  # pylint: disable=broad-except
         logger.error("Failed to load configured provider %s", provider)
         raise
 
 
+def _load_meter_provider(provider: str) -> "MeterProvider":
+    return cast("MeterProvider", _load_provider(provider))
+
+
+def _load_trace_provider(provider: str) -> "TracerProvider":
+    return cast("TracerProvider", _load_provider(provider))
+
+
 # Pattern for matching up until the first '/' after the 'https://' part.
 _URL_PATTERN = r"(https?|ftp)://.*?/"
 

opentelemetry-api/src/opentelemetry/util/__init__.py

@@ -16,16 +16,16 @@
 from unittest import TestCase
 from unittest.mock import patch
 
-from opentelemetry.configuration import Configuration  # type: ignore
+from opentelemetry.configuration import Configuration
 
 
 class TestConfiguration(TestCase):
-    def tearDown(self):
+    def tearDown(self) -> None:
         # This call resets the attributes of the Configuration class so that
         # each test is executed in the same conditions.
         Configuration._reset()
 
-    def test_singleton(self):
+    def test_singleton(self) -> None:
         self.assertIsInstance(Configuration(), Configuration)
         self.assertIs(Configuration(), Configuration())
 
@@ -39,7 +39,7 @@ def test_singleton(self):
             "OPENTELEMETRY_PTHON_TRACEX_PROVIDER": "tracex_provider",
         },
     )
-    def test_environment_variables(self):  # type: ignore
+    def test_environment_variables(self):
         self.assertEqual(
             Configuration().METER_PROVIDER, "meter_provider"
         )  # pylint: disable=no-member
@@ -62,16 +62,21 @@ def test_property(self):
         with self.assertRaises(AttributeError):
             Configuration().TRACER_PROVIDER = "new_tracer_provider"
 
-    def test_slots(self):
+    def test_slots(self) -> None:
         with self.assertRaises(AttributeError):
             Configuration().XYZ = "xyz"  # pylint: disable=assigning-non-slot
 
-    def test_getattr(self):
+    def test_getattr(self) -> None:
+        # literal access
         self.assertIsNone(Configuration().XYZ)
 
-    def test_reset(self):
+        # dynamic access
+        self.assertIsNone(getattr(Configuration(), "XYZ"))
+        self.assertIsNone(Configuration().get("XYZ", None))
+
+    def test_reset(self) -> None:
         environ_patcher = patch.dict(
-            "os.environ",  # type: ignore
+            "os.environ",
             {"OPENTELEMETRY_PYTHON_TRACER_PROVIDER": "tracer_provider"},
         )
 
@@ -96,7 +101,7 @@ def test_reset(self):
             "OPENTELEMETRY_PYTHON_FALSE": "False",
         },
     )
-    def test_boolean(self):
+    def test_boolean(self) -> None:
         self.assertIsInstance(
             Configuration().TRUE, bool
         )  # pylint: disable=no-member
@@ -114,7 +119,7 @@ def test_boolean(self):
             "OPENTELEMETRY_PYTHON_NON_INTEGER": "-12z3",
         },
     )
-    def test_integer(self):
+    def test_integer(self) -> None:
         self.assertEqual(
             Configuration().POSITIVE_INTEGER, 123
         )  # pylint: disable=no-member
@@ -133,7 +138,7 @@ def test_integer(self):
             "OPENTELEMETRY_PYTHON_NON_FLOAT": "-12z3.123",
         },
     )
-    def test_float(self):
+    def test_float(self) -> None:
         self.assertEqual(
             Configuration().POSITIVE_FLOAT, 123.123
         )  # pylint: disable=no-member