✨ feat(sentry): add optional Sentry integration for bot and API

[itisnotyourenv]

Pull Request

Description

Add optional Sentry error tracking to both the Litestar API and aiogram bot entry points. If no sentry section is present in the YAML config, the app runs exactly as before.

Changes:

• SentryConfig Pydantic model with DSN, environment, and sample rate fields
• init_sentry() helper in src/infrastructure/sentry.py
• Sentry initialization in both API (create_app) and bot (main) startup paths
• sentry-sdk[litestar]>=2.0.0 dependency (aiogram integration is auto-discovered)
• Commented-out sentry section in config-example.yaml

Type of Change

• New feature (non-breaking change which adds functionality)
• Configuration change

Testing

• Tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

Code Quality

• I have performed a self-review of my own code
• My changes generate no new warnings
• I have made corresponding changes to the documentation if needed
• Pre-commit hooks pass (run pre-commit run --all-files)
• Ruff linting passes (ruff check src/ tests/)
• Ruff formatting passes (ruff format src/ tests/ --check)

Additional Notes

• Framework integrations (LitestarIntegration, AiogramIntegration) are auto-discovered by sentry-sdk v2 — no explicit integration setup needed
• The import sentry_sdk is inside init_sentry() body so it only executes when Sentry is actually configured
• All 207 unit tests pass; the coverage threshold failure (55%) is pre-existing (integration tests require test DB)

---

Checklist for Reviewers:

• Code follows project conventions and style guidelines
• Changes are well-tested with appropriate test coverage
• Documentation is updated if necessary
• Security implications have been considered
• Performance implications have been considered

🤖 Generated with Claude Code

[claude]

PR Review: ✨ feat(sentry): add optional Sentry integration for bot and API

Overall the implementation is clean, minimal, and follows the project's architecture well. The opt-in pattern is correct and the lazy import is a nice touch. A few issues worth addressing before merge.

---

🔴 Critical Issues

1. Dangerous production defaults for sample rates

src/infrastructure/config.py

traces_sample_rate: float = 1.0   # 100% tracing overhead in production
profiles_sample_rate: float = 1.0  # 100% profiling overhead in production

Both fields default to 1.0, which means every request will be traced and profiled unless explicitly overridden. Profiling at 100% can cause significant latency overhead. Sentry's own docs recommend starting at 0.1 (10%) or lower for production. Suggested fix:

traces_sample_rate: float = 0.1
profiles_sample_rate: float = 0.1

Also, profiles_sample_rate is documented by Sentry as applying within sampled traces, so having traces_sample_rate=0.0 with profiles_sample_rate=1.0 is a no-op. There is no cross-field validation to catch this misconfiguration.

2. No range validation on sample rate fields

Values outside [0.0, 1.0] are silently accepted by Pydantic but rejected by the Sentry SDK at runtime. This makes misconfiguration hard to debug. The existing PostgresConfig already uses field_validator as a pattern — this should follow suit:

from pydantic import field_validator

@field_validator("traces_sample_rate", "profiles_sample_rate")
@classmethod
def validate_sample_rate(cls, v: float) -> float:
    if not 0.0 <= v <= 1.0:
        raise ValueError("Sample rate must be between 0.0 and 1.0")
    return v

---

🟡 Suggestions for Improvement

3. environment defaults to "production" — risky default

environment: str = "production"

A developer who forgets to set environment in their config will accidentally send local/dev errors tagged as production to Sentry. A safer default is "development", which is also consistent with what the example config shows ("development"). The explicit opt-in to "production" is much less dangerous than the reverse.

4. No error handling around sentry_sdk.init()

init_sentry has no try/except. If the DSN is malformed or the Sentry endpoint is unreachable at startup, the exception will propagate and crash the application. For an optional integration, a graceful degradation is more appropriate:

try:
    sentry_sdk.init(...)
    logger.info("Sentry initialized for environment: %s", config.sentry.environment)
except Exception:
    logger.warning("Failed to initialize Sentry; continuing without it", exc_info=True)

5. Dependency placement in pyproject.toml

sentry-sdk[litestar] is inserted between gunicorn (last runtime dep) and pytest-timeout (first test tool), making it look like a test dependency visually. While it is in the correct section, alphabetically it belongs between pyyaml and sqlalchemy. Minor, but clean ordering helps readability.

6. DSN validation

There is no validation that dsn is a non-empty string or resembles a valid Sentry DSN URL. A field_validator with a basic URL format check (or at minimum a non-empty check) would catch typos early.

---

🟢 What's Done Well

• Lazy import pattern — import sentry_sdk inside init_sentry() is the right approach; avoids import side-effects when Sentry is disabled. The # noqa: PLC0415 comment is correct.
• Fully optional — sentry: SentryConfig | None = None means zero behaviour change for existing deployments.
• Both entrypoints covered — create_app() and main() are both wired up symmetrically.
• Test structure — class-based tests, happy path + default values + missing required field are all present.
• config-example.yaml — the commented-out block is a good UX touch.
• Commit message — well-structured, matches project conventions.
• PR description — clearly explains motivation, what changed, and notes the auto-discovery behaviour.

---

❓ Questions for Clarification

