Repository navigation
feat(api): add forward-compatible kwargs to Logger and LoggerProvider #5851
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| `opentelemetry-api`, `opentelemetry-sdk`: accept forward-compatible `**kwargs` (`LoggerOptions`, `LoggerProviderOptions`) in `Logger.emit`, `Logger.enabled` and `LoggerProvider.get_logger` |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,7 +28,7 @@ | |||||||||||||||||
| from logging import getLogger | ||||||||||||||||||
| from os import environ | ||||||||||||||||||
| from time import time_ns | ||||||||||||||||||
| from typing import cast, overload | ||||||||||||||||||
| from typing import TYPE_CHECKING, TypedDict, cast, overload | ||||||||||||||||||
|
|
||||||||||||||||||
| from typing_extensions import deprecated | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -42,9 +42,26 @@ | |||||||||||||||||
| from opentelemetry.util._providers import _load_provider | ||||||||||||||||||
| from opentelemetry.util.types import AnyValue, Attributes | ||||||||||||||||||
|
|
||||||||||||||||||
| if TYPE_CHECKING: | ||||||||||||||||||
| from typing_extensions import Unpack | ||||||||||||||||||
|
|
||||||||||||||||||
| _logger = getLogger(__name__) | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| class LoggerProviderOptions(TypedDict): | ||||||||||||||||||
| """Additional keyword-only arguments accepted by `LoggerProvider.get_logger`. | ||||||||||||||||||
|
|
||||||||||||||||||
| Reserved for forward compatibility, currently empty. | ||||||||||||||||||
| """ | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| class LoggerOptions(TypedDict): | ||||||||||||||||||
| """Additional keyword-only arguments accepted by `Logger.emit` and `Logger.enabled`. | ||||||||||||||||||
|
|
||||||||||||||||||
| Reserved for forward compatibility, currently empty. | ||||||||||||||||||
| """ | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| class LogRecord(ABC): | ||||||||||||||||||
| """A LogRecord instance represents an event being logged. | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -165,12 +182,14 @@ def emit( | |||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||
| event_name: str | None = None, | ||||||||||||||||||
| exception: BaseException | None = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT of using Table from AI testing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, let me give it a go. May also update the advisory attributes PR with this convention as well. |
||||||||||||||||||
| ) -> None: ... | ||||||||||||||||||
|
|
||||||||||||||||||
| @overload | ||||||||||||||||||
| def emit( | ||||||||||||||||||
| self, | ||||||||||||||||||
| record: LogRecord, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
| ) -> None: ... | ||||||||||||||||||
|
|
||||||||||||||||||
| @abstractmethod | ||||||||||||||||||
|
|
@@ -187,6 +206,7 @@ def emit( | |||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||
| event_name: str | None = None, | ||||||||||||||||||
| exception: BaseException | None = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
| ) -> None: | ||||||||||||||||||
| """Emits a :class:`LogRecord` representing a log to the processing pipeline.""" | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -197,6 +217,7 @@ def enabled( | |||||||||||||||||
| context: Context | None = None, | ||||||||||||||||||
| severity_number: SeverityNumber | None = None, | ||||||||||||||||||
| event_name: str | None = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
| ) -> bool: | ||||||||||||||||||
| """Returns whether the logger is enabled for the given arguments. | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -227,12 +248,14 @@ def emit( | |||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||
| event_name: str | None = None, | ||||||||||||||||||
| exception: BaseException | None = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
| ) -> None: ... | ||||||||||||||||||
|
|
||||||||||||||||||
| @overload | ||||||||||||||||||
| def emit( # pylint:disable=arguments-differ | ||||||||||||||||||
| self, | ||||||||||||||||||
| record: LogRecord, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
| ) -> None: ... | ||||||||||||||||||
|
|
||||||||||||||||||
| def emit( | ||||||||||||||||||
|
|
@@ -248,6 +271,7 @@ def emit( | |||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||
| event_name: str | None = None, | ||||||||||||||||||
| exception: BaseException | None = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
| ) -> None: | ||||||||||||||||||
| pass | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -257,6 +281,7 @@ def enabled( | |||||||||||||||||
| context: Context | None = None, | ||||||||||||||||||
| severity_number: SeverityNumber | None = None, | ||||||||||||||||||
| event_name: str | None = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
| ) -> bool: | ||||||||||||||||||
| return False | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -268,11 +293,13 @@ def __init__( # pylint: disable=super-init-not-called | |||||||||||||||||
| version: str | None = None, | ||||||||||||||||||
| schema_url: str | None = None, | ||||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerProviderOptions], | ||||||||||||||||||
| ): | ||||||||||||||||||
| self._name = name | ||||||||||||||||||
| self._version = version | ||||||||||||||||||
| self._schema_url = schema_url | ||||||||||||||||||
| self._attributes = attributes | ||||||||||||||||||
| self._get_logger_kwargs = kwargs | ||||||||||||||||||
| self._real_logger: Logger | None = None | ||||||||||||||||||
| self._noop_logger = NoOpLogger(name) | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -287,6 +314,7 @@ def _logger(self) -> Logger: | |||||||||||||||||
| self._version, | ||||||||||||||||||
| self._schema_url, | ||||||||||||||||||
| self._attributes, | ||||||||||||||||||
| **self._get_logger_kwargs, | ||||||||||||||||||
| ) | ||||||||||||||||||
| return self._real_logger | ||||||||||||||||||
| return self._noop_logger | ||||||||||||||||||
|
|
@@ -304,12 +332,14 @@ def emit( | |||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||
| event_name: str | None = None, | ||||||||||||||||||
| exception: BaseException | None = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
| ) -> None: ... | ||||||||||||||||||
|
|
||||||||||||||||||
| @overload | ||||||||||||||||||
| def emit( # pylint:disable=arguments-differ | ||||||||||||||||||
| self, | ||||||||||||||||||
| record: LogRecord, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
| ) -> None: ... | ||||||||||||||||||
|
|
||||||||||||||||||
| def emit( | ||||||||||||||||||
|
|
@@ -325,9 +355,10 @@ def emit( | |||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||
| event_name: str | None = None, | ||||||||||||||||||
| exception: BaseException | None = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
| ) -> None: | ||||||||||||||||||
| if record: | ||||||||||||||||||
| self._logger.emit(record) | ||||||||||||||||||
| self._logger.emit(record, **kwargs) | ||||||||||||||||||
| else: | ||||||||||||||||||
| self._logger.emit( | ||||||||||||||||||
| timestamp=timestamp, | ||||||||||||||||||
|
|
@@ -339,6 +370,7 @@ def emit( | |||||||||||||||||
| attributes=attributes, | ||||||||||||||||||
| event_name=event_name, | ||||||||||||||||||
| exception=exception, | ||||||||||||||||||
| **kwargs, | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| def enabled( | ||||||||||||||||||
|
|
@@ -347,11 +379,13 @@ def enabled( | |||||||||||||||||
| context: Context | None = None, | ||||||||||||||||||
| severity_number: SeverityNumber | None = None, | ||||||||||||||||||
| event_name: str | None = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerOptions], | ||||||||||||||||||
| ) -> bool: | ||||||||||||||||||
| return self._logger.enabled( | ||||||||||||||||||
| context=context, | ||||||||||||||||||
| severity_number=severity_number, | ||||||||||||||||||
| event_name=event_name, | ||||||||||||||||||
| **kwargs, | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -367,6 +401,7 @@ def get_logger( | |||||||||||||||||
| version: str | None = None, | ||||||||||||||||||
| schema_url: str | None = None, | ||||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerProviderOptions], | ||||||||||||||||||
| ) -> Logger: | ||||||||||||||||||
| """Returns a `Logger` for use by the given instrumentation library. | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -394,6 +429,9 @@ def get_logger( | |||||||||||||||||
|
|
||||||||||||||||||
| attributes: Optional. Specifies the instrumentation scope attributes to | ||||||||||||||||||
| associate with emitted telemetry. | ||||||||||||||||||
|
|
||||||||||||||||||
| kwargs: Additional keyword-only arguments, see `LoggerProviderOptions`. | ||||||||||||||||||
| Reserved for forward compatibility. | ||||||||||||||||||
| """ | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -406,6 +444,7 @@ def get_logger( | |||||||||||||||||
| version: str | None = None, | ||||||||||||||||||
| schema_url: str | None = None, | ||||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerProviderOptions], | ||||||||||||||||||
| ) -> Logger: | ||||||||||||||||||
| """Returns a NoOpLogger.""" | ||||||||||||||||||
| return NoOpLogger(name, version=version, schema_url=schema_url, attributes=attributes) | ||||||||||||||||||
|
|
@@ -418,19 +457,22 @@ def get_logger( | |||||||||||||||||
| version: str | None = None, | ||||||||||||||||||
| schema_url: str | None = None, | ||||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerProviderOptions], | ||||||||||||||||||
| ) -> Logger: | ||||||||||||||||||
| if _LOGGER_PROVIDER: | ||||||||||||||||||
| return _LOGGER_PROVIDER.get_logger( | ||||||||||||||||||
| name, | ||||||||||||||||||
| version=version, | ||||||||||||||||||
| schema_url=schema_url, | ||||||||||||||||||
| attributes=attributes, | ||||||||||||||||||
| **kwargs, | ||||||||||||||||||
| ) | ||||||||||||||||||
| return ProxyLogger( | ||||||||||||||||||
| name, | ||||||||||||||||||
| version=version, | ||||||||||||||||||
| schema_url=schema_url, | ||||||||||||||||||
| attributes=attributes, | ||||||||||||||||||
| **kwargs, | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -481,6 +523,7 @@ def get_logger( | |||||||||||||||||
| logger_provider: LoggerProvider | None = None, | ||||||||||||||||||
| schema_url: str | None = None, | ||||||||||||||||||
| attributes: Attributes = None, | ||||||||||||||||||
| **kwargs: Unpack[LoggerProviderOptions], | ||||||||||||||||||
| ) -> Logger: | ||||||||||||||||||
| """Returns a `Logger` for use within a python process. | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -496,4 +539,5 @@ def get_logger( | |||||||||||||||||
| instrumenting_library_version, | ||||||||||||||||||
| schema_url, | ||||||||||||||||||
| attributes, | ||||||||||||||||||
| **kwargs, | ||||||||||||||||||
| ) | ||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we leave this empty and add new parameters to the actual signatures, or would we actually add new params here?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess there's a few options here:
**kwargs: Neveras you suggested above, and just add additional keyword arguments to the API/SDK signatures directly. This should be safe since if an implementation has**kwargs: Never, it must always accept any keyword arguments (at runtime), even though it might ignore them.**kwargs: Neverinitially, and then switch to**kwargs: Unpack[LoggerOptions]once we have additional options we want to add.I actually didn't think about the first case until you brought it up, and I do somewhat like it better.