DM-48476: Sentry - the most basic integration possible

Let's use Sentry in the most basic possible way and see what
happens.Don't trace the SSE endpoint because Sentry holds spans in
memory aslong as the connection is open. It seems like the
uninstrumented SSEendopoint also holds on to memory for some reason that
I (Dan) don'tunderstand, but tracing it definitely makes it worse.

docs/user-guide/environment-variables.rst

@@ -30,6 +30,11 @@ See the `Phalanx documentation for Times Square <https://phalanx.lsst.io/applica
    (string) The base URL of the Rubin Science Platform environment.
    This is used for creating URLs to services, such as JupyterHub.
 
+.. envvar:: TS_ENVIRONMENT_NAME
+
+   (string) The name of the Rubin Science Platform environment.
+   This is used as the Sentry environment.
+
 .. envvar:: TS_GAFAELFAWR_TOKEN
 
    (secret string) This token is used to make an admin API call to Gafaelfawr to get a token for the user.
@@ -74,3 +79,8 @@ See the `Phalanx documentation for Times Square <https://phalanx.lsst.io/applica
 .. envvar:: TS_GITHUB_ORGS
 
    (string) A comma-separated list of GitHub organizations that Times Square will sync notebooks from. This is used to filter out incidental GitHub App installations from the general public.
+
+.. envvar:: TS_SENTRY_TRACES_SAMPLE_RATE
+
+   (float) The percentage of transactions to send to Sentry, expressed as a float between 0 and 1. 0 means send no traces, 1 means send every trace.
+

pyproject.toml

@@ -120,6 +120,9 @@ extend-exclude = [
 "src/timessquare/config.py" = [
     "FBT001",  # Pydantic validators take boolean arguments
 ]
+"src/timessquare/sentry.py" = [
+    "S311",  # We're not using random.random for cryptography
+]
 "tests/**" = [
     "C901",    # tests are allowed to be complex, sometimes that's convenient
     "D101",    # tests don't need docstrings

src/timessquare/config.py

@@ -64,6 +64,16 @@ class Config(BaseSettings):
         ),
     ]
 
+    environment_name: Annotated[
+        str,
+        Field(
+            alias="TS_ENVIRONMENT_NAME",
+            description=(
+                "The Phalanx name of the Rubin Science Platform environment."
+            ),
+        ),
+    ]
+
     gafaelfawr_token: Annotated[
         SecretStr,
         Field(
@@ -226,6 +236,28 @@ class Config(BaseSettings):
         ),
     ] = None
 
+    sentry_dsn: Annotated[
+        str | None,
+        Field(
+            alias="TS_SENTRY_DSN",
+            description="DSN for sending events to Sentry.",
+        ),
+    ] = None
+
+    sentry_traces_sample_rate: Annotated[
+        float,
+        Field(
+            alias="TS_SENTRY_TRACES_SAMPLE_RATE",
+            description=(
+                "The percentage of transactions to send to Sentry, expressed "
+                "as a float between 0 and 1. 0 means send no traces, 1 means "
+                "send every trace."
+            ),
+            ge=0,
+            le=1,
+        ),
+    ] = 0
+
     @field_validator("path_prefix")
     @classmethod
     def validate_path_prefix(cls, v: str) -> str:

src/timessquare/handlers/internal.py

@@ -19,12 +19,15 @@
 """FastAPI router for all internal handlers."""
 
 
+@internal_router.get("/healthcheck")
 @internal_router.get(
     "/",
     description=(
         "Return metadata about the running application. Can also be used as"
         " a health check. This route is not exposed outside the cluster and"
-        " therefore cannot be used by external clients."
+        " therefore cannot be used by external clients. This route is also"
+        " exposed at /healthcheck so that Sentry tracing will ignore it:"
+        " https://docs.sentry.io/concepts/data-management/filtering/#transactions-coming-from-health-check"
     ),
     response_model_exclude_none=True,
     summary="Application metadata",
@@ -34,6 +37,8 @@ async def get_index() -> Metadata:
     """GET ``/`` (the app's internal root).
 
     By convention, this endpoint returns only the application's metadata.
+    It is also exposed at ``/healthcheck`` so that Sentry tracing will ignore
+    it: https://docs.sentry.io/concepts/data-management/filtering/#transactions-coming-from-health-check
     """
     return get_metadata(
         package_name="times-square",

src/timessquare/main.py

@@ -15,6 +15,7 @@
 from importlib.metadata import version
 from pathlib import Path
 
+import sentry_sdk
 from fastapi import FastAPI
 from fastapi.openapi.utils import get_openapi
 from safir.database import create_database_engine, is_database_current
@@ -24,6 +25,7 @@
 from safir.fastapi import ClientRequestError, client_request_error_handler
 from safir.logging import configure_logging, configure_uvicorn_logging
 from safir.middleware.x_forwarded import XForwardedMiddleware
+from safir.sentry import before_send_handler
 from safir.slack.webhook import SlackRouteErrorHandler
 from structlog import get_logger
 
@@ -32,9 +34,17 @@
 from .handlers.external import external_router
 from .handlers.internal import internal_router
 from .handlers.v1 import v1_router
+from .sentry import make_traces_sampler
 
 __all__ = ["app", "config"]
 
+sentry_sdk.init(
+    dsn=config.sentry_dsn,
+    environment=config.environment_name,
+    before_send=before_send_handler,
+    traces_sampler=make_traces_sampler(config.sentry_traces_sample_rate),
+)
+
 
 @asynccontextmanager
 async def lifespan(app: FastAPI) -> AsyncIterator:

src/timessquare/sentry.py

@@ -0,0 +1,33 @@
+"""Sentry integration helpers."""
+
+import re
+from collections.abc import Callable
+from typing import Any
+
+__all__ = ["make_traces_sampler"]
+
+EVENTS_REGEX = re.compile("/pages/.*/events$")
+
+
+def make_traces_sampler(
+    original_rate: float,
+) -> Callable[[dict[str, Any]], float]:
+    """Don't instrument events SSE endpoint to avoid leaking memory.
+
+    Sample every other trace at the configured rate.
+
+    When an SSE endpoint is instrumented, Sentry accumlates spans for every
+    sent event in memory until the initial connection is closed. Without Sentry
+    tracing instrumentation, SSE endpoints don't leak memory.
+    """
+
+    def traces_sampler(context: dict[str, Any]) -> float:
+        try:
+            path = context["asgi_scope"]["path"]
+            if EVENTS_REGEX.search(path):
+                return 0
+        except IndexError:
+            pass
+        return original_rate
+
+    return traces_sampler

src/timessquare/worker/main.py

@@ -8,10 +8,12 @@
 
 import arq
 import httpx
+import sentry_sdk
 import structlog
 from safir.database import create_database_engine, is_database_current
 from safir.dependencies.db_session import db_session_dependency
 from safir.logging import configure_logging
+from safir.sentry import before_send_handler
 from safir.slack.blockkit import SlackMessage, SlackTextField
 from safir.slack.webhook import SlackWebhookClient
 
@@ -31,6 +33,13 @@
     repo_removed,
 )
 
+sentry_sdk.init(
+    dsn=config.sentry_dsn,
+    environment=config.environment_name,
+    before_send=before_send_handler,
+    traces_sample_rate=config.sentry_traces_sample_rate,
+)
+
 
 async def startup(ctx: dict[Any, Any]) -> None:
     """Set up the worker context."""

tests/handlers/internal_test.py

@@ -5,21 +5,29 @@
 from typing import TYPE_CHECKING
 
 import pytest
+from httpx import Response
 
 from timessquare.config import config
 
 if TYPE_CHECKING:
     from httpx import AsyncClient
 
 
-@pytest.mark.asyncio
-async def test_get_index(client: AsyncClient) -> None:
-    """Test ``GET /``."""
-    response = await client.get("/")
+def assert_response(response: Response) -> None:
     assert response.status_code == 200
     data = response.json()
     assert data["name"] == config.name
     assert isinstance(data["version"], str)
     assert isinstance(data["description"], str)
     assert isinstance(data["repository_url"], str)
     assert isinstance(data["documentation_url"], str)
+
+
+@pytest.mark.asyncio
+async def test_get_index(client: AsyncClient) -> None:
+    """Test ``GET /``."""
+    response = await client.get("/")
+    assert_response(response)
+
+    response = await client.get("/healthcheck")
+    assert_response(response)

tox.ini

@@ -47,6 +47,7 @@ setenv =
     SAFIR_PROFILE = development
     TS_ALEMBIC_CONFIG_PATH = {toxinidir}/alembic.ini
     TS_ENVIRONMENT_URL = https://test.example.com
+    TS_ENVIRONMENT_NAME = testing
     TS_DATABASE_URL = postgresql://timessquare@localhost:5433/timessquare
     TS_DATABASE_PASSWORD = INSECURE-PASSWORD
     TS_REDIS_URL = redis://localhost:6379/0
@@ -86,6 +87,7 @@ commands = pre-commit run --all-files
 description = Run Alembic against a test database
 setenv =
     TS_ENVIRONMENT_URL = https://test.example.com
+    TS_ENVIRONMENT_NAME = testing
     TS_PATH_PREFIX = /times-square/api
     TS_ALEMBIC_CONFIG_PATH = {toxinidir}/alembic.ini
     TS_DATABASE_URL = postgresql://[email protected]:5432/timessquare
@@ -107,6 +109,7 @@ commands =
     times-square {posargs}
 setenv =
     TS_ENVIRONMENT_URL = https://test.example.com
+    TS_ENVIRONMENT_NAME = testing
     TS_PATH_PREFIX = /times-square/api
     TS_ALEMBIC_CONFIG_PATH = {toxinidir}/alembic.ini
     TS_DATABASE_URL = postgresql://[email protected]:5432/timessquare
@@ -126,6 +129,7 @@ whitelist_externals =
 setenv =
     SAFIR_PROFILE = development
     TS_ENVIRONMENT_URL = https://test.example.com
+    TS_ENVIRONMENT_NAME = testing
     TS_PATH_PREFIX = /times-square/api
     TS_ALEMBIC_CONFIG_PATH = {toxinidir}/alembic.ini
     TS_DATABASE_URL = postgresql://[email protected]:5432/timessquare
@@ -148,6 +152,7 @@ commands_pre =
 description = Build documentation (HTML) with Sphinx.
 setenv =
     TS_ENVIRONMENT_URL = https://test.example.com
+    TS_ENVIRONMENT_NAME = testing
     TS_DATABASE_URL = postgresql://timessquare@localhost:5433/timessquare
     TS_DATABASE_PASSWORD = INSECURE-PASSWORD
     TS_GAFAELFAWR_TOKEN = gt-eOfLolxU8FJ1xr08U7RTbg.Jr-KHSeISXwR5GXHiLemhw
@@ -165,6 +170,7 @@ commands =
 description = Check links in documentation.
 setenv =
     TS_ENVIRONMENT_URL = https://test.example.com
+    TS_ENVIRONMENT_NAME = testing
     TS_DATABASE_URL = postgresql://timessquare@localhost:5433/timessquare
     TS_DATABASE_PASSWORD = INSECURE-PASSWORD
     TS_GAFAELFAWR_TOKEN = gt-eOfLolxU8FJ1xr08U7RTbg.Jr-KHSeISXwR5GXHiLemhw