Skip to content

Commit e8710e7

Browse files
authored
Don't include debug logs in Clock unless explicitly enabled (#19278)
Fixes #19276 This log with stack traces results in a ton of noise in the logs and is confusing to users since it looks like it's an error in the logs. This PR removes the stack trace from the log. This can be re-enabled on demand if it is deemed necessary in the future. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [X] Pull request is based on the develop branch * [X] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [X] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
1 parent 978ae0b commit e8710e7

File tree

2 files changed

+33
-10
lines changed

2 files changed

+33
-10
lines changed

changelog.d/19278.misc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Don't include debug logs in `Clock` unless explicitly enabled.

synapse/util/clock.py

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from twisted.internet.task import LoopingCall
3030

3131
from synapse.logging import context
32+
from synapse.logging.loggers import ExplicitlyConfiguredLogger
3233
from synapse.types import ISynapseThreadlessReactor
3334
from synapse.util import log_failure
3435
from synapse.util.duration import Duration
@@ -39,6 +40,25 @@
3940

4041
logger = logging.getLogger(__name__)
4142

43+
original_logger_class = logging.getLoggerClass()
44+
logging.setLoggerClass(ExplicitlyConfiguredLogger)
45+
clock_debug_logger = logging.getLogger("synapse.util.clock.debug")
46+
"""
47+
A logger for debugging what is scheduling calls.
48+
49+
Ideally, these wouldn't be gated behind an `ExplicitlyConfiguredLogger` as including logs
50+
from this logger would be helpful to track when things are being scheduled. However, for
51+
these logs to be meaningful, they need to include a stack trace to show what initiated the
52+
call in the first place.
53+
54+
Since the stack traces can create a lot of noise and make the logs hard to read (unless you're
55+
specifically debugging scheduling issues) we want users to opt-in to seeing these logs. To enable
56+
this, they must explicitly set `synapse.util.clock.debug` in the logging configuration. Note that
57+
this setting won't inherit the log level from the parent logger.
58+
"""
59+
# Restore the original logger class
60+
logging.setLoggerClass(original_logger_class)
61+
4262

4363
class Clock:
4464
"""
@@ -174,7 +194,7 @@ def _looping_call_common(
174194
looping_call_context_string = "looping_call_now"
175195

176196
def wrapped_f(*args: P.args, **kwargs: P.kwargs) -> Deferred:
177-
logger.debug(
197+
clock_debug_logger.debug(
178198
"%s(%s): Executing callback", looping_call_context_string, instance_id
179199
)
180200

@@ -222,7 +242,7 @@ def wrapped_f(*args: P.args, **kwargs: P.kwargs) -> Deferred:
222242
d.addErrback(log_failure, "Looping call died", consumeErrors=False)
223243
self._looping_calls.append(call)
224244

225-
logger.debug(
245+
clock_debug_logger.debug(
226246
"%s(%s): Scheduled looping call every %sms later",
227247
looping_call_context_string,
228248
instance_id,
@@ -283,7 +303,7 @@ def call_later(
283303
raise Exception("Cannot start delayed call. Clock has been shutdown")
284304

285305
def wrapped_callback(*args: Any, **kwargs: Any) -> None:
286-
logger.debug("call_later(%s): Executing callback", call_id)
306+
clock_debug_logger.debug("call_later(%s): Executing callback", call_id)
287307

288308
assert context.current_context() is context.SENTINEL_CONTEXT, (
289309
"Expected `call_later` callback from the reactor to start with the sentinel logcontext "
@@ -327,7 +347,7 @@ def wrapped_callback(*args: Any, **kwargs: Any) -> None:
327347
delay.as_secs(), wrapped_callback, *args, **kwargs
328348
) # type: ignore[call-later-not-tracked]
329349

330-
logger.debug(
350+
clock_debug_logger.debug(
331351
"call_later(%s): Scheduled call for %ss later (tracked for shutdown: %s)",
332352
call_id,
333353
delay,
@@ -347,7 +367,7 @@ def cancel_call_later(
347367
self, wrapped_call: "DelayedCallWrapper", ignore_errs: bool = False
348368
) -> None:
349369
try:
350-
logger.debug(
370+
clock_debug_logger.debug(
351371
"cancel_call_later: cancelling scheduled call %s", wrapped_call.call_id
352372
)
353373
wrapped_call.delayed_call.cancel()
@@ -367,7 +387,7 @@ def cancel_all_delayed_calls(self, ignore_errs: bool = True) -> None:
367387
# will result in the call removing itself from the map mid-iteration.
368388
for call_id, call in list(self._call_id_to_delayed_call.items()):
369389
try:
370-
logger.debug(
390+
clock_debug_logger.debug(
371391
"cancel_all_delayed_calls: cancelling scheduled call %s", call_id
372392
)
373393
call.cancel()
@@ -396,7 +416,9 @@ def call_when_running(
396416
instance_id = random_string_insecure_fast(5)
397417

398418
def wrapped_callback(*args: Any, **kwargs: Any) -> None:
399-
logger.debug("call_when_running(%s): Executing callback", instance_id)
419+
clock_debug_logger.debug(
420+
"call_when_running(%s): Executing callback", instance_id
421+
)
400422

401423
# Since this callback can be invoked immediately if the reactor is already
402424
# running, we can't always assume that we're running in the sentinel
@@ -436,7 +458,7 @@ def wrapped_callback(*args: Any, **kwargs: Any) -> None:
436458
# callWhenRunning should be called.
437459
self._reactor.callWhenRunning(wrapped_callback, *args, **kwargs) # type: ignore[prefer-synapse-clock-call-when-running]
438460

439-
logger.debug(
461+
clock_debug_logger.debug(
440462
"call_when_running(%s): Scheduled call",
441463
instance_id,
442464
# Find out who is scheduling the call which makes it easy to follow in the
@@ -472,7 +494,7 @@ def add_system_event_trigger(
472494
instance_id = random_string_insecure_fast(5)
473495

474496
def wrapped_callback(*args: Any, **kwargs: Any) -> None:
475-
logger.debug(
497+
clock_debug_logger.debug(
476498
"add_system_event_trigger(%s): Executing %s %s callback",
477499
instance_id,
478500
phase,
@@ -509,7 +531,7 @@ def wrapped_callback(*args: Any, **kwargs: Any) -> None:
509531
# logcontext to the reactor
510532
context.run_in_background(callback, *args, **kwargs)
511533

512-
logger.debug(
534+
clock_debug_logger.debug(
513535
"add_system_event_trigger(%s) for %s %s",
514536
instance_id,
515537
phase,

0 commit comments

Comments
 (0)