diff --git a/swh/core/api/gunicorn_config.py b/swh/core/api/gunicorn_config.py index 0ab91af..7de6bb7 100644 --- a/swh/core/api/gunicorn_config.py +++ b/swh/core/api/gunicorn_config.py @@ -1,41 +1,41 @@ # Copyright (C) 2019 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information """Default values for gunicorn's configuration. Other packages may override them by importing `*` from this module and redefining functions and variables they want. May be imported by gunicorn using `--config 'python:swh.core.api.gunicorn_config'`.""" from ..sentry import init_sentry def post_fork( server, worker, *, default_sentry_dsn=None, flask=True, sentry_integrations=None, extra_sentry_kwargs={}, disable_logging_events=True, ): # Initializes sentry as soon as possible in gunicorn's worker processes. sentry_integrations = sentry_integrations or [] if flask: from sentry_sdk.integrations.flask import FlaskIntegration sentry_integrations.append(FlaskIntegration()) init_sentry( - default_sentry_dsn, + sentry_dsn=default_sentry_dsn, integrations=sentry_integrations, extra_kwargs=extra_sentry_kwargs, disable_logging_events=disable_logging_events, ) diff --git a/swh/core/api/tests/test_gunicorn.py b/swh/core/api/tests/test_gunicorn.py index e55d377..8e77bda 100644 --- a/swh/core/api/tests/test_gunicorn.py +++ b/swh/core/api/tests/test_gunicorn.py @@ -1,142 +1,159 @@ # Copyright (C) 2019 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information import os from unittest.mock import patch import pkg_resources import swh.core.api.gunicorn_config as gunicorn_config def test_post_fork_default(): with patch("sentry_sdk.init") as sentry_sdk_init: gunicorn_config.post_fork(None, None) sentry_sdk_init.assert_not_called() def test_post_fork_with_dsn_env(): flask_integration = object() # unique object to check for equality logging_integration = object() # unique object to check for equality with patch( "sentry_sdk.integrations.flask.FlaskIntegration", new=lambda: flask_integration ), patch( "sentry_sdk.integrations.logging.LoggingIntegration", new=lambda event_level: logging_integration, ), patch( "sentry_sdk.init" ) as sentry_sdk_init, patch.dict( os.environ, {"SWH_SENTRY_DSN": "test_dsn"} ): gunicorn_config.post_fork(None, None) sentry_sdk_init.assert_called_once_with( dsn="test_dsn", integrations=[flask_integration, logging_integration], debug=False, release=None, environment=None, ) def test_post_fork_with_package_env(): flask_integration = object() logging_integration = object() with patch( "sentry_sdk.integrations.flask.FlaskIntegration", new=lambda: flask_integration ), patch( "sentry_sdk.integrations.logging.LoggingIntegration", new=lambda event_level: logging_integration, ), patch( "sentry_sdk.init" ) as sentry_sdk_init, patch.dict( os.environ, { "SWH_SENTRY_DSN": "test_dsn", "SWH_SENTRY_ENVIRONMENT": "tests", "SWH_MAIN_PACKAGE": "swh.core", }, ): gunicorn_config.post_fork(None, None) version = pkg_resources.get_distribution("swh.core").version sentry_sdk_init.assert_called_once_with( dsn="test_dsn", integrations=[flask_integration, logging_integration], debug=False, release="swh.core@" + version, environment="tests", ) def test_post_fork_debug(): flask_integration = object() logging_integration = object() with patch( "sentry_sdk.integrations.flask.FlaskIntegration", new=lambda: flask_integration ), patch( "sentry_sdk.integrations.logging.LoggingIntegration", new=lambda event_level: logging_integration, ), patch( "sentry_sdk.init" ) as sentry_sdk_init, patch.dict( os.environ, {"SWH_SENTRY_DSN": "test_dsn", "SWH_SENTRY_DEBUG": "1"} ): gunicorn_config.post_fork(None, None) sentry_sdk_init.assert_called_once_with( dsn="test_dsn", integrations=[flask_integration, logging_integration], debug=True, release=None, environment=None, ) def test_post_fork_no_flask(): logging_integration = object() with patch("sentry_sdk.init") as sentry_sdk_init, patch( "sentry_sdk.integrations.logging.LoggingIntegration", new=lambda event_level: logging_integration, ), patch.dict(os.environ, {"SWH_SENTRY_DSN": "test_dsn"}): gunicorn_config.post_fork(None, None, flask=False) sentry_sdk_init.assert_called_once_with( dsn="test_dsn", integrations=[logging_integration], debug=False, release=None, environment=None, ) +def test_post_fork_override_logging_events_envvar(): + + with patch("sentry_sdk.init") as sentry_sdk_init, patch.dict( + os.environ, + {"SWH_SENTRY_DSN": "test_dsn", "SWH_SENTRY_DISABLE_LOGGING_EVENTS": "false"}, + ): + gunicorn_config.post_fork(None, None, flask=False) + + sentry_sdk_init.assert_called_once_with( + dsn="test_dsn", + integrations=[], + debug=False, + release=None, + environment=None, + ) + + def test_post_fork_extras(): flask_integration = object() # unique object to check for equality with patch( "sentry_sdk.integrations.flask.FlaskIntegration", new=lambda: flask_integration ): with patch("sentry_sdk.init") as sentry_sdk_init: with patch.dict(os.environ, {"SWH_SENTRY_DSN": "test_dsn"}): gunicorn_config.post_fork( None, None, sentry_integrations=["foo"], extra_sentry_kwargs={"bar": "baz"}, disable_logging_events=False, ) sentry_sdk_init.assert_called_once_with( dsn="test_dsn", integrations=["foo", flask_integration], debug=False, bar="baz", release=None, environment=None, ) diff --git a/swh/core/cli/__init__.py b/swh/core/cli/__init__.py index c064f91..c354f4d 100644 --- a/swh/core/cli/__init__.py +++ b/swh/core/cli/__init__.py @@ -1,188 +1,188 @@ # Copyright (C) 2019 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information import logging import logging.config from typing import Optional import warnings import click import pkg_resources LOG_LEVEL_NAMES = ["NOTSET", "DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"] CONTEXT_SETTINGS = dict(help_option_names=["-h", "--help"]) logger = logging.getLogger(__name__) class AliasedGroup(click.Group): """A simple Group that supports command aliases, as well as notes related to options""" def __init__(self, name=None, commands=None, **attrs): self.option_notes = attrs.pop("option_notes", None) self.aliases = {} super().__init__(name, commands, **attrs) def get_command(self, ctx, cmd_name): return super().get_command(ctx, self.aliases.get(cmd_name, cmd_name)) def add_alias(self, name, alias): if not isinstance(name, str): name = name.name self.aliases[alias] = name def format_options(self, ctx, formatter): click.Command.format_options(self, ctx, formatter) if self.option_notes: with formatter.section("Notes"): formatter.write_text(self.option_notes) self.format_commands(ctx, formatter) def clean_exit_on_signal(signal, frame): """Raise a SystemExit exception to let command-line clients wind themselves down on exit""" raise SystemExit(0) def validate_loglevel_params(ctx, param, value): """Validate the --log-level parameters, with multiple values""" if value is None: return None return [validate_loglevel(ctx, param, v) for v in value] def validate_loglevel(ctx, param, value): """Validate a single loglevel specification, of the form LOGLEVEL or module:LOGLEVEL.""" if ":" in value: try: module, log_level = value.split(":") except ValueError: raise click.BadParameter( "Invalid log level specification `%s`, " "needs to be in format `module:LOGLEVEL`" % value ) else: module = None log_level = value if log_level not in LOG_LEVEL_NAMES: raise click.BadParameter( "Log level %s unknown (in `%s`) needs to be one of %s" % (log_level, value, ", ".join(LOG_LEVEL_NAMES)) ) return (module, log_level) @click.group( context_settings=CONTEXT_SETTINGS, cls=AliasedGroup, option_notes="""\ If both options are present, --log-level values will override the configuration in --log-config. The --log-config YAML must conform to the logging.config.dictConfig schema documented at https://docs.python.org/3/library/logging.config.html. """, ) @click.option( "--log-level", "-l", "log_levels", default=None, callback=validate_loglevel_params, multiple=True, help=( "Log level (defaults to INFO). " "Can override the log level for a specific module, by using the " "``specific.module:LOGLEVEL`` syntax (e.g. ``--log-level swh.core:DEBUG`` " "will enable DEBUG logging for swh.core)." ), ) @click.option( "--log-config", default=None, type=click.File("r"), help="Python yaml logging configuration file.", ) @click.option( "--sentry-dsn", default=None, help="DSN of the Sentry instance to report to" ) @click.option( "--sentry-debug/--no-sentry-debug", default=False, hidden=True, help="Enable debugging of sentry", ) @click.pass_context def swh(ctx, log_levels, log_config, sentry_dsn, sentry_debug): """Command line interface for Software Heritage.""" import signal import yaml from ..sentry import init_sentry signal.signal(signal.SIGTERM, clean_exit_on_signal) signal.signal(signal.SIGINT, clean_exit_on_signal) - init_sentry(sentry_dsn, debug=sentry_debug) + init_sentry(sentry_dsn=sentry_dsn, debug=sentry_debug) set_default_loglevel: Optional[str] = None if log_config: logging.config.dictConfig(yaml.safe_load(log_config.read())) set_default_loglevel = logging.root.getEffectiveLevel() if not log_levels: log_levels = [] for module, log_level in log_levels: logger = logging.getLogger(module) log_level = logging.getLevelName(log_level) logger.setLevel(log_level) if module is None: set_default_loglevel = log_level if not set_default_loglevel: logging.root.setLevel("INFO") set_default_loglevel = "INFO" ctx.ensure_object(dict) ctx.obj["log_level"] = set_default_loglevel def main(): # Even though swh() sets up logging, we need an earlier basic logging setup # for the next few logging statements logging.basicConfig() # load plugins that define cli sub commands for entry_point in pkg_resources.iter_entry_points("swh.cli.subcommands"): try: cmd = entry_point.load() if isinstance(cmd, click.BaseCommand): # for BW compat, auto add click commands warnings.warn( f"{entry_point.name}: automagic addition of click commands " f"to the main swh group is deprecated", DeprecationWarning, ) swh.add_command(cmd, name=entry_point.name) # otherwise it's expected to be a module which has been loaded # it's the responsibility of the click commands/groups in this # module to transitively have the main swh group as parent. except Exception as e: logger.warning("Could not load subcommand %s: %r", entry_point.name, e) return swh(auto_envvar_prefix="SWH") if __name__ == "__main__": main() diff --git a/swh/core/sentry.py b/swh/core/sentry.py index 2f28607..4674b54 100644 --- a/swh/core/sentry.py +++ b/swh/core/sentry.py @@ -1,76 +1,94 @@ # Copyright (C) 2019-2020 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information +import logging import os from typing import Dict, List, Optional import pkg_resources +logger = logging.getLogger(__name__) + def get_sentry_release(): main_package = os.environ.get("SWH_MAIN_PACKAGE") if main_package: version = pkg_resources.get_distribution(main_package).version return f"{main_package}@{version}" else: return None -def envvar_is_positive(envvar: str) -> bool: - """Check whether a given environment variable looks like a positive boolean value""" - return os.environ.get(envvar, "false").lower() in ("t", "true", "y", "yes", "1") +def override_with_bool_envvar(envvar: str, default: bool) -> bool: + """Override the `default` with the environment variable `envvar` parsed as a boolean""" + envvalue = os.environ.get(envvar, "") + if envvalue.lower() in ("t", "true", "y", "yes", "1"): + return True + elif envvalue.lower() in ("f", "false", "n", "no", "0"): + return False + else: + if envvalue: + logger.warning( + "Could not interpret environment variable %s=%r as boolean, " + "using default value %s", + envvar, + envvalue, + default, + ) + return default def init_sentry( - sentry_dsn: str, + sentry_dsn: Optional[str] = None, *, - debug: Optional[bool] = None, - disable_logging_events: Optional[bool] = None, + environment: Optional[str] = None, + debug: bool = False, + disable_logging_events: bool = False, integrations: Optional[List] = None, extra_kwargs: Optional[Dict] = None, ): """Configure the sentry integration Args: - sentry_dsn: sentry DSN; where sentry report will be sent (if empty, pulled from - :envvar:`SWH_SENTRY_DSN`) - debug: turn on sentry SDK debug mode (if ``None``, pulled from - :envvar:`SWH_SENTRY_DEBUG`) + sentry_dsn: Sentry DSN; where sentry report will be sent. Overridden by + :envvar:`SWH_SENTRY_DSN` + environment: Sentry environment. Overridden by :envvar:`SWH_SENTRY_ENVIRONMENT` + debug: turn on Sentry SDK debug mode. Overridden by :envvar:`SWH_SENTRY_DEBUG` disable_logging_events: if set, disable the automatic reporting of error/exception - log entries as sentry events (if ``None``, pulled from - :envvar:`SWH_SENTRY_DISABLE_LOGGING_EVENTS`) - integrations: list of dedicated sentry integrations objects + log entries as Sentry events. Overridden by + :envvar:`SWH_SENTRY_DISABLE_LOGGING_EVENTS` + integrations: list of dedicated Sentry integrations to include extra_kwargs: dict of additional parameters passed to :func:`sentry_sdk.init` """ if integrations is None: integrations = [] if extra_kwargs is None: extra_kwargs = {} - sentry_dsn = sentry_dsn or os.environ.get("SWH_SENTRY_DSN", "") - environment = os.environ.get("SWH_SENTRY_ENVIRONMENT") + sentry_dsn = os.environ.get("SWH_SENTRY_DSN", sentry_dsn) + environment = os.environ.get("SWH_SENTRY_ENVIRONMENT", environment) - if debug is None: - debug = envvar_is_positive("SWH_SENTRY_DEBUG") - if disable_logging_events is None: - disable_logging_events = envvar_is_positive("SWH_SENTRY_DISABLE_LOGGING_EVENTS") + debug = override_with_bool_envvar("SWH_SENTRY_DEBUG", debug) + disable_logging_events = override_with_bool_envvar( + "SWH_SENTRY_DISABLE_LOGGING_EVENTS", disable_logging_events + ) if sentry_dsn: import sentry_sdk if disable_logging_events: from sentry_sdk.integrations.logging import LoggingIntegration integrations.append(LoggingIntegration(event_level=None)) sentry_sdk.init( release=get_sentry_release(), environment=environment, dsn=sentry_dsn, integrations=integrations, debug=debug, **extra_kwargs, ) diff --git a/swh/core/tests/test_sentry.py b/swh/core/tests/test_sentry.py index 2e51fdc..fd26d93 100644 --- a/swh/core/tests/test_sentry.py +++ b/swh/core/tests/test_sentry.py @@ -1,68 +1,103 @@ # Copyright (C) 2022 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information import logging +import pytest from sentry_sdk import capture_message -from swh.core.sentry import init_sentry +from swh.core.sentry import init_sentry, override_with_bool_envvar + + +@pytest.mark.parametrize( + "envvalue,retval", + ( + ("y", True), + ("n", False), + ("0", False), + ("true", True), + ("FaLsE", False), + ("1", True), + ), +) +def test_override_with_bool_envvar(monkeypatch, envvalue: str, retval: bool): + """Test if the override_with_bool_envvar function returns appropriate results""" + envvar = "OVERRIDE_WITH_BOOL_ENVVAR" + monkeypatch.setenv(envvar, envvalue) + for default in (True, False): + assert override_with_bool_envvar(envvar, default) == retval + + +def test_override_with_bool_envvar_logging(monkeypatch, caplog): + envvar = "OVERRIDE_WITH_BOOL_ENVVAR" + monkeypatch.setenv(envvar, "not a boolean env value") + for default in (True, False): + caplog.clear() + assert override_with_bool_envvar(envvar, default) == default + assert len(caplog.records) == 1 + assert ( + "OVERRIDE_WITH_BOOL_ENVVAR='not a boolean env value'" + in caplog.records[0].getMessage() + ) + assert f"using default value {default}" in caplog.records[0].getMessage() + assert caplog.records[0].levelname == "WARNING" def test_sentry(): reports = [] init_sentry("http://example.org", extra_kwargs={"transport": reports.append}) capture_message("Something went wrong") logging.error("Stupid error") assert len(reports) == 2 assert reports[0]["message"] == "Something went wrong" assert reports[1]["logentry"]["message"] == "Stupid error" def test_sentry_no_logging(): reports = [] init_sentry( "http://example.org", disable_logging_events=True, extra_kwargs={"transport": reports.append}, ) capture_message("Something went wrong") logging.error("Stupid error") assert len(reports) == 1 assert reports[0]["message"] == "Something went wrong" def test_sentry_no_logging_from_venv(monkeypatch): monkeypatch.setenv("SWH_SENTRY_DISABLE_LOGGING_EVENTS", "True") reports = [] init_sentry( "http://example.org", extra_kwargs={"transport": reports.append}, ) capture_message("Something went wrong") logging.error("Stupid error") assert len(reports) == 1 assert reports[0]["message"] == "Something went wrong" def test_sentry_logging_from_venv(monkeypatch): monkeypatch.setenv("SWH_SENTRY_DISABLE_LOGGING_EVENTS", "false") reports = [] init_sentry( "http://example.org", extra_kwargs={"transport": reports.append}, ) capture_message("Something went wrong") logging.error("Stupid error") assert len(reports) == 2