From 51b7e4cacf099953e830a56ed2523c13c06aece2 Mon Sep 17 00:00:00 2001 From: Attila Sasvari Date: Tue, 30 Jul 2024 18:22:08 +0100 Subject: [PATCH] Running mypy on SDK resources (following up #3995) (#4053) --- CHANGELOG.md | 5 +- .../src/opentelemetry/attributes/__init__.py | 47 ++++++++-------- .../opentelemetry/sdk/resources/__init__.py | 55 ++++++++++++------- tox.ini | 4 +- 4 files changed, 66 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c69fee1738..2a2fc5dc3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased -- Added py.typed file to top-level module ([#4084](https://github.com/open-telemetry/opentelemetry-python/pull/4084)) +- Running mypy on SDK resources + ([#4053](https://github.com/open-telemetry/opentelemetry-python/pull/4053)) +- Added py.typed file to top-level module + ([#4084](https://github.com/open-telemetry/opentelemetry-python/pull/4084)) - Drop Final annotation from Enum in semantic conventions ([#4085](https://github.com/open-telemetry/opentelemetry-python/pull/4085)) - Update log export example to not use root logger ([#4090](https://github.com/open-telemetry/opentelemetry-python/pull/4090)) diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index 72ee38abd7..497952984d 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -11,13 +11,12 @@ # 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 import logging import threading from collections import OrderedDict from collections.abc import MutableMapping -from typing import Optional, Sequence, Union +from typing import Optional, Sequence, Tuple, Union from opentelemetry.util import types @@ -31,7 +30,7 @@ def _clean_attribute( key: str, value: types.AttributeValue, max_len: Optional[int] -) -> Optional[types.AttributeValue]: +) -> Optional[Union[types.AttributeValue, Tuple[Union[str, int, float], ...]]]: """Checks if attribute value is valid and cleans it if required. The function returns the cleaned value or None if the value is not valid. @@ -59,7 +58,7 @@ def _clean_attribute( cleaned_seq = [] for element in value: - element = _clean_attribute_value(element, max_len) + element = _clean_attribute_value(element, max_len) # type: ignore if element is None: cleaned_seq.append(element) continue @@ -110,7 +109,7 @@ def _clean_attribute( def _clean_attribute_value( value: types.AttributeValue, limit: Optional[int] -) -> Union[types.AttributeValue, None]: +) -> Optional[types.AttributeValue]: if value is None: return None @@ -126,7 +125,7 @@ def _clean_attribute_value( return value -class BoundedAttributes(MutableMapping): +class BoundedAttributes(MutableMapping): # type: ignore """An ordered dict with a fixed max capacity. Oldest elements are dropped when the dict is full and a new element is @@ -149,28 +148,32 @@ def __init__( self.dropped = 0 self.max_value_len = max_value_len # OrderedDict is not used until the maxlen is reached for efficiency. - self._dict = {} # type: dict | OrderedDict - self._lock = threading.RLock() # type: threading.RLock + + self._dict: Union[ + MutableMapping[str, types.AttributeValue], + OrderedDict[str, types.AttributeValue], + ] = {} + self._lock = threading.RLock() if attributes: for key, value in attributes.items(): self[key] = value self._immutable = immutable - def __repr__(self): + def __repr__(self) -> str: return f"{dict(self._dict)}" - def __getitem__(self, key): + def __getitem__(self, key: str) -> types.AttributeValue: return self._dict[key] - def __setitem__(self, key, value): - if getattr(self, "_immutable", False): + def __setitem__(self, key: str, value: types.AttributeValue) -> None: + if getattr(self, "_immutable", False): # type: ignore raise TypeError with self._lock: if self.maxlen is not None and self.maxlen == 0: self.dropped += 1 return - value = _clean_attribute(key, value, self.max_value_len) + value = _clean_attribute(key, value, self.max_value_len) # type: ignore if value is not None: if key in self._dict: del self._dict[key] @@ -179,23 +182,23 @@ def __setitem__(self, key, value): ): if not isinstance(self._dict, OrderedDict): self._dict = OrderedDict(self._dict) - self._dict.popitem(last=False) + self._dict.popitem(last=False) # type: ignore self.dropped += 1 - self._dict[key] = value + self._dict[key] = value # type: ignore - def __delitem__(self, key): - if getattr(self, "_immutable", False): + def __delitem__(self, key: str) -> None: + if getattr(self, "_immutable", False): # type: ignore raise TypeError with self._lock: del self._dict[key] - def __iter__(self): + def __iter__(self): # type: ignore with self._lock: - return iter(self._dict.copy()) + return iter(self._dict.copy()) # type: ignore - def __len__(self): + def __len__(self) -> int: return len(self._dict) - def copy(self): - return self._dict.copy() + def copy(self): # type: ignore + return self._dict.copy() # type: ignore diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 1fed32c0be..bfb43fae03 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -63,6 +63,8 @@ import typing from json import dumps from os import environ +from types import ModuleType +from typing import List, MutableMapping, Optional, cast from urllib import parse from opentelemetry.attributes import BoundedAttributes @@ -75,10 +77,14 @@ from opentelemetry.util._importlib_metadata import entry_points, version from opentelemetry.util.types import AttributeValue +psutil: Optional[ModuleType] = None + try: - import psutil + import psutil as psutil_module + + pustil = psutil_module except ImportError: - psutil = None + pass LabelValue = AttributeValue Attributes = typing.Mapping[str, LabelValue] @@ -141,12 +147,15 @@ TELEMETRY_AUTO_VERSION = ResourceAttributes.TELEMETRY_AUTO_VERSION TELEMETRY_SDK_LANGUAGE = ResourceAttributes.TELEMETRY_SDK_LANGUAGE -_OPENTELEMETRY_SDK_VERSION = version("opentelemetry-sdk") +_OPENTELEMETRY_SDK_VERSION: str = version("opentelemetry-sdk") class Resource: """A Resource is an immutable representation of the entity producing telemetry as Attributes.""" + _attributes: BoundedAttributes + _schema_url: str + def __init__( self, attributes: Attributes, schema_url: typing.Optional[str] = None ): @@ -173,7 +182,7 @@ def create( if not attributes: attributes = {} - resource_detectors = [] + resource_detectors: List[ResourceDetector] = [] resource = _DEFAULT_RESOURCE @@ -184,6 +193,7 @@ def create( if "otel" not in otel_experimental_resource_detectors: otel_experimental_resource_detectors.append("otel") + resource_detector: str for resource_detector in otel_experimental_resource_detectors: resource_detectors.append( next( @@ -191,19 +201,19 @@ def create( entry_points( group="opentelemetry_resource_detector", name=resource_detector.strip(), - ) + ) # type: ignore ) ).load()() ) - resource = get_aggregated_resources( resource_detectors, _DEFAULT_RESOURCE ).merge(Resource(attributes, schema_url)) if not resource.attributes.get(SERVICE_NAME, None): default_service_name = "unknown_service" - process_executable_name = resource.attributes.get( - PROCESS_EXECUTABLE_NAME, None + process_executable_name = cast( + Optional[str], + resource.attributes.get(PROCESS_EXECUTABLE_NAME, None), ) if process_executable_name: default_service_name += ":" + process_executable_name @@ -241,8 +251,8 @@ def merge(self, other: "Resource") -> "Resource": Returns: The newly-created Resource. """ - merged_attributes = self.attributes.copy() - merged_attributes.update(other.attributes) + merged_attributes = self.attributes.copy() # type: ignore + merged_attributes.update(other.attributes) # type: ignore if self.schema_url == "": schema_url = other.schema_url @@ -257,8 +267,7 @@ def merge(self, other: "Resource") -> "Resource": other.schema_url, ) return self - - return Resource(merged_attributes, schema_url) + return Resource(merged_attributes, schema_url) # type: ignore def __eq__(self, other: object) -> bool: if not isinstance(other, Resource): @@ -268,15 +277,18 @@ def __eq__(self, other: object) -> bool: and self._schema_url == other._schema_url ) - def __hash__(self): + def __hash__(self) -> int: return hash( - f"{dumps(self._attributes.copy(), sort_keys=True)}|{self._schema_url}" + f"{dumps(self._attributes.copy(), sort_keys=True)}|{self._schema_url}" # type: ignore ) - def to_json(self, indent=4) -> str: + def to_json(self, indent: int = 4) -> str: + attributes: MutableMapping[str, AttributeValue] = dict( + self._attributes + ) return dumps( { - "attributes": dict(self._attributes), + "attributes": attributes, # type: ignore "schema_url": self._schema_url, }, indent=indent, @@ -294,7 +306,7 @@ def to_json(self, indent=4) -> str: class ResourceDetector(abc.ABC): - def __init__(self, raise_on_error=False): + def __init__(self, raise_on_error: bool = False) -> None: self.raise_on_error = raise_on_error @abc.abstractmethod @@ -365,16 +377,17 @@ def detect(self) -> "Resource": resource_info[PROCESS_PARENT_PID] = os.getppid() if psutil is not None: - process = psutil.Process() - resource_info[PROCESS_OWNER] = process.username() + process: psutil_module.Process = psutil.Process() + username = process.username() + resource_info[PROCESS_OWNER] = username - return Resource(resource_info) + return Resource(resource_info) # type: ignore def get_aggregated_resources( detectors: typing.List["ResourceDetector"], initial_resource: typing.Optional[Resource] = None, - timeout=5, + timeout: int = 5, ) -> "Resource": """Retrieves resources from detectors in the order that they were passed diff --git a/tox.ini b/tox.ini index 5024ff31d8..1720ce065c 100644 --- a/tox.ini +++ b/tox.ini @@ -126,7 +126,7 @@ setenv = ; i.e: CONTRIB_REPO_SHA=dde62cebffe519c35875af6d06fae053b3be65ec tox -e CONTRIB_REPO_SHA={env:CONTRIB_REPO_SHA:main} CONTRIB_REPO=git+https://github.com/open-telemetry/opentelemetry-python-contrib.git@{env:CONTRIB_REPO_SHA} - mypy: MYPYPATH={toxinidir}/opentelemetry-api/src/:{toxinidir}/tests/opentelemetry-test-utils/src/ + mypy: MYPYPATH={toxinidir}/opentelemetry-api/src/:{toxinidir}/opentelemetry-semantic-conventions/src/:{toxinidir}/opentelemetry-sdk/src/:{toxinidir}/tests/opentelemetry-test-utils/src/ commands_pre = @@ -314,7 +314,9 @@ commands = coverage: {toxinidir}/scripts/coverage.sh + mypy: mypy --version mypy: mypy --install-types --non-interactive --namespace-packages --explicit-package-bases opentelemetry-api/src/opentelemetry/ + mypy: mypy --install-types --non-interactive --namespace-packages --explicit-package-bases opentelemetry-sdk/src/opentelemetry/sdk/resources mypy: mypy --install-types --non-interactive --namespace-packages --explicit-package-bases opentelemetry-semantic-conventions/src/opentelemetry/semconv/ ; For test code, we don't want to enforce the full mypy strictness