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

Optimise LoggingContext creation and copying #2792

Merged
merged 2 commits into from
Jan 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions synapse/util/logcontext.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,16 @@ def get_thread_resource_usage():
class LoggingContext(object):
"""Additional context for log formatting. Contexts are scoped within a
"with" block.

Args:
name (str): Name for the context for debugging.
"""

__slots__ = [
"previous_context", "name", "usage_start", "usage_end", "main_thread",
"__dict__", "tag", "alive",
"previous_context", "name", "ru_stime", "ru_utime",
"db_txn_count", "db_txn_duration", "usage_start", "usage_end",
"main_thread", "alive",
"request", "tag",
]

thread_local = threading.local()
Expand Down Expand Up @@ -96,7 +99,9 @@ def __init__(self, name=None):
self.db_txn_count = 0
self.db_txn_duration = 0.
self.usage_start = None
self.usage_end = None
self.main_thread = threading.current_thread()
self.request = None
self.tag = ""
self.alive = True

Expand All @@ -105,7 +110,11 @@ def __str__(self):

@classmethod
def current_context(cls):
"""Get the current logging context from thread local storage"""
"""Get the current logging context from thread local storage

Returns:
LoggingContext: the current logging context
"""
return getattr(cls.thread_local, "current_context", cls.sentinel)

@classmethod
Expand Down Expand Up @@ -155,11 +164,13 @@ def __exit__(self, type, value, traceback):
self.alive = False

def copy_to(self, record):
"""Copy fields from this context to the record"""
for key, value in self.__dict__.items():
setattr(record, key, value)
"""Copy logging fields from this context to a log record or
another LoggingContext
"""

record.ru_utime, record.ru_stime = self.get_resource_usage()
# 'request' is the only field we currently use in the logger, so that's
# all we need to copy
record.request = self.request

def start(self):
if threading.current_thread() is not self.main_thread:
Expand Down
14 changes: 7 additions & 7 deletions tests/crypto/test_keyring.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def setUp(self):

def check_context(self, _, expected):
self.assertEquals(
getattr(LoggingContext.current_context(), "test_key", None),
getattr(LoggingContext.current_context(), "request", None),
expected
)

Expand All @@ -82,7 +82,7 @@ def test_wait_for_previous_lookups(self):
lookup_2_deferred = defer.Deferred()

with LoggingContext("one") as context_one:
context_one.test_key = "one"
context_one.request = "one"

wait_1_deferred = kr.wait_for_previous_lookups(
["server1"],
Expand All @@ -96,7 +96,7 @@ def test_wait_for_previous_lookups(self):
wait_1_deferred.addBoth(self.check_context, "one")

with LoggingContext("two") as context_two:
context_two.test_key = "two"
context_two.request = "two"

# set off another wait. It should block because the first lookup
# hasn't yet completed.
Expand Down Expand Up @@ -137,15 +137,15 @@ def test_verify_json_objects_for_server_awaits_previous_requests(self):
@defer.inlineCallbacks
def get_perspectives(**kwargs):
self.assertEquals(
LoggingContext.current_context().test_key, "11",
LoggingContext.current_context().request, "11",
)
with logcontext.PreserveLoggingContext():
yield persp_deferred
defer.returnValue(persp_resp)
self.http_client.post_json.side_effect = get_perspectives

with LoggingContext("11") as context_11:
context_11.test_key = "11"
context_11.request = "11"

# start off a first set of lookups
res_deferreds = kr.verify_json_objects_for_server(
Expand Down Expand Up @@ -173,7 +173,7 @@ def get_perspectives(**kwargs):
self.assertIs(LoggingContext.current_context(), context_11)

context_12 = LoggingContext("12")
context_12.test_key = "12"
context_12.request = "12"
with logcontext.PreserveLoggingContext(context_12):
# a second request for a server with outstanding requests
# should block rather than start a second call
Expand Down Expand Up @@ -211,7 +211,7 @@ def test_verify_json_for_server(self):
sentinel_context = LoggingContext.current_context()

with LoggingContext("one") as context_one:
context_one.test_key = "one"
context_one.request = "one"

defer = kr.verify_json_for_server("server9", {})
try:
Expand Down
16 changes: 8 additions & 8 deletions tests/util/test_logcontext.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,27 @@ class LoggingContextTestCase(unittest.TestCase):

def _check_test_key(self, value):
self.assertEquals(
LoggingContext.current_context().test_key, value
LoggingContext.current_context().request, value
)

def test_with_context(self):
with LoggingContext() as context_one:
context_one.test_key = "test"
context_one.request = "test"
self._check_test_key("test")

@defer.inlineCallbacks
def test_sleep(self):
@defer.inlineCallbacks
def competing_callback():
with LoggingContext() as competing_context:
competing_context.test_key = "competing"
competing_context.request = "competing"
yield sleep(0)
self._check_test_key("competing")

reactor.callLater(0, competing_callback)

with LoggingContext() as context_one:
context_one.test_key = "one"
context_one.request = "one"
yield sleep(0)
self._check_test_key("one")

Expand All @@ -43,14 +43,14 @@ def _test_preserve_fn(self, function):

@defer.inlineCallbacks
def cb():
context_one.test_key = "one"
context_one.request = "one"
yield function()
self._check_test_key("one")

callback_completed[0] = True

with LoggingContext() as context_one:
context_one.test_key = "one"
context_one.request = "one"

# fire off function, but don't wait on it.
logcontext.preserve_fn(cb)()
Expand Down Expand Up @@ -107,7 +107,7 @@ def blocking_function():
sentinel_context = LoggingContext.current_context()

with LoggingContext() as context_one:
context_one.test_key = "one"
context_one.request = "one"

d1 = logcontext.make_deferred_yieldable(blocking_function())
# make sure that the context was reset by make_deferred_yieldable
Expand All @@ -124,7 +124,7 @@ def test_make_deferred_yieldable_on_non_deferred(self):
argument isn't actually a deferred"""

with LoggingContext() as context_one:
context_one.test_key = "one"
context_one.request = "one"

d1 = logcontext.make_deferred_yieldable("bum")
self._check_test_key("one")
Expand Down