Repository navigation
Conversation
Pull request dashboard statusWaiting on the author · refreshed 2026-10-29 15:45 UTC Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):
Status above doesn't look right?
|
| attributes: Attributes = None, | ||
| event_name: str | None = None, | ||
| exception: BaseException | None = None, | ||
| **kwargs: Unpack[LoggerOptions], |
There was a problem hiding this comment.
WDYT of using **kwargs: Never? Based on some (AI) testing I did, mypy and pyright don't behave consistently; mypy doesn't force implementations to have a catch all **kwargs in their signature if the TypedDict is empty (like we have here).
Table from AI testing
| Base-class annotation | (2) typos at call site are caught | (3) override that drops **kwargs | Notes |
|---|---|---|---|
| **kwargs: Any | ❌ pyright, ❌ mypy | ✅ both | What OTel metrics does today (implicitly). Forces implementers, but callers can pass anything. |
| **kwargs: Never | ✅ both ("int not assignable to Never") | ✅ both | Single method, no runtime cost, works in both checkers. Slightly unusual-looking |
| **kwargs: Unpack[EmptyTypedDict] | ✅ both | ✅ pyright, ❌ mypy | mypy treats an empty TypedDict unpack as "no extra params" and accepts an implementation declared with **kwargs. |
There was a problem hiding this comment.
Sure, let me give it a go. May also update the advisory attributes PR with this convention as well.
| """ | ||
|
|
||
|
|
||
| class LoggerOptions(TypedDict): |
There was a problem hiding this comment.
Would we leave this empty and add new parameters to the actual signatures, or would we actually add new params here?
There was a problem hiding this comment.
I guess there's a few options here:
- Use
**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. - Stick with
**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.
Description
Makes the logs API forward compatible: new keyword-only parameters can be added later without breaking existing
Logger/LoggerProviderimplementations.LoggerOptionsandLoggerProviderOptions, exported fromopentelemetry._logs.Logger.emit,Logger.enabled,LoggerProvider.get_loggerand the module-levelget_logger()now accept**kwargs: Unpack[...]of these types.ProxyLoggerProvider/ProxyLoggerforward the kwargs; NoOp and SDK implementations accept and ignore them.Logger.emitgains@overloads that match the API so pyright accepts the override.Type of change
Please delete options that are not relevant.
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.
tox -e precommittox -e typecheck