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

Commit

Permalink
Fix @tag_args being off-by-one (ahead) (#13452)
Browse files Browse the repository at this point in the history
Fix @tag_args being off-by-one (ahead)

Example:

```
argspec.args=[
  'self',
  'room_id'
]

args=(
  <synapse.storage.databases.main.DataStore object at 0x10d0b8d00>,
  '!HBehERstyQBxyJDLfR:my.synapse.server'
)
```

---

The previous logic was also flawed and we can end up in a situation like this:

```
argspec.args=['self', 'dest', 'room_id', 'limit', 'extremities']

args=(<synapse.federation.federation_client.FederationClient object at 0x7f1651c18160>, 'hs1', '!jAEHKIubyIfuLOdfpY:hs1')
```

From this source:
```py
async def backfill(
    self, dest: str, room_id: str, limit: int, extremities: Collection[str]
) -> Optional[List[EventBase]]:
```

And this usage:
```py
events = await self._federation_client.backfill(
    dest, room_id, limit=limit, extremities=extremities
)
```

which would previously cause this error:
```
synapse_main | 2022-08-04 06:13:12,051 - synapse.handlers.federation - 424 - ERROR - GET-5 - Failed to backfill from hs1 because tuple index out of range
synapse_main | Traceback (most recent call last):
synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/handlers/federation.py", line 392, in try_backfill
synapse_main |     await self._federation_event_handler.backfill(
synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/logging/tracing.py", line 828, in _wrapper
synapse_main |     return await func(*args, **kwargs)
synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/handlers/federation_event.py", line 593, in backfill
synapse_main |     events = await self._federation_client.backfill(
synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/logging/tracing.py", line 828, in _wrapper
synapse_main |     return await func(*args, **kwargs)
synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/logging/tracing.py", line 827, in _wrapper
synapse_main |     with wrapping_logic(func, *args, **kwargs):
synapse_main |   File "/usr/local/lib/python3.9/contextlib.py", line 119, in __enter__
synapse_main |     return next(self.gen)
synapse_main |   File "/usr/local/lib/python3.9/site-packages/synapse/logging/tracing.py", line 922, in _wrapping_logic
synapse_main |     set_attribute("ARG_" + arg, str(args[i + 1]))  # type: ignore[index]
synapse_main | IndexError: tuple index out of range
```
  • Loading branch information
MadLittleMods authored Aug 4, 2022
1 parent ec24813 commit 860fdd9
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 2 deletions.
1 change: 1 addition & 0 deletions changelog.d/13452.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix `@tag_args` being off-by-one with the arguments when tagging a span (tracing).
15 changes: 13 additions & 2 deletions synapse/logging/opentracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,11 @@ def trace(func: Callable[P, R]) -> Callable[P, R]:
def tag_args(func: Callable[P, R]) -> Callable[P, R]:
"""
Tags all of the args to the active span.
Args:
func: `func` is assumed to be a method taking a `self` parameter, or a
`classmethod` taking a `cls` parameter. In either case, a tag is not
created for this parameter.
"""

if not opentracing:
Expand All @@ -909,8 +914,14 @@ def tag_args(func: Callable[P, R]) -> Callable[P, R]:
@wraps(func)
def _tag_args_inner(*args: P.args, **kwargs: P.kwargs) -> R:
argspec = inspect.getfullargspec(func)
for i, arg in enumerate(argspec.args[1:]):
set_tag("ARG_" + arg, str(args[i])) # type: ignore[index]
# We use `[1:]` to skip the `self` object reference and `start=1` to
# make the index line up with `argspec.args`.
#
# FIXME: We could update this handle any type of function by ignoring the
# first argument only if it's named `self` or `cls`. This isn't fool-proof
# but handles the idiomatic cases.
for i, arg in enumerate(args[1:], start=1): # type: ignore[index]
set_tag("ARG_" + argspec.args[i], str(arg))
set_tag("args", str(args[len(argspec.args) :])) # type: ignore[index]
set_tag("kwargs", str(kwargs))
return func(*args, **kwargs)
Expand Down

0 comments on commit 860fdd9

Please sign in to comment.