Skip to content

Commit f0aae62

Browse files
Cheaper logcontext debug logs (random_string_insecure_fast(...)) (#19094)
Follow-up to #18966 During the weekly Backend team meeting, it was mentioned that `random_string(...)` was taking a significant amount of CPU on `matrix.org`. This makes sense as it relies on [`secrets.choice(...)`](https://docs.python.org/3/library/secrets.html#secrets.choice), a cryptographically secure function that is inherently computationally expensive. And since #18966, we're calling `random_string(...)` as part of a bunch of logcontext utilities. Since we don't need cryptographically secure random strings for our debug logs, this PR is introducing a new `random_string_insecure_fast(...)` function that uses [`random.choice(...)`](https://docs.python.org/3/library/random.html#random.choice) which uses pseudo-random numbers that are "both fast and threadsafe".
1 parent 3495991 commit f0aae62

File tree

3 files changed

+22
-4
lines changed

3 files changed

+22
-4
lines changed

changelog.d/19094.misc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Use cheaper random string function in logcontext utilities.

synapse/logging/context.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
from twisted.python.threadpool import ThreadPool
5454

5555
from synapse.logging.loggers import ExplicitlyConfiguredLogger
56-
from synapse.util.stringutils import random_string
56+
from synapse.util.stringutils import random_string_insecure_fast
5757

5858
if TYPE_CHECKING:
5959
from synapse.logging.scopecontextmanager import _LogContextScope
@@ -657,7 +657,7 @@ def __init__(
657657
self, new_context: LoggingContextOrSentinel = SENTINEL_CONTEXT
658658
) -> None:
659659
self._new_context = new_context
660-
self._instance_id = random_string(5)
660+
self._instance_id = random_string_insecure_fast(5)
661661

662662
def __enter__(self) -> None:
663663
logcontext_debug_logger.debug(
@@ -859,7 +859,7 @@ def run_in_background(
859859
Note that the returned Deferred does not follow the synapse logcontext
860860
rules.
861861
"""
862-
instance_id = random_string(5)
862+
instance_id = random_string_insecure_fast(5)
863863
calling_context = current_context()
864864
logcontext_debug_logger.debug(
865865
"run_in_background(%s): called with logcontext=%s", instance_id, calling_context
@@ -1012,7 +1012,7 @@ def make_deferred_yieldable(deferred: "defer.Deferred[T]") -> "defer.Deferred[T]
10121012
restores the old context once the awaitable completes (execution passes from the
10131013
reactor back to the code).
10141014
"""
1015-
instance_id = random_string(5)
1015+
instance_id = random_string_insecure_fast(5)
10161016
logcontext_debug_logger.debug(
10171017
"make_deferred_yieldable(%s): called with logcontext=%s",
10181018
instance_id,

synapse/util/stringutils.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#
2121
#
2222
import itertools
23+
import random
2324
import re
2425
import secrets
2526
import string
@@ -56,6 +57,10 @@ def random_string(length: int) -> str:
5657
"""Generate a cryptographically secure string of random letters.
5758
5859
Drawn from the characters: `a-z` and `A-Z`
60+
61+
Because this is generated from cryptographic sources, it takes a notable amount of
62+
effort to generate (computationally expensive). If you don't need cryptographic
63+
security, consider using `random_string_insecure_fast` for better performance.
5964
"""
6065
return "".join(secrets.choice(string.ascii_letters) for _ in range(length))
6166

@@ -68,6 +73,18 @@ def random_string_with_symbols(length: int) -> str:
6873
return "".join(secrets.choice(_string_with_symbols) for _ in range(length))
6974

7075

76+
def random_string_insecure_fast(length: int) -> str:
77+
"""
78+
Generate a string of random letters (insecure, fast). This is a more performant but
79+
insecure version of `random_string`.
80+
81+
WARNING: Not for security or cryptographic uses. Use `random_string` instead.
82+
83+
Drawn from the characters: `a-z` and `A-Z`
84+
"""
85+
return "".join(random.choice(string.ascii_letters) for _ in range(length))
86+
87+
7188
def is_ascii(s: bytes) -> bool:
7289
try:
7390
s.decode("ascii").encode("ascii")

0 commit comments

Comments
 (0)