From a41f2cf01dd87128b9862c891153899b289ec87a Mon Sep 17 00:00:00 2001
From: Renaud Boyer <renaud.boyer@sofwareheritage.org>
Date: Wed, 8 Jan 2025 17:05:28 +0100
Subject: [PATCH] sentry: Implement performance tracing
---
swh/core/api/gunicorn_config.py | 3 ++
swh/core/api/tests/test_gunicorn.py | 60 +++++++++++++++++++++++++++++
swh/core/sentry.py | 47 ++++++++++++++++++++--
swh/core/tests/test_cli.py | 5 +++
swh/core/tests/test_sentry.py | 42 +++++++++++++++++++-
5 files changed, 153 insertions(+), 4 deletions(-)
diff --git a/swh/core/api/gunicorn_config.py b/swh/core/api/gunicorn_config.py
index 7de6bb7a..1e2ffbea 100644
--- a/swh/core/api/gunicorn_config.py
+++ b/swh/core/api/gunicorn_config.py
@@ -11,6 +11,7 @@ and redefining functions and variables they want.
May be imported by gunicorn using
`--config 'python:swh.core.api.gunicorn_config'`."""
+from typing import Optional
from ..sentry import init_sentry
@@ -24,6 +25,7 @@ def post_fork(
sentry_integrations=None,
extra_sentry_kwargs={},
disable_logging_events=True,
+ traces_sample_rate: Optional[float] = None,
):
# Initializes sentry as soon as possible in gunicorn's worker processes.
@@ -38,4 +40,5 @@ def post_fork(
integrations=sentry_integrations,
extra_kwargs=extra_sentry_kwargs,
disable_logging_events=disable_logging_events,
+ traces_sample_rate=traces_sample_rate,
)
diff --git a/swh/core/api/tests/test_gunicorn.py b/swh/core/api/tests/test_gunicorn.py
index c19d693a..3a4b21a5 100644
--- a/swh/core/api/tests/test_gunicorn.py
+++ b/swh/core/api/tests/test_gunicorn.py
@@ -29,6 +29,7 @@ def test_post_fork_default(mocker):
debug=False,
release="0.0.0",
environment=None,
+ traces_sample_rate=None,
)
@@ -53,6 +54,7 @@ def test_post_fork_with_dsn_env(mocker):
debug=False,
release=None,
environment=None,
+ traces_sample_rate=None,
)
@@ -87,6 +89,7 @@ def test_post_fork_with_package_env(mocker):
debug=False,
release="swh.core@" + version,
environment="tests",
+ traces_sample_rate=None,
)
@@ -114,6 +117,7 @@ def test_post_fork_debug(mocker):
debug=True,
release=None,
environment=None,
+ traces_sample_rate=None,
)
@@ -134,6 +138,7 @@ def test_post_fork_no_flask(mocker):
debug=False,
release=None,
environment=None,
+ traces_sample_rate=None,
)
@@ -152,6 +157,7 @@ def test_post_fork_override_logging_events_envvar(mocker):
debug=False,
release=None,
environment=None,
+ traces_sample_rate=None,
)
@@ -178,4 +184,58 @@ def test_post_fork_extras(mocker):
bar="baz",
release=None,
environment=None,
+ traces_sample_rate=None,
+ )
+
+
+def test_post_fork_traces_sample_rate(mocker):
+ flask_integration = object() # unique object to check for equality
+ logging_integration = object() # unique object to check for equality
+ mocker.patch(
+ "sentry_sdk.integrations.flask.FlaskIntegration", new=lambda: flask_integration
+ )
+ mocker.patch(
+ "sentry_sdk.integrations.logging.LoggingIntegration",
+ new=lambda event_level: logging_integration,
+ )
+ sentry_sdk_init = mocker.patch("sentry_sdk.init")
+ mocker.patch.dict(os.environ, {"SWH_SENTRY_DSN": "test_dsn"})
+
+ gunicorn_config.post_fork(None, None, traces_sample_rate=1.0)
+
+ sentry_sdk_init.assert_called_once_with(
+ dsn="test_dsn",
+ integrations=[flask_integration, logging_integration],
+ debug=False,
+ release=None,
+ environment=None,
+ traces_sample_rate=1.0,
+ )
+
+
+def test_post_fork_override_traces_sample_rate_envvar(mocker):
+ flask_integration = object() # unique object to check for equality
+ logging_integration = object() # unique object to check for equality
+ mocker.patch(
+ "sentry_sdk.integrations.flask.FlaskIntegration", new=lambda: flask_integration
+ )
+ mocker.patch(
+ "sentry_sdk.integrations.logging.LoggingIntegration",
+ new=lambda event_level: logging_integration,
+ )
+ sentry_sdk_init = mocker.patch("sentry_sdk.init")
+ mocker.patch.dict(
+ os.environ,
+ {"SWH_SENTRY_DSN": "test_dsn", "SWH_SENTRY_TRACES_SAMPLE_RATE": "0.999"},
+ )
+
+ 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,
+ traces_sample_rate=0.999,
)
diff --git a/swh/core/sentry.py b/swh/core/sentry.py
index 6fd5b1bf..a0dd1554 100644
--- a/swh/core/sentry.py
+++ b/swh/core/sentry.py
@@ -46,6 +46,37 @@ def override_with_bool_envvar(envvar: str, default: bool) -> bool:
return default
+def override_with_float_envvar(
+ envvar: str, default: Optional[float]
+) -> Optional[float]:
+ """Override `default` with the environment variable `envvar` casted as a float.
+
+ `default` is returned if the environment variable `envvar` is missing or if
+ we're not able to cast it to a float.
+
+ Args:
+ envvar: the name of the environment variable
+ default: default value
+
+ Returns:
+ A float or `default`
+ """
+ envvalue = os.environ.get(envvar)
+ if envvalue is None:
+ return default
+ try:
+ return float(envvalue)
+ except ValueError:
+ logger.warning(
+ "Could not interpret environment variable %s=%r as float, "
+ "using default value %s",
+ envvar,
+ envvalue,
+ default,
+ )
+ return default
+
+
def init_sentry(
sentry_dsn: Optional[str] = None,
*,
@@ -54,10 +85,11 @@ def init_sentry(
debug: bool = False,
disable_logging_events: bool = False,
integrations: Optional[List] = None,
+ traces_sample_rate: Optional[float] = None,
extra_kwargs: Optional[Dict] = None,
deferred_init: bool = False,
-):
- """Configure the sentry integration
+) -> None:
+ """Configure the sentry integration.
Args:
sentry_dsn: Sentry DSN; where sentry report will be sent. Overridden by
@@ -70,10 +102,12 @@ def init_sentry(
log entries as Sentry events. Overridden by
:envvar:`SWH_SENTRY_DISABLE_LOGGING_EVENTS`
integrations: list of dedicated Sentry integrations to include
+ traces_sample_rate: a number between 0 and 1, controlling the percentage chance a
+ given transaction will be sent to Sentry. Overridden by
+ :envvar:`SWH_SENTRY_TRACES_SAMPLE_RATE`
extra_kwargs: dict of additional parameters passed to :func:`sentry_sdk.init`
deferred_init: indicates that sentry will be properly initialized in subsequent
calls and that no warnings about missing DSN should be logged
-
"""
if integrations is None:
integrations = []
@@ -101,10 +135,17 @@ def init_sentry(
integrations.append(LoggingIntegration(event_level=None))
+ # to completely disable tracing `traces_sample_rate` should be set to None instead
+ # of 0.0
+ traces_sample_rate = override_with_float_envvar(
+ "SWH_SENTRY_TRACES_SAMPLE_RATE", traces_sample_rate
+ )
+
sentry_sdk.init(
release=get_sentry_release(main_package, sentry_dsn),
environment=environment,
dsn=sentry_dsn,
+ traces_sample_rate=traces_sample_rate,
integrations=integrations,
debug=debug,
**extra_kwargs,
diff --git a/swh/core/tests/test_cli.py b/swh/core/tests/test_cli.py
index cdd50e4a..170cb1e1 100644
--- a/swh/core/tests/test_cli.py
+++ b/swh/core/tests/test_cli.py
@@ -124,6 +124,7 @@ def test_command(swhmain, mocker):
integrations=[],
release="0.0.0",
environment=None,
+ traces_sample_rate=None,
)
assert_result(result)
assert result.output.strip() == "Hello SWH!"
@@ -185,6 +186,7 @@ def test_sentry(swhmain, mocker):
integrations=[],
release=None,
environment=None,
+ traces_sample_rate=None,
)
@@ -207,6 +209,7 @@ def test_sentry_debug(swhmain, mocker):
integrations=[],
release=None,
environment=None,
+ traces_sample_rate=None,
)
@@ -231,6 +234,7 @@ def test_sentry_env(swhmain, mocker):
integrations=[],
release=None,
environment=None,
+ traces_sample_rate=None,
)
@@ -259,6 +263,7 @@ def test_sentry_env_main_package(swhmain, mocker):
integrations=[],
release="swh.core@" + version,
environment="tests",
+ traces_sample_rate=None,
)
diff --git a/swh/core/tests/test_sentry.py b/swh/core/tests/test_sentry.py
index 44ad156d..a5405907 100644
--- a/swh/core/tests/test_sentry.py
+++ b/swh/core/tests/test_sentry.py
@@ -9,7 +9,11 @@ import pytest
import sentry_sdk
from sentry_sdk import capture_exception, capture_message, set_tag
-from swh.core.sentry import init_sentry, override_with_bool_envvar
+from swh.core.sentry import (
+ init_sentry,
+ override_with_bool_envvar,
+ override_with_float_envvar,
+)
SENTRY_DSN = "https://user@example.org/1234"
@@ -48,6 +52,31 @@ def test_override_with_bool_envvar_logging(monkeypatch, caplog):
assert caplog.records[0].levelname == "WARNING"
+@pytest.mark.parametrize(
+ "envvalue,retval",
+ (("1.0", 1.0), ("0.0", 0.0), ("0", 0.0), ("a", None)),
+)
+def test_override_with_float_envvar(monkeypatch, envvalue: str, retval: bool):
+ envvar = "OVERRIDE_WITH_FLOAT_ENVVAR"
+ monkeypatch.setenv(envvar, envvalue)
+ assert override_with_float_envvar(envvar, None) == retval
+
+
+def test_override_with_float_envvar_logging(monkeypatch, caplog):
+ envvar = "OVERRIDE_WITH_FLOAT_ENVVAR"
+ monkeypatch.setenv(envvar, "not a float env value")
+ for default in (None, 1.0):
+ caplog.clear()
+ assert override_with_float_envvar(envvar, default) == default
+ assert len(caplog.records) == 1
+ assert (
+ "OVERRIDE_WITH_FLOAT_ENVVAR='not a float 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(SENTRY_DSN, extra_kwargs={"transport": reports.append})
@@ -160,3 +189,14 @@ def test_sentry_deferred_init(caplog, deferred_init):
)
else:
assert not caplog.records
+
+
+@pytest.mark.parametrize("traces_sample_rate", [1.0, 0.0, None])
+def test_sentry_traces_sample_rate(caplog, traces_sample_rate):
+ init_sentry(None, traces_sample_rate=traces_sample_rate)
+ client = sentry_sdk.get_client()
+ if traces_sample_rate is not None:
+ assert client.options["traces_sample_rate"] == traces_sample_rate
+ else:
+ assert client.options["enable_tracing"] is None
+ assert client.options["traces_sample_rate"] is None
--
GitLab