1. Is profiles_sample_rate intentionally exposed? Sentry profiling is still considered experimental in the Python SDK and requires a separate Sentry plan. Leaving it out of the initial implementation would simplify the surface area.
2. Should init_sentry be called as early as possible (before other initialisation, to capture startup errors), or is the current placement at the top of create_app/main sufficient?
3. Are there plans to expose additional SDK options like send_default_pii, release, or before_send? If so, worth noting in the PR to avoid multiple config model changes.

---

Missing Test Coverage

• No test for init_sentry when sentry_sdk.init() raises an exception (especially important if error handling is added per suggestion build: setup database testing infrastructure with factory-boy #4).
• The logger.info call after init is not asserted. Minor, but logging in infrastructure code is worth verifying since it's the only user-visible signal that Sentry started successfully.
• No test for invalid traces_sample_rate / profiles_sample_rate values (blocked by missing validation — add both together).

---

Overall Assessment: Request Changes

The integration is architecturally sound and well-structured. The two blockers are:

1. The 1.0 defaults for sample rates (will hurt production performance)
2. Missing range validation (will produce cryptic runtime errors on misconfiguration)

The environment default is a softer concern but worth fixing in the same pass. All other items are optional improvements.

[claude]

Code Review — PR #77: Optional Sentry Integration

Overall this is a clean, well-scoped addition that follows the project's conventions. Two issues need addressing before merge; the rest are suggestions.

---

🔴 Critical Issues

1. sentry-sdk is a hard production dependency despite being runtime-optional

sentry-sdk[litestar]>=2.0.0 is listed in [project.dependencies] alongside aiogram, litestar, etc. Since Sentry is entirely opt-in at runtime (guarded by config.sentry is None), every deployment — including those that never configure Sentry — is forced to install ~450 KB of SDK.

The better approach is an optional dependency group:

[project.optional-dependencies]
sentry = ["sentry-sdk[litestar]>=2.0.0"]

Installed when needed via uv sync --extra sentry. The lazy import sentry_sdk inside init_sentry() already handles the case where it isn't installed, but right now the package is always present so the guard is never exercised.

---

2. Dangerous production defaults: traces_sample_rate=1.0 and profiles_sample_rate=1.0

Both fields default to 1.0 (100%). In a production environment this means:

• Every request is traced → measurable latency overhead from the Sentry SDK intercepting calls.
• Every request is profiled → even higher overhead (profiling is the more expensive of the two).
• Sentry bill scales directly with traffic from day one.

Sentry's own documentation recommends 0.1 (10%) or lower for production traces, and an even smaller profiles_sample_rate. The example config mirrors these defaults so new users will likely copy them unchanged.

Suggested safer defaults:

traces_sample_rate: float = 0.1
profiles_sample_rate: float = 0.1

And update config-example.yaml accordingly.

---

🟡 Suggestions

3. DSN validation is absent

dsn: str accepts any string including "". An empty or malformed DSN won't be caught at config load; it will only fail (silently, due to the broad except Exception) when sentry_sdk.init() is called at startup. Consider adding a basic guard:

from pydantic import AnyUrl
dsn: AnyUrl

or at minimum min_length=1.

---

4. send_default_pii is not explicitly set

The Sentry SDK defaults send_default_pii to False, but since this project handles Telegram user data, it is worth making the behaviour explicit and exposable in config for auditability:

class SentryConfig(BaseModel):
    ...
    send_default_pii: bool = False

sentry_sdk.init(
    ...
    send_default_pii=config.sentry.send_default_pii,
)

---

5. No release field

release is one of Sentry's most useful features for grouping issues by deployment. Consider adding:

release: str | None = None

and passing it to sentry_sdk.init() when set. It can be populated via GIT_COMMIT_SHA in CI.

---

6. Misleading test in test_no_sentry_config_skips_init

def test_no_sentry_config_skips_init(self):
    config = _make_config(sentry=None)
    with patch("sentry_sdk.init") as mock_init:
        init_sentry(config)
        mock_init.assert_not_called()

When config.sentry is None, init_sentry returns before the import sentry_sdk line is ever reached. The patch context manager is therefore irrelevant — the assertion passes vacuously because sentry_sdk was never touched. The test gives false confidence.

A more meaningful version:

def test_no_sentry_config_skips_init(self):
    config = _make_config(sentry=None)
    # Ensure sentry_sdk is not imported at all when sentry is unconfigured
    with patch.dict("sys.modules", {"sentry_sdk": None}):
        init_sentry(config)  # would raise ImportError if sentry_sdk were imported

---

✅ What's Done Well

• Lazy import pattern (import sentry_sdk inside the guard) is correct — avoids import-time cost when Sentry is not configured.
• Graceful failure (except Exception + logger.warning(exc_info=True)) is appropriate here; a broken Sentry config should never crash the app.
• SentryConfig validation is clean and consistent with the existing Pydantic patterns in the file.
• Both entry points (create_app and main) are wired symmetrically.
• Test structure (class-based, parametrize, dedicated helper _make_config) matches project conventions.
• Commits are clean and logically separated.
• PR description clearly explains the changes, trade-offs, and caveats.

---

Questions

1. Is there a plan to pass a release identifier from CI? Without it, issue grouping across deploys won't work well.
2. Has the Litestar integration been validated to not leak auth headers (e.g., Authorization, X-Telegram-Data) in captured request breadcrumbs?

---

Verdict: Request Changes

The two critical issues (hard dependency + unsafe production defaults) should be resolved before merge. Everything else is non-blocking but worth addressing in a follow-up.