Skip to content

Commit 9748e97

Browse files
devonhjason-famedly
authored andcommitted
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 e08f4f2 commit 9748e97

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.stringutils import random_string_insecure_fast
@@ -38,6 +39,25 @@
3839

3940
logger = logging.getLogger(__name__)
4041

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

4262
class Clock:
4363
"""
@@ -173,7 +193,7 @@ def _looping_call_common(
173193
looping_call_context_string = "looping_call_now"
174194

175195
def wrapped_f(*args: P.args, **kwargs: P.kwargs) -> Deferred:
176-
logger.debug(
196+
clock_debug_logger.debug(
177197
"%s(%s): Executing callback", looping_call_context_string, instance_id
178198
)
179199

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

224-
logger.debug(
244+
clock_debug_logger.debug(
225245
"%s(%s): Scheduled looping call every %sms later",
226246
looping_call_context_string,
227247
instance_id,
@@ -282,7 +302,7 @@ def call_later(
282302
raise Exception("Cannot start delayed call. Clock has been shutdown")
283303

284304
def wrapped_callback(*args: Any, **kwargs: Any) -> None:
285-
logger.debug("call_later(%s): Executing callback", call_id)
305+
clock_debug_logger.debug("call_later(%s): Executing callback", call_id)
286306

287307
assert context.current_context() is context.SENTINEL_CONTEXT, (
288308
"Expected `call_later` callback from the reactor to start with the sentinel logcontext "
@@ -324,7 +344,7 @@ def wrapped_callback(*args: Any, **kwargs: Any) -> None:
324344
# be called.
325345
call = self._reactor.callLater(delay, wrapped_callback, *args, **kwargs) # type: ignore[call-later-not-tracked]
326346

327-
logger.debug(
347+
clock_debug_logger.debug(
328348
"call_later(%s): Scheduled call for %ss later (tracked for shutdown: %s)",
329349
call_id,
330350
delay,
@@ -344,7 +364,7 @@ def cancel_call_later(
344364
self, wrapped_call: "DelayedCallWrapper", ignore_errs: bool = False
345365
) -> None:
346366
try:
347-
logger.debug(
367+
clock_debug_logger.debug(
348368
"cancel_call_later: cancelling scheduled call %s", wrapped_call.call_id
349369
)
350370
wrapped_call.delayed_call.cancel()
@@ -364,7 +384,7 @@ def cancel_all_delayed_calls(self, ignore_errs: bool = True) -> None:
364384
# will result in the call removing itself from the map mid-iteration.
365385
for call_id, call in list(self._call_id_to_delayed_call.items()):
366386
try:
367-
logger.debug(
387+
clock_debug_logger.debug(
368388
"cancel_all_delayed_calls: cancelling scheduled call %s", call_id
369389
)
370390
call.cancel()
@@ -393,7 +413,9 @@ def call_when_running(
393413
instance_id = random_string_insecure_fast(5)
394414

395415
def wrapped_callback(*args: Any, **kwargs: Any) -> None:
396-
logger.debug("call_when_running(%s): Executing callback", instance_id)
416+
clock_debug_logger.debug(
417+
"call_when_running(%s): Executing callback", instance_id
418+
)
397419

398420
# Since this callback can be invoked immediately if the reactor is already
399421
# running, we can't always assume that we're running in the sentinel
@@ -433,7 +455,7 @@ def wrapped_callback(*args: Any, **kwargs: Any) -> None:
433455
# callWhenRunning should be called.
434456
self._reactor.callWhenRunning(wrapped_callback, *args, **kwargs) # type: ignore[prefer-synapse-clock-call-when-running]
435457

436-
logger.debug(
458+
clock_debug_logger.debug(
437459
"call_when_running(%s): Scheduled call",
438460
instance_id,
439461
# Find out who is scheduling the call which makes it easy to follow in the
@@ -469,7 +491,7 @@ def add_system_event_trigger(
469491
instance_id = random_string_insecure_fast(5)
470492

471493
def wrapped_callback(*args: Any, **kwargs: Any) -> None:
472-
logger.debug(
494+
clock_debug_logger.debug(
473495
"add_system_event_trigger(%s): Executing %s %s callback",
474496
instance_id,
475497
phase,
@@ -506,7 +528,7 @@ def wrapped_callback(*args: Any, **kwargs: Any) -> None:
506528
# logcontext to the reactor
507529
context.run_in_background(callback, *args, **kwargs)
508530

509-
logger.debug(
531+
clock_debug_logger.debug(
510532
"add_system_event_trigger(%s) for %s %s",
511533
instance_id,
512534
phase,

0 commit comments

Comments
 (0)