Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Running mypy on sdk resources #773 #4360

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
+23 −24
Open

Running mypy on sdk resources #773 #4360

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3ab4dc7
Running mypy on sdk resources #773
prabhakarjuzgar Dec 16, 2024
c38154e
Update changelog
prabhakarjuzgar Dec 16, 2024
5c6d89b
Remove unicode chars and unwanted ignore
prabhakarjuzgar Dec 18, 2024
fc6c6e5
Remove unnecessary changes from CHANGELOG
prabhakarjuzgar Dec 18, 2024
822e841
Change importlib_metadata to importlib.metadata
prabhakarjuzgar Dec 20, 2024
192db0c
Address review comment
prabhakarjuzgar Dec 20, 2024
853c00f
Use importlib_metadata if importlib.data is unavailable
prabhakarjuzgar Dec 21, 2024
2ddbd55
Merge branch 'main' into pr773
xrmx Dec 23, 2024
d42a650
Remove unnecessary changes
prabhakarjuzgar Dec 23, 2024
99f1ff4
do not change implementation
prabhakarjuzgar Dec 23, 2024
825c727
do not change implementation final
prabhakarjuzgar Dec 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- Run mypy on SDK resources
prabhakarjuzgar marked this conversation as resolved.
Show resolved Hide resolved
([#4360](https://github.com/open-telemetry/opentelemetry-python/pull/4360))

## Version 1.29.0/0.50b0 (2024-12-11)

- Fix crash exporting a log record with None body
Expand Down Expand Up @@ -274,7 +277,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## Version 1.21.0/0.42b0 (2023-11-01)

- Fix `SumAggregation`
 ([#3390](https://github.com/open-telemetry/opentelemetry-python/pull/3390))
([#3390](https://github.com/open-telemetry/opentelemetry-python/pull/3390))
- Fix handling of empty metric collection cycles
([#3335](https://github.com/open-telemetry/opentelemetry-python/pull/3335))
- Fix error when no LoggerProvider configured for LoggingHandler
Expand All @@ -294,7 +297,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Implement Process Resource detector
([#3472](https://github.com/open-telemetry/opentelemetry-python/pull/3472))


## Version 1.20.0/0.41b0 (2023-09-04)

- Modify Prometheus exporter to translate non-monotonic Sums into Gauges
Expand Down Expand Up @@ -323,7 +325,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Default LogRecord observed_timestamp to current timestamp
[#3377](https://github.com/open-telemetry/opentelemetry-python/pull/3377))


## Version 1.18.0/0.39b0 (2023-05-19)

- Select histogram aggregation with an environment variable
Expand All @@ -343,7 +344,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add benchmark tests for metrics
([#3267](https://github.com/open-telemetry/opentelemetry-python/pull/3267))


## Version 1.17.0/0.38b0 (2023-03-22)

- Implement LowMemory temporality
Expand Down Expand Up @@ -1691,7 +1691,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Remove dependency on 'backoff' library
([#3679](https://github.com/open-telemetry/opentelemetry-python/pull/3679))


- Make create_gauge non-abstract method
([#3817](https://github.com/open-telemetry/opentelemetry-python/pull/3817))
- Make `tracer.start_as_current_span()` decorator work with async functions
Expand Down
27 changes: 14 additions & 13 deletions opentelemetry-sdk/src/opentelemetry/sdk/error_handler/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ def _handle(self, error: Exception, *args, **kwargs):

from abc import ABC, abstractmethod
from logging import getLogger
from typing import Optional

from opentelemetry.util._importlib_metadata import entry_points

Expand All @@ -69,7 +70,7 @@ def _handle(self, error: Exception, *args, **kwargs):

class ErrorHandler(ABC):
@abstractmethod
def _handle(self, error: Exception, *args, **kwargs):
def _handle(self, error: Exception, *args, **kwargs) -> None: # type: ignore
"""
Handle an exception
"""
Expand All @@ -83,7 +84,7 @@ class _DefaultErrorHandler(ErrorHandler):
"""

# pylint: disable=useless-return
def _handle(self, error: Exception, *args, **kwargs):
def _handle(self, error: Exception, *args, **kwargs) -> None: # type: ignore
logger.exception("Error handled by default error handler: ")
return None

Expand All @@ -105,26 +106,26 @@ def __new__(cls) -> "GlobalErrorHandler":

return cls._instance

def __enter__(self):
def __enter__(self) -> None:
pass

# pylint: disable=no-self-use
def __exit__(self, exc_type, exc_value, traceback):
if exc_value is None:
def __exit__(self, exc_type, exc_value, traceback) -> Optional[bool]: # type: ignore
if exc_value is None: # type: ignore
return None

plugin_handled = False

error_handler_entry_points = entry_points(
error_handler_entry_points = entry_points( # type: ignore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be typed, https://github.com/python/importlib_metadata/blob/main/importlib_metadata/__init__.py#L1039C42-L1039C43
Need to check if we are exporting it from our util module

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is - https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/util/_importlib_metadata.py#L28

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

basedpyright: Import "opentelemetry.util._importlib_metadata" could not be resolved. This results in following errors when mypy is executed -

opentelemetry-sdk/src/opentelemetry/sdk/error_handler/init.py:119: error: Type of variable becomes "Any" due to an unfollowed import [no-any-unimported]
opentelemetry-sdk/src/opentelemetry/sdk/error_handler/init.py:119: error: Expression has type "Any" [misc]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xrmx Please let me know if the above errors can be handled differently.

Copy link
Contributor

@xrmx xrmx Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean something like this error_handler_entry_points : Entrypoints = entry_points(

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok. I added:

mypy: mypy --install-types --non-interactive --namespace-packages --explicit-package-bases opentelemetry-sdk/src/opentelemetry/sdk/error_handler

I've confirmed. mypy cannot find opentelemetry/util, so it converts entry_points to Any.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy: MYPYPATH={toxinidir}/opentelemetry-api/src/:{toxinidir}/opentelemetry-semantic-conventions/src/:{toxinidir}/opentelemetry-sdk/src/:{toxinidir}/tests/opentelemetry-test-utils/src/

includes {toxinidir}/opentelemetry-api/src/.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, so my point is that it should have been found, but it's not.

Copy link
Author

@prabhakarjuzgar prabhakarjuzgar Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I changed
from importlib_metadata import (
to
from importlib.metadata import (
in https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/util/_importlib_metadata.py
mypy is able to find the imports. importlib_metadata is valid for python < 3.8.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update the PR to change importlib_metadata to imortlib.metadata and will remove ignore in the files -
opentelemetry-api/src/opentelemetry/context/__init__. opentelemetry-api/src/opentelemetry/propagate/__init__.py opentelemetry-api/src/opentelemetry/util/_importlib_metadata.py opentelemetry-api/src/opentelemetry/util/_providers.py

group="opentelemetry_error_handler"
)

for error_handler_entry_point in error_handler_entry_points:
error_handler_class = error_handler_entry_point.load()
for error_handler_entry_point in error_handler_entry_points: # type: ignore
error_handler_class = error_handler_entry_point.load() # type: ignore

if issubclass(error_handler_class, exc_value.__class__):
if issubclass(error_handler_class, exc_value.__class__): # type: ignore
try:
error_handler_class()._handle(exc_value)
error_handler_class()._handle(exc_value) # type: ignore
plugin_handled = True

# pylint: disable=broad-exception-caught
Expand All @@ -133,11 +134,11 @@ def __exit__(self, exc_type, exc_value, traceback):
"%s error while handling error"
" %s by error handler %s",
error_handling_error.__class__.__name__,
exc_value.__class__.__name__,
error_handler_class.__name__,
exc_value.__class__.__name__, # type: ignore
error_handler_class.__name__, # type: ignore
)

if not plugin_handled:
_DefaultErrorHandler()._handle(exc_value)
_DefaultErrorHandler()._handle(exc_value) # type: ignore

return True
14 changes: 7 additions & 7 deletions opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def collect(self, point_attributes: Attributes) -> Optional[Exemplar]:
{
k: v
for k, v in self.__attributes.items()
if k not in point_attributes
if k not in point_attributes # type: ignore
}
if self.__attributes
else None
Expand Down Expand Up @@ -162,8 +162,8 @@ class BucketIndexError(ValueError):
class FixedSizeExemplarReservoirABC(ExemplarReservoir):
"""Abstract class for a reservoir with fixed size."""

def __init__(self, size: int, **kwargs) -> None:
super().__init__(**kwargs)
def __init__(self, size: int, **kwargs) -> None: # type: ignore
super().__init__(**kwargs) # type: ignore
self._size: int = size
self._reservoir_storage: Mapping[int, ExemplarBucket] = defaultdict(
ExemplarBucket
Expand Down Expand Up @@ -257,8 +257,8 @@ class SimpleFixedSizeExemplarReservoir(FixedSizeExemplarReservoirABC):
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#simplefixedsizeexemplarreservoir
"""

def __init__(self, size: int = 1, **kwargs) -> None:
super().__init__(size, **kwargs)
def __init__(self, size: int = 1, **kwargs) -> None: # type: ignore
super().__init__(size, **kwargs) # type: ignore
self._measurements_seen: int = 0

def _reset(self) -> None:
Expand Down Expand Up @@ -292,8 +292,8 @@ class AlignedHistogramBucketExemplarReservoir(FixedSizeExemplarReservoirABC):
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#alignedhistogrambucketexemplarreservoir
"""

def __init__(self, boundaries: Sequence[float], **kwargs) -> None:
super().__init__(len(boundaries) + 1, **kwargs)
def __init__(self, boundaries: Sequence[float], **kwargs) -> None: # type: ignore
super().__init__(len(boundaries) + 1, **kwargs) # type: ignore
self._boundaries: Sequence[float] = boundaries

def offer(
Expand Down