diff --git a/changelog.d/13452.misc b/changelog.d/13452.misc
new file mode 100644
index 0000000000..13d1523de2
--- /dev/null
+++ b/changelog.d/13452.misc
@@ -0,0 +1 @@
+Fix `@tag_args` being off-by-one with the arguments when tagging a span (tracing).
diff --git a/synapse/logging/opentracing.py b/synapse/logging/opentracing.py
index c1aa205eed..fa3f76c27f 100644
--- a/synapse/logging/opentracing.py
+++ b/synapse/logging/opentracing.py
@@ -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:
@@ -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)
|
