From 0ea4fb1b857ae7091117f7b69e778ebcbd6e8455 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Sun, 14 Jul 2019 01:06:29 +1000 Subject: [PATCH 1/7] remove deprecated options, command line options, and simplify the logging code in preparation for structured logging changes --- changelog.d/5678.misc | 1 + changelog.d/5678.removal | 1 + synapse/config/logger.py | 100 +++++---------------------------------- 3 files changed, 13 insertions(+), 89 deletions(-) create mode 100644 changelog.d/5678.misc create mode 100644 changelog.d/5678.removal diff --git a/changelog.d/5678.misc b/changelog.d/5678.misc new file mode 100644 index 000000000000..2d9a19ad23bf --- /dev/null +++ b/changelog.d/5678.misc @@ -0,0 +1 @@ +Synapse will now always redirect standard I/O to the logging system. diff --git a/changelog.d/5678.removal b/changelog.d/5678.removal new file mode 100644 index 000000000000..2ad86a5e701c --- /dev/null +++ b/changelog.d/5678.removal @@ -0,0 +1 @@ +Synapse now no longer accepts the `-v`/`--verbose`, `-f`/`--log-file`, `-n`/`--no-redirect-stdio`, or `--log-config` command line flags, and removes the deprecated `verbose`, `no_redirect_stdio`, and `log_file` configuration file options. Users of these options should migrate their options into the dedicated log configuration. diff --git a/synapse/config/logger.py b/synapse/config/logger.py index 0f5554211c6a..a21d9fcba8bd 100644 --- a/synapse/config/logger.py +++ b/synapse/config/logger.py @@ -75,10 +75,7 @@ class LoggingConfig(Config): def read_config(self, config, **kwargs): - self.verbosity = config.get("verbose", 0) - self.no_redirect_stdio = config.get("no_redirect_stdio", False) self.log_config = self.abspath(config.get("log_config")) - self.log_file = self.abspath(config.get("log_file")) def generate_config_section(self, config_dir_path, server_name, **kwargs): log_config = os.path.join(config_dir_path, server_name + ".log.config") @@ -93,46 +90,6 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): % locals() ) - def read_arguments(self, args): - if args.verbose is not None: - self.verbosity = args.verbose - if args.no_redirect_stdio is not None: - self.no_redirect_stdio = args.no_redirect_stdio - if args.log_config is not None: - self.log_config = args.log_config - if args.log_file is not None: - self.log_file = args.log_file - - def add_arguments(cls, parser): - logging_group = parser.add_argument_group("logging") - logging_group.add_argument( - "-v", - "--verbose", - dest="verbose", - action="count", - help="The verbosity level. Specify multiple times to increase " - "verbosity. (Ignored if --log-config is specified.)", - ) - logging_group.add_argument( - "-f", - "--log-file", - dest="log_file", - help="File to log to. (Ignored if --log-config is specified.)", - ) - logging_group.add_argument( - "--log-config", - dest="log_config", - default=None, - help="Python logging config file", - ) - logging_group.add_argument( - "-n", - "--no-redirect-stdio", - action="store_true", - default=None, - help="Do not redirect stdout/stderr to the log", - ) - def generate_files(self, config, config_dir_path): log_config = config.get("log_config") if log_config and not os.path.exists(log_config): @@ -152,61 +109,31 @@ def setup_logging(config, use_worker_options=False): config (LoggingConfig | synapse.config.workers.WorkerConfig): configuration data - use_worker_options (bool): True to use 'worker_log_config' and - 'worker_log_file' options instead of 'log_config' and 'log_file'. + 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. """ log_config = config.worker_log_config if use_worker_options else config.log_config - log_file = config.worker_log_file if use_worker_options else config.log_file - - log_format = ( - "%(asctime)s - %(name)s - %(lineno)d - %(levelname)s - %(request)s" - " - %(message)s" - ) if log_config is None: - # We don't have a logfile, so fall back to the 'verbosity' param from - # the config or cmdline. (Note that we generate a log config for new - # installs, so this will be an unusual case) - level = logging.INFO - level_for_storage = logging.INFO - if config.verbosity: - level = logging.DEBUG - if config.verbosity > 1: - level_for_storage = logging.DEBUG + log_format = ( + "%(asctime)s - %(name)s - %(lineno)d - %(levelname)s - %(request)s" + " - %(message)s" + ) logger = logging.getLogger("") - logger.setLevel(level) - - logging.getLogger("synapse.storage.SQL").setLevel(level_for_storage) + logger.setLevel(logging.INFO) + logging.getLogger("synapse.storage.SQL").setLevel(logging.INFO) formatter = logging.Formatter(log_format) - if log_file: - # TODO: Customisable file size / backup count - handler = logging.handlers.RotatingFileHandler( - log_file, maxBytes=(1000 * 1000 * 100), backupCount=3, encoding="utf8" - ) - - def sighup(signum, stack): - logger.info("Closing log file due to SIGHUP") - handler.doRollover() - logger.info("Opened new log file due to SIGHUP") - - else: - handler = logging.StreamHandler() - - def sighup(*args): - pass + handler = logging.StreamHandler() handler.setFormatter(formatter) - handler.addFilter(LoggingContextFilter(request="")) - logger.addHandler(handler) else: - def load_log_config(): with open(log_config, "r") as f: logging.config.dictConfig(yaml.safe_load(f)) @@ -217,8 +144,7 @@ def sighup(*args): logging.info("Reloaded log config from %s due to SIGHUP", log_config) load_log_config() - - appbase.register_sighup(sighup) + appbase.register_sighup(sighup) # make sure that the first thing we log is a thing we can grep backwards # for @@ -252,8 +178,4 @@ def _log(event): return observer(event) - globalLogBeginner.beginLoggingTo( - [_log], redirectStandardIO=not config.no_redirect_stdio - ) - if not config.no_redirect_stdio: - print("Redirected stdout/stderr to logs") + globalLogBeginner.beginLoggingTo([_log], redirectStandardIO=False) From af7bc1c7f4d3155b71663d3f24aa1be7aad06e74 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Sun, 14 Jul 2019 01:12:45 +1000 Subject: [PATCH 2/7] this should be true --- synapse/config/logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/logger.py b/synapse/config/logger.py index a21d9fcba8bd..8c54627e3b0a 100644 --- a/synapse/config/logger.py +++ b/synapse/config/logger.py @@ -178,4 +178,4 @@ def _log(event): return observer(event) - globalLogBeginner.beginLoggingTo([_log], redirectStandardIO=False) + globalLogBeginner.beginLoggingTo([_log], redirectStandardIO=True) From 83bf10c7b0ecad7bc26121f6c367a88ac169e270 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Sun, 14 Jul 2019 01:13:08 +1000 Subject: [PATCH 3/7] cleanup --- synapse/config/logger.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/config/logger.py b/synapse/config/logger.py index 8c54627e3b0a..2a326776fb25 100644 --- a/synapse/config/logger.py +++ b/synapse/config/logger.py @@ -134,6 +134,7 @@ def setup_logging(config, use_worker_options=False): handler.addFilter(LoggingContextFilter(request="")) logger.addHandler(handler) else: + def load_log_config(): with open(log_config, "r") as f: logging.config.dictConfig(yaml.safe_load(f)) From 9c19dc3204bc50122aa78b2d2e83b79c139c4778 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Sun, 14 Jul 2019 01:21:17 +1000 Subject: [PATCH 4/7] remove disused arguments from workers too --- synapse/config/workers.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 3b75471d8585..246d72cd611b 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -31,8 +31,6 @@ def read_config(self, config, **kwargs): self.worker_listeners = config.get("worker_listeners", []) self.worker_daemonize = config.get("worker_daemonize") self.worker_pid_file = config.get("worker_pid_file") - self.worker_log_file = config.get("worker_log_file") - self.worker_log_config = config.get("worker_log_config") # The host used to connect to the main synapse self.worker_replication_host = config.get("worker_replication_host", None) @@ -78,9 +76,5 @@ def read_arguments(self, args): if args.daemonize is not None: self.worker_daemonize = args.daemonize - if args.log_config is not None: - self.worker_log_config = args.log_config - if args.log_file is not None: - self.worker_log_file = args.log_file if args.manhole is not None: self.worker_manhole = args.worker_manhole From 14d1f6a1613faa05104ad4fc744f22cf9cc8c803 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Tue, 16 Jul 2019 23:08:38 +1000 Subject: [PATCH 5/7] put redirect stdio back --- changelog.d/5678.misc | 1 - synapse/config/logger.py | 22 +++++++++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) delete mode 100644 changelog.d/5678.misc diff --git a/changelog.d/5678.misc b/changelog.d/5678.misc deleted file mode 100644 index 2d9a19ad23bf..000000000000 --- a/changelog.d/5678.misc +++ /dev/null @@ -1 +0,0 @@ -Synapse will now always redirect standard I/O to the logging system. diff --git a/synapse/config/logger.py b/synapse/config/logger.py index 2a326776fb25..37a2d8c3319c 100644 --- a/synapse/config/logger.py +++ b/synapse/config/logger.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + import logging import logging.config import os @@ -76,6 +77,7 @@ class LoggingConfig(Config): def read_config(self, config, **kwargs): self.log_config = self.abspath(config.get("log_config")) + self.no_redirect_stdio = config.get("no_redirect_stdio", False) def generate_config_section(self, config_dir_path, server_name, **kwargs): log_config = os.path.join(config_dir_path, server_name + ".log.config") @@ -90,6 +92,20 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs): % locals() ) + def read_arguments(self, args): + if args.no_redirect_stdio is not None: + self.no_redirect_stdio = args.no_redirect_stdio + + def add_arguments(cls, parser): + logging_group = parser.add_argument_group("logging") + logging_group.add_argument( + "-n", + "--no-redirect-stdio", + action="store_true", + default=None, + help="Do not redirect stdout/stderr to the log", + ) + def generate_files(self, config, config_dir_path): log_config = config.get("log_config") if log_config and not os.path.exists(log_config): @@ -179,4 +195,8 @@ def _log(event): return observer(event) - globalLogBeginner.beginLoggingTo([_log], redirectStandardIO=True) + globalLogBeginner.beginLoggingTo( + [_log], redirectStandardIO=not self.no_redirect_stdio + ) + if not config.no_redirect_stdio: + print("Redirected stdout/stderr to logs") From 1528f9dd5863db9184a2908c2b893a3860620b54 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Tue, 16 Jul 2019 23:09:46 +1000 Subject: [PATCH 6/7] put redirect stdio back --- synapse/config/logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/logger.py b/synapse/config/logger.py index 37a2d8c3319c..5d3c9af54ace 100644 --- a/synapse/config/logger.py +++ b/synapse/config/logger.py @@ -196,7 +196,7 @@ def _log(event): return observer(event) globalLogBeginner.beginLoggingTo( - [_log], redirectStandardIO=not self.no_redirect_stdio + [_log], redirectStandardIO=not config.no_redirect_stdio ) if not config.no_redirect_stdio: print("Redirected stdout/stderr to logs") From 8ae081f1c35ab349c5679f42380e9a29b744ed61 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Tue, 16 Jul 2019 23:10:23 +1000 Subject: [PATCH 7/7] put redirect stdio back --- changelog.d/5678.removal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/5678.removal b/changelog.d/5678.removal index 2ad86a5e701c..085b84fda69d 100644 --- a/changelog.d/5678.removal +++ b/changelog.d/5678.removal @@ -1 +1 @@ -Synapse now no longer accepts the `-v`/`--verbose`, `-f`/`--log-file`, `-n`/`--no-redirect-stdio`, or `--log-config` command line flags, and removes the deprecated `verbose`, `no_redirect_stdio`, and `log_file` configuration file options. Users of these options should migrate their options into the dedicated log configuration. +Synapse now no longer accepts the `-v`/`--verbose`, `-f`/`--log-file`, or `--log-config` command line flags, and removes the deprecated `verbose` and `log_file` configuration file options. Users of these options should migrate their options into the dedicated log configuration.