Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add type annotations to trace decorator #13328

Merged
merged 11 commits into from
Jul 19, 2022
Merged

Add type annotations to trace decorator #13328

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c6914ee
Split trace into two functions.
clokep Jul 19, 2022
893cb52
Annotate trace and trace_with_opname.
clokep Jul 19, 2022
52318fb
Federation timeout is an Optional[int].
clokep Jul 19, 2022
fd0d962
The 'version' query parameter is required.
clokep Jul 19, 2022
245eb11
Generalize return type.
clokep Jul 19, 2022
725b44a
Fix simple end_to_end_keys errors.
clokep Jul 19, 2022
5e99e74
(Mostly) fix get_e2e_device_keys_and_signatures signature.
clokep Jul 19, 2022
244c719
Fix remaining errors.
clokep Jul 19, 2022
99f35bb
Newsfragment
clokep Jul 19, 2022
65a7218
Fix invariance.
clokep Jul 19, 2022
cde9700
Fix Literal import.
clokep Jul 19, 2022
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
14 changes: 7 additions & 7 deletions synapse/logging/opentracing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,30 +797,30 @@ def extract_text_map(carrier: Dict[str, str]) -> Optional["opentracing.SpanConte
# Tracing decorators


def trace_with_opname(opname: Optional[str] = None):
def trace_with_opname(opname: str) -> Callable[[Callable[P, R]], Callable[P, R]]:
"""
Decorator to trace a function with a custom opname.

See the module's doc string for usage examples.

"""

def decorator(func):
def decorator(func: Callable[P, R]) -> Callable[P, R]:
if opentracing is None:
return func # type: ignore[unreachable]

if inspect.iscoroutinefunction(func):

@wraps(func)
async def _trace_inner(*args, **kwargs):
async def _trace_inner(*args: P.args, **kwargs: P.kwargs) -> R:
with start_active_span(opname):
return await func(*args, **kwargs)
return await func(*args, **kwargs) # type: ignore[misc]
Copy link
Member Author

@clokep clokep Jul 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

For completeness this is ignoring:

error: Incompatible types in "await" (actual type "R", expected type "Awaitable[Any]")  [misc]

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. I think this is fine: within this branch, func is a coroutine so should return an awaitable. It's a shame mypy can't deduce this for itself, given the use of TypeGuard in typeshed.

Oh, but that bit of typeshed I quoted only just landed this morning! So we might find that this ignore is unnecessary in a future mypy release.


else:
# The other case here handles both sync functions and those
# decorated with inlineDeferred.
@wraps(func)
def _trace_inner(*args, **kwargs):
def _trace_inner(*args: P.args, **kwargs: P.kwargs) -> R:
scope = start_active_span(opname)
scope.__enter__()

Expand Down Expand Up @@ -855,12 +855,12 @@ def err_back(result: R) -> R:
scope.__exit__(type(e), None, e.__traceback__)
raise

return _trace_inner
return _trace_inner # type: ignore[return-value]
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the error here, out of interest?

Copy link
Member Author

Choose a reason for hiding this comment

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

synapse/logging/opentracing.py:858: error: Incompatible return value type (got "Callable[P, Coroutine[Any, Any, R]]", expected "Callable[P, R]")  [return-value]

Actually now that I split this up I might be able to do simple overloads with returning Awaitable[R] in a couple situations and remove this ignore?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure that's possible actually. Any idea if we should do something about this one?

Copy link
Contributor

Choose a reason for hiding this comment

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

As sad as I am to leave a typing problem lying around like this, I think I'd probably leave the ignore as is. The important thing is that we accurately annotate the decorator itself; its internals don't have an effect on the rest of the function.

Copy link
Member Author

Choose a reason for hiding this comment

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

its internals don't have an effect on the rest of the function.

This was kind of my conclusion after messing with it for a bit.


return decorator


def trace(func):
def trace(func: Callable[P, R]) -> Callable[P, R]:
"""
Decorator to trace a function.

Expand Down