Add sentry error reporting

[arobson-ods]

Refs #27 — the API should stream errors to Sentry using the standard hooks.

What changed

• New src/register_your_data_api/sentry.py, the only module importing sentry_sdk. Initialised in main.py immediately before the application object is created.
• Sends three things to Sentry: unhandled exceptions, log records at ERROR or above (including from third-party libraries; the audit logger is excluded), and request traces.
• Optional by design: no DSN, a malformed DSN, or a test run leaves the SDK uninitialised and the API unaffected.
• Withholds what the SDK would otherwise transmit: stack frame local variables, request bodies, outgoing request query strings, and the audit log entirely.
• A failed startup is now logged rather than printed, so the failure that stops the service is reported.
• Config: SENTRY_DSN, SENTRY_ENVIRONMENT, SENTRY_TRACES_SAMPLE_RATE, all optional. Documented in README.md, .env.example and CHANGELOG.md.

Architecture / scope decisions

• Decoupled from application code. sentry.py is the only importer of the SDK; everything else reports errors by logging them, which Sentry's logging and Starlette integrations pick up. The existing app_logger.error(..., exc_info=True) in unhandled_exception_handler already did this, so exception_handlers.py needed no change at all. Replacing the service means editing one file.
• Config is read from the environment directly, not through Context. Context isn't created until the application lifespan runs, which is after the Sentry SDK must be initialised. get_environment_config() reproduces Context's precedence: .env first, overridden by real environment variables.
• SENTRY_DSN is deliberately not in Context._REQUIRED_ENV_VARS. Error reporting should not be the reason the API fails to start.

Testing

• 300 passed, 1 skipped (pre-existing). flake8, black, isort, mypy --strict, bandit all clean.
• Tests in tests/unit/test_sentry.py, plus tests/unit/test_main.py for startup reporting.
• The withholding tests assert against what a capturing transport actually received, using canary values not merely that an option was set. Each was confirmed to fail before its fix.
• Verified end to end against the real Sentry project: a genuine error from the running app arrived as event 0b6f4e8036be471096c100dc38db8659.
• tests/conftest.py empties SENTRY_DSN for the session, so the suite cannot report to a real project.

Notes for reviewer

• Two real leaks were found and fixed during implementation, both verified with canaries: frame locals transmitted a live bearer token (an ASGI frame's locals hold the raw request, whose headers are (bytes, bytes) tuples the scrubber cannot match by name), and audit-log CRITICAL records carry the Authorization header contents. Both pinned by regression tests.
• Request tracing is on by default which goes beyond what Add Sentry to stream crashes so that we get alerts #27 asked for. SENTRY_TRACES_SAMPLE_RATE defaults to 1.0 in code, so every request sends a transaction; it is overridable per deployment, and 0 stops transactions entirely while leaving error reporting untouched. It is enabled because Sentry's own setup guidance treats errors plus tracing as the recommended default init for a new project — it says to take that default "as written" and not to "pare it back to errors-only", and recommends tracing specifically when an HTTP framework such as FastAPI is detected. Its sample-rate guidance is 1.0, "lower to 0.1–0.2 in high-traffic production".
• send_default_pii=False is weaker than its name suggests; it does not cover request headers or bodies. Bodies in particular were being sent on successful requests via the transaction event. max_request_body_size="never" is what withholds them. The README documents this.
• Git ignored .pem before realised convention was to store in keys directory so commit probably superfluous (but harmless)

TODO before merge

• Alert rules set up through Sentry UI
• per-VM deployment config

[simon-20]

Hi @arobson-ods,

This looks good.

I've marked as Request changes, but I really just have a question:

given that we're blanking outgoing query strings on the requests to SuiteCRM, should we also scrub the query strings from incoming requests? If so, then once that is added, it's ready to Approve.

[arobson-ods]

Many thanks, @simon-20. You're right that the inconsistency is a problem. But I wonder whether the core issue is that blanking outbound query strings doesn't actually buys us anything. Looking at it again I realised record IDs are already in request.url, so scrubbing the query string is actually rather pointless. Probably being over-zealous in query string scrubbing because BDS query strings are credential bearing. I don't know that scrubbing IDs is practically achievable while still sending Sentry anything useful. Or even required since they're encrypted both in transport and at rest. I think now that the rule should just be: keep credentials out of Sentry. So my proposal is to remove the outbound scrubbing rather than apply it inbound. If you agree I'll make that change.

[simon-20]

Many thanks, @simon-20. You're right that the inconsistency is a problem. But I wonder whether the core issue is that blanking outbound query strings doesn't actually buys us anything. Looking at it again I realised record IDs are already in request.url, so scrubbing the query string is actually rather pointless. Probably being over-zealous in query string scrubbing because BDS query strings are credential bearing. I don't know that scrubbing IDs is practically achievable while still sending Sentry anything useful. Or even required since they're encrypted both in transport and at rest. I think now that the rule should just be: keep credentials out of Sentry. So my proposal is to remove the outbound scrubbing rather than apply it inbound. If you agree I'll make that change.

Yes, that makes sense. Sounds good to me.