sentry: always override init settings with the environment variables

Instead of only overriding the initial settings when they're left unset,
always override the defaults when the environment variables are set.

This makes the behavior more consistent with the way we usually handle
environment variables. It also allows setting the environment variable
`SWH_SENTRY_DISABLE_LOGGING_EVENTS=false`, to enable events based on the
logging framework in gunicorn backends (where the default has been
flipped to true).

swh/core/api/gunicorn_config.py

@@ -34,7 +34,7 @@ def post_fork(
         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,

swh/core/api/tests/test_gunicorn.py

@@ -117,6 +117,23 @@ def test_post_fork_no_flask():
     )
 
 
+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(

swh/core/cli/__init__.py

@@ -132,7 +132,7 @@ def swh(ctx, log_levels, log_config, sentry_dsn, sentry_debug):
     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
 

swh/core/sentry.py

@@ -3,11 +3,14 @@
 # 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")
@@ -18,30 +21,45 @@ def get_sentry_release():
         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`
 
     """
@@ -50,13 +68,13 @@ def init_sentry(
     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

swh/core/tests/test_sentry.py

@@ -5,9 +5,44 @@
 
 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():