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

Implement a structured logging output system. #5680

Merged
merged 61 commits into from
Aug 28, 2019
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
4071317
first cut of structured logging
hawkowl Jul 13, 2019
0ea4fb1
remove deprecated options, command line options, and simplify the log…
hawkowl Jul 13, 2019
af7bc1c
this should be true
hawkowl Jul 13, 2019
83bf10c
cleanup
hawkowl Jul 13, 2019
9c19dc3
remove disused arguments from workers too
hawkowl Jul 13, 2019
c19f2d5
Merge branch 'hawkowl/prune-logging-options' into hawkowl/structured-…
hawkowl Jul 13, 2019
32b9dd0
fix and push up
hawkowl Jul 13, 2019
83507eb
fix
hawkowl Jul 13, 2019
5cec494
update to use 18.9
hawkowl Jul 13, 2019
69cd070
fix sytests?
hawkowl Jul 15, 2019
51048fb
fix sytests?
hawkowl Jul 15, 2019
0b76783
make a misc
hawkowl Jul 15, 2019
9f5d825
Merge remote-tracking branch 'origin/develop' into hawkowl/structured…
hawkowl Jul 18, 2019
47820c5
fix
hawkowl Jul 18, 2019
5260a8b
fix
hawkowl Jul 18, 2019
586fd94
fix
hawkowl Jul 18, 2019
50fcd60
fix
hawkowl Jul 18, 2019
534b15d
Merge remote-tracking branch 'origin/develop' into hawkowl/structured…
hawkowl Jul 23, 2019
ae34f25
review comments
hawkowl Jul 23, 2019
34b3fce
review comments
hawkowl Jul 23, 2019
ba5b355
use native python read
hawkowl Jul 23, 2019
64c5cdb
Merge remote-tracking branch 'origin/develop' into hawkowl/structured…
hawkowl Jul 26, 2019
d1fc0ff
set the log level on the namespace too
hawkowl Jul 26, 2019
fa764c6
review comments
hawkowl Jul 26, 2019
b19156a
Merge remote-tracking branch 'origin/develop' into hawkowl/structured…
hawkowl Jul 29, 2019
6b4a540
Merge remote-tracking branch 'origin/develop' into hawkowl/structured…
hawkowl Jul 29, 2019
2fbc0e1
Merge remote-tracking branch 'origin/develop' into hawkowl/structured…
hawkowl Jul 30, 2019
aafc342
fix
hawkowl Jul 31, 2019
cdb8a72
Merge remote-tracking branch 'origin/develop' into hawkowl/structured…
hawkowl Jul 31, 2019
755d7e8
more test
hawkowl Jul 31, 2019
fb9b18a
Merge remote-tracking branch 'origin/develop' into hawkowl/structured…
hawkowl Aug 7, 2019
680fdee
fix
hawkowl Aug 7, 2019
db1b1b3
tests
hawkowl Aug 7, 2019
92289a7
fix
hawkowl Aug 7, 2019
e7c62b5
pass in a proper lang
hawkowl Aug 7, 2019
55391d2
try this to see if it'll go faster
hawkowl Aug 7, 2019
b2fa4ad
docs:
hawkowl Aug 7, 2019
9cf3c28
docs
hawkowl Aug 7, 2019
673d380
docs
hawkowl Aug 7, 2019
aae792d
more test
hawkowl Aug 7, 2019
b50d3a9
more test
hawkowl Aug 7, 2019
03ac5b4
fluentd log to export on console
hawkowl Aug 8, 2019
d10b9b8
fix
hawkowl Aug 8, 2019
51b3e81
fix
hawkowl Aug 8, 2019
232064e
cleanups
hawkowl Aug 8, 2019
d004e93
style fix
hawkowl Aug 8, 2019
5bbc8e7
docs and typing
hawkowl Aug 9, 2019
b611e53
run mypy and check for errors
hawkowl Aug 9, 2019
77f2334
run mypy and check for errors
hawkowl Aug 9, 2019
eb3c75d
add it to CI
hawkowl Aug 9, 2019
71d695a
docs
hawkowl Aug 9, 2019
589463f
Merge remote-tracking branch 'origin/develop' into hawkowl/structured…
hawkowl Aug 27, 2019
04f084c
some introductory logs
hawkowl Aug 27, 2019
8697d51
failing test
hawkowl Aug 27, 2019
4477c69
failing test
hawkowl Aug 27, 2019
2684627
Merge remote-tracking branch 'origin/develop' into hawkowl/structured…
hawkowl Aug 28, 2019
9864220
fix the test
hawkowl Aug 28, 2019
c400d15
fix the tests
hawkowl Aug 28, 2019
267501a
fix mypy
hawkowl Aug 28, 2019
518e48e
fix old tests
hawkowl Aug 28, 2019
0a0845f
fix old tests
hawkowl Aug 28, 2019
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ _trial_temp*/
/*.signing.key
/env/
/homeserver*.yaml
/logs
/media_store/
/uploads

Expand Down
1 change: 1 addition & 0 deletions changelog.d/5680.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Lay the groundwork for structured logging output.
12 changes: 7 additions & 5 deletions synapse/app/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,20 @@

logger = logging.getLogger(__name__)

# list of tuples of function, args list, kwargs dict
_sighup_callbacks = []


def register_sighup(func):
def register_sighup(func, *args, **kwargs):
"""
Register a function to be called when a SIGHUP occurs.

Args:
func (function): Function to be called when sent a SIGHUP signal.
Will be called with a single argument, the homeserver.
Will be called with a single default argument, the homeserver.
*args, **kwargs: args and kwargs to be passed to the target function.
"""
_sighup_callbacks.append(func)
_sighup_callbacks.append((func, args, kwargs))


def start_worker_reactor(appname, config, run_command=reactor.run):
Expand Down Expand Up @@ -249,8 +251,8 @@ def handle_sighup(*args, **kwargs):
sd_channel = sdnotify.SystemdNotifier()
sd_channel.notify("RELOADING=1")

for i in _sighup_callbacks:
i(hs)
for i, args, kwargs in _sighup_callbacks:
i(hs, *args, **kwargs)

sd_channel.notify("READY=1")

Expand Down
103 changes: 61 additions & 42 deletions synapse/config/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@

import synapse
from synapse.app import _base as appbase
from synapse.logging._structured import (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

surely we shouldn't be importing from _ modules?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not an internally public utility API, so the _ is warranted

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's not an internally public utility API, you shouldn't be calling it.

Sorry, I'm really not seeing what makes this different from anything else in the synapse codebase. (I'm aware there's a _ import on the line above. That one's misguided too.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is pretty bog standard python naming convention https://pep8.org/#descriptive-naming-styles

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So are you saying that this is different to the rest of synapse (if so, how?), or that the rest of synapse is doing it wrong?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, it seems like a trivial point to be arguing over, but unless there's an understanding of whether there should be an underscore or not, we're going to end up with a mix?

reload_structured_logging,
setup_structured_logging,
)
from synapse.logging.context import LoggingContextFilter
from synapse.util.versionstring import get_version_string

Expand Down Expand Up @@ -119,21 +123,10 @@ def generate_files(self, config, config_dir_path):
log_config_file.write(DEFAULT_LOG_CONFIG.substitute(log_file=log_file))


def setup_logging(config, use_worker_options=False):
""" Set up python logging

Args:
config (LoggingConfig | synapse.config.workers.WorkerConfig):
configuration data

use_worker_options (bool): True to use the 'worker_log_config' option
instead of 'log_config'.

register_sighup (func | None): Function to call to register a
sighup handler.
def _setup_stdlib_logging(config, log_config):
"""
Set up Python stdlib logging.
"""
log_config = config.worker_log_config if use_worker_options else config.log_config

if log_config is None:
log_format = (
"%(asctime)s - %(name)s - %(lineno)d - %(levelname)s - %(request)s"
Expand All @@ -151,35 +144,10 @@ def setup_logging(config, use_worker_options=False):
handler.addFilter(LoggingContextFilter(request=""))
logger.addHandler(handler)
else:
logging.config.dictConfig(log_config)

def load_log_config():
with open(log_config, "r") as f:
logging.config.dictConfig(yaml.safe_load(f))

def sighup(*args):
# it might be better to use a file watcher or something for this.
load_log_config()
logging.info("Reloaded log config from %s due to SIGHUP", log_config)

load_log_config()
appbase.register_sighup(sighup)

# make sure that the first thing we log is a thing we can grep backwards
# for
logging.warn("***** STARTING SERVER *****")
logging.warn("Server %s version %s", sys.argv[0], get_version_string(synapse))
logging.info("Server hostname: %s", config.server_name)

# It's critical to point twisted's internal logging somewhere, otherwise it
# stacks up and leaks kup to 64K object;
# see: https://twistedmatrix.com/trac/ticket/8164
#
# Routing to the python logging framework could be a performance problem if
# the handlers blocked for a long time as python.logging is a blocking API
# see https://twistedmatrix.com/documents/current/core/howto/logger.html
# filed as https://github.com/matrix-org/synapse/issues/1727
#
# However this may not be too much of a problem if we are just writing to a file.
# Route Twisted's native logging through to the standard library logging
# system.
observer = STDLibLogObserver()

def _log(event):
Expand All @@ -201,3 +169,54 @@ def _log(event):
)
if not config.no_redirect_stdio:
print("Redirected stdout/stderr to logs")


def _reload_stdlib_logging(*args, log_config=None):
logger = logging.getLogger("")

if not log_config:
logger.warn("Reloaded a blank config?")

logging.config.dictConfig(log_config)


def setup_logging(config, use_worker_options=False):
"""
Set up the logging subsystem.

Args:
config (LoggingConfig | synapse.config.workers.WorkerConfig):
configuration data

use_worker_options (bool): True to use the 'worker_log_config' option
instead of 'log_config'.
"""
log_config = config.worker_log_config if use_worker_options else config.log_config

def read_config(*args, callback=None):
if log_config is None:
return None

with open(log_config, "rb") as f:
log_config_body = yaml.safe_load(f.read())

if callback:
callback(log_config=log_config_body)
logging.info("Reloaded log config from %s due to SIGHUP", log_config)

return log_config_body

log_config_body = read_config()

if log_config_body and log_config_body.get("version") == 2:
hawkowl marked this conversation as resolved.
Show resolved Hide resolved
setup_structured_logging(config, log_config_body)
appbase.register_sighup(read_config, callback=reload_structured_logging)
else:
_setup_stdlib_logging(config, log_config_body)
appbase.register_sighup(read_config, callback=_reload_stdlib_logging)

# make sure that the first thing we log is a thing we can grep backwards
# for
logging.warn("***** STARTING SERVER *****")
logging.warn("Server %s version %s", sys.argv[0], get_version_string(synapse))
logging.info("Server hostname: %s", config.server_name)
Loading