marcel/familienarchiv
1
0
Fork 0
Code Issues 117 Pull Requests 1 Actions 2 Packages Projects Releases Wiki Activity

feat(backend): integrate sentry-spring-boot-starter for exception reporting to GlitchTip #580

New Issue
Open
opened 2026-05-14 15:05:44 +02:00 by marcel · 8 comments
marcel commented 2026-05-14 15:05:44 +02:00
Owner
Copy Link

Context

GlitchTip speaks the Sentry SDK wire protocol. Adding sentry-spring-boot-starter-jakarta to the Spring Boot backend automatically captures unhandled exceptions thrown from @RestController methods and sends them to GlitchTip with full stack traces and request context.

DomainException 4xx errors (not-found, forbidden, conflict) are user-generated conditions — they must NOT be sent to GlitchTip, as they would flood the error tracker with noise. Only unexpected 5xx errors should be captured.

When SENTRY_DSN is empty (default in .env.example), Sentry is fully disabled — the application starts cleanly and logs Sentry is disabled.

Depends on: GlitchTip infrastructure issue — a project DSN must exist before this can be tested end-to-end

backend/pom.xml — add dependency

<dependency>
    <groupId>io.sentry</groupId>
    <artifactId>sentry-spring-boot-starter-jakarta</artifactId>
    <version>8.5.0</version>   <!-- check https://github.com/getsentry/sentry-java/releases for latest stable 8.x -->
</dependency>

backend/src/main/resources/application.yml

sentry:
  dsn: ${SENTRY_DSN:}
  environment: ${SPRING_PROFILES_ACTIVE:dev}
  traces-sample-rate: 1.0
  send-default-pii: false
  enable-tracing: true
  ignored-exceptions-for-type:
    - org.raddatz.familienarchiv.exception.DomainException

ignored-exceptions-for-type prevents all DomainException instances (404, 403, 409) from being reported. Only unhandled exceptions that bubble up as 5xx responses will be captured.

docker-compose.yml — add env var to backend service

environment:
  SENTRY_DSN: ${SENTRY_DSN:-}

.env.example — already added by scaffold issue; confirm this line exists:

SENTRY_DSN=

Acceptance Criteria

  • ./mvnw clean package succeeds
  • ./mvnw test passes — no test regressions
  • When SENTRY_DSN is empty, application startup log contains Sentry is disabled and no errors
  • When SENTRY_DSN is set to the GlitchTip Django project DSN:
    • Throwing a RuntimeException from a test endpoint causes an event in GlitchTip within 30 seconds
    • The captured event shows the full Java stack trace
    • Making a request that triggers DomainException.notFound(...) does not produce a GlitchTip event (verify by checking GlitchTip Issues list remains unchanged)
  • HTTP 5xx errors caused by unexpected exceptions produce GlitchTip events; HTTP 4xx errors from DomainException do not

Definition of Done

  • All acceptance criteria checked
  • pom.xml and application.yml committed
  • docker-compose.yml env var added
  • Committed on a feature branch, PR opened against main
## Context GlitchTip speaks the Sentry SDK wire protocol. Adding `sentry-spring-boot-starter-jakarta` to the Spring Boot backend automatically captures unhandled exceptions thrown from `@RestController` methods and sends them to GlitchTip with full stack traces and request context. `DomainException` 4xx errors (not-found, forbidden, conflict) are user-generated conditions — they must NOT be sent to GlitchTip, as they would flood the error tracker with noise. Only unexpected 5xx errors should be captured. When `SENTRY_DSN` is empty (default in `.env.example`), Sentry is fully disabled — the application starts cleanly and logs `Sentry is disabled`. **Depends on:** GlitchTip infrastructure issue — a project DSN must exist before this can be tested end-to-end ## `backend/pom.xml` — add dependency ```xml <dependency> <groupId>io.sentry</groupId> <artifactId>sentry-spring-boot-starter-jakarta</artifactId> <version>8.5.0</version> <!-- check https://github.com/getsentry/sentry-java/releases for latest stable 8.x --> </dependency> ``` ## `backend/src/main/resources/application.yml` ```yaml sentry: dsn: ${SENTRY_DSN:} environment: ${SPRING_PROFILES_ACTIVE:dev} traces-sample-rate: 1.0 send-default-pii: false enable-tracing: true ignored-exceptions-for-type: - org.raddatz.familienarchiv.exception.DomainException ``` `ignored-exceptions-for-type` prevents all `DomainException` instances (404, 403, 409) from being reported. Only unhandled exceptions that bubble up as 5xx responses will be captured. ## `docker-compose.yml` — add env var to backend service ```yaml environment: SENTRY_DSN: ${SENTRY_DSN:-} ``` ## `.env.example` — already added by scaffold issue; confirm this line exists: ``` SENTRY_DSN= ``` ## Acceptance Criteria - [ ] `./mvnw clean package` succeeds - [ ] `./mvnw test` passes — no test regressions - [ ] When `SENTRY_DSN` is empty, application startup log contains `Sentry is disabled` and no errors - [ ] When `SENTRY_DSN` is set to the GlitchTip Django project DSN: - [ ] Throwing a `RuntimeException` from a test endpoint causes an event in GlitchTip within 30 seconds - [ ] The captured event shows the full Java stack trace - [ ] Making a request that triggers `DomainException.notFound(...)` does **not** produce a GlitchTip event (verify by checking GlitchTip Issues list remains unchanged) - [ ] HTTP 5xx errors caused by unexpected exceptions produce GlitchTip events; HTTP 4xx errors from `DomainException` do not ## Definition of Done - All acceptance criteria checked - `pom.xml` and `application.yml` committed - `docker-compose.yml` env var added - Committed on a feature branch, PR opened against `main`
marcel added this to the Observability Stack — Grafana LGTM + GlitchTip milestone 2026-05-14 15:05:44 +02:00
marcel added the P2-mediumfeaturephase-7: monitoring labels 2026-05-14 15:06:14 +02:00
marcel commented 2026-05-14 15:28:32 +02:00
Author
Owner
Copy Link

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

  • The implementation is configuration-only: pom.xml + application.yaml + docker-compose.yml. No new Java classes needed. This is the right approach — Sentry's Spring Boot auto-configuration handles everything.
  • ignored-exceptions-for-type with DomainException is correct in principle, but there is a subtle gap: DomainException.internal(...) produces a 5xx response and is also a DomainException instance. The issue specifies "only 5xx should be captured", but DomainException.internal(...) is used in 8+ places in the codebase (OCR unavailable, file upload failures, invite code generation). These 5xx DomainException instances will be silently swallowed by the ignored-exceptions rule. This contradicts the stated goal.
    • Concrete example: OcrService.java throws DomainException.internal(ErrorCode.OCR_SERVICE_UNAVAILABLE, ...) — an operational failure worth tracking in GlitchTip. The current config will suppress it.
  • GlobalExceptionHandler already has a handleGeneric(Exception ex) catch-all that calls log.error("Unhandled exception", ex). The Sentry SDK will intercept before this handler fires for uncaught exceptions, which is correct. But DomainException instances caught by handleDomain() are consumed by the handler and never re-thrown — Sentry will not see them regardless of the ignored list. The ignored-exceptions setting is only redundant assurance; the real mechanism is handler-level suppression.
  • traces-sample-rate: 1.0 combined with enable-tracing: true enables full distributed tracing. For a family archive with modest traffic this is fine, but be aware GlitchTip's tracing support is partial (performance data may not render as expected).
  • send-default-pii: false is correctly set — good default for GDPR.

Recommendations

  • Address the 5xx DomainException gap: Either (a) use a Sentry BeforeSend hook in a @Configuration class that re-allows DomainException instances whose getStatus().is5xxServerError() is true, or (b) accept the current behavior and document it explicitly in the issue body. Option (b) is simpler and may be acceptable if OCR failures are already visible in logs. Make the decision explicit.
  • The ignored-exceptions-for-type list is clean as-is for the 4xx suppression goal. No change needed there.
  • Consider omitting enable-tracing: true / traces-sample-rate: 1.0 until GlitchTip's tracing features are confirmed to work on your self-hosted version. These settings add overhead without guaranteed value. Safe to add in a follow-up.
  • The issue's AC includes "Making a request that triggers DomainException.notFound(...) does not produce a GlitchTip event". This is verifiable and well-written. Add a parallel AC: "Throwing DomainException.internal(...) from a test endpoint either does or explicitly does not produce a GlitchTip event (document the chosen behavior)."
## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - The implementation is configuration-only: `pom.xml` + `application.yaml` + `docker-compose.yml`. No new Java classes needed. This is the right approach — Sentry's Spring Boot auto-configuration handles everything. - `ignored-exceptions-for-type` with `DomainException` is correct in principle, but there is a subtle gap: `DomainException.internal(...)` produces a **5xx** response and is also a `DomainException` instance. The issue specifies "only 5xx should be captured", but `DomainException.internal(...)` is used in 8+ places in the codebase (OCR unavailable, file upload failures, invite code generation). These 5xx `DomainException` instances will be **silently swallowed** by the ignored-exceptions rule. This contradicts the stated goal. - Concrete example: `OcrService.java` throws `DomainException.internal(ErrorCode.OCR_SERVICE_UNAVAILABLE, ...)` — an operational failure worth tracking in GlitchTip. The current config will suppress it. - `GlobalExceptionHandler` already has a `handleGeneric(Exception ex)` catch-all that calls `log.error("Unhandled exception", ex)`. The Sentry SDK will intercept before this handler fires for uncaught exceptions, which is correct. But `DomainException` instances caught by `handleDomain()` are consumed by the handler and never re-thrown — Sentry will not see them regardless of the ignored list. The ignored-exceptions setting is only redundant assurance; the real mechanism is handler-level suppression. - `traces-sample-rate: 1.0` combined with `enable-tracing: true` enables full distributed tracing. For a family archive with modest traffic this is fine, but be aware GlitchTip's tracing support is partial (performance data may not render as expected). - `send-default-pii: false` is correctly set — good default for GDPR. ### Recommendations - **Address the 5xx `DomainException` gap**: Either (a) use a Sentry `BeforeSend` hook in a `@Configuration` class that re-allows `DomainException` instances whose `getStatus().is5xxServerError()` is true, or (b) accept the current behavior and document it explicitly in the issue body. Option (b) is simpler and may be acceptable if OCR failures are already visible in logs. Make the decision explicit. - The `ignored-exceptions-for-type` list is clean as-is for the 4xx suppression goal. No change needed there. - Consider omitting `enable-tracing: true` / `traces-sample-rate: 1.0` until GlitchTip's tracing features are confirmed to work on your self-hosted version. These settings add overhead without guaranteed value. Safe to add in a follow-up. - The issue's AC includes "Making a request that triggers `DomainException.notFound(...)` does **not** produce a GlitchTip event". This is verifiable and well-written. Add a parallel AC: "Throwing `DomainException.internal(...)` from a test endpoint either **does** or explicitly **does not** produce a GlitchTip event (document the chosen behavior)."
marcel commented 2026-05-14 15:28:47 +02:00
Author
Owner
Copy Link

🏛️ Markus Keller — Senior Application Architect

Observations

  • This is a cross-cutting concern (error reporting) added via library auto-configuration. It correctly avoids touching domain packages — no service or controller code changes. That's the right architectural approach.
  • The sentry-spring-boot-starter-jakarta at version 8.5.0 is a substantial third-party dependency. It integrates via Spring Boot auto-configuration, which means it will attempt to instrument HTTP requests, transactions, and potentially bean creation at startup. Worth confirming it doesn't conflict with the existing Jetty setup (the project uses Jetty, not Tomcat — sentry-spring-boot-starter-jakarta generally works with any servlet container, but it's worth a conscious check).
  • The SENTRY_DSN: ${SENTRY_DSN:-} pattern in docker-compose.yml uses the dash-default syntax (empty string default). The application's sentry.dsn: ${SENTRY_DSN:} uses the colon-no-default syntax which also resolves to empty string. Both are correct and consistent.
  • Adding Sentry to the context diagram (docs/architecture/c4/l1-context.puml) is technically warranted — it is a new external system the backend communicates with. However, since GlitchTip is self-hosted on the same VPS, it could reasonably appear in l2-containers.puml instead as an internal service. The infrastructure issue that provisions GlitchTip should own the container-level diagram update; this issue owns at most the "backend → GlitchTip" arrow.
  • This issue depends on a GlitchTip infrastructure issue (mentioned in the body). That dependency should be tracked in Gitea as a "blocked by" link to prevent the PR from merging without a testable DSN.

Recommendations

  • Confirm Jetty compatibility with sentry-spring-boot-starter-jakarta during the first ./mvnw clean package run. If there are classpath conflicts, the fallback is sentry-spring-jakarta (manual setup, no starter).
  • If enable-tracing: true is kept, document in the issue whether GlitchTip's hosted version supports Sentry performance data — partial tracing support is a known limitation of GlitchTip vs. full Sentry SaaS.
  • The architecture diagram update (L1 or L2 context) should be in scope for whichever issue delivers the full GlitchTip stack, not necessarily this one. Flag it as a dependency, not a blocker for this PR.
  • The application.yaml config block fits cleanly in the base config (not dev-profile-only) because SENTRY_DSN defaults to empty. This is correct — Sentry is disabled when the var is absent, so there's no "dev only" concern.
## 🏛️ Markus Keller — Senior Application Architect ### Observations - This is a cross-cutting concern (error reporting) added via library auto-configuration. It correctly avoids touching domain packages — no service or controller code changes. That's the right architectural approach. - The `sentry-spring-boot-starter-jakarta` at version 8.5.0 is a substantial third-party dependency. It integrates via Spring Boot auto-configuration, which means it will attempt to instrument HTTP requests, transactions, and potentially bean creation at startup. Worth confirming it doesn't conflict with the existing Jetty setup (the project uses Jetty, not Tomcat — `sentry-spring-boot-starter-jakarta` generally works with any servlet container, but it's worth a conscious check). - The `SENTRY_DSN: ${SENTRY_DSN:-}` pattern in `docker-compose.yml` uses the dash-default syntax (empty string default). The application's `sentry.dsn: ${SENTRY_DSN:}` uses the colon-no-default syntax which also resolves to empty string. Both are correct and consistent. - Adding Sentry to the context diagram (`docs/architecture/c4/l1-context.puml`) is technically warranted — it is a new external system the backend communicates with. However, since GlitchTip is self-hosted on the same VPS, it could reasonably appear in `l2-containers.puml` instead as an internal service. The infrastructure issue that provisions GlitchTip should own the container-level diagram update; this issue owns at most the "backend → GlitchTip" arrow. - This issue depends on a GlitchTip infrastructure issue (mentioned in the body). That dependency should be tracked in Gitea as a "blocked by" link to prevent the PR from merging without a testable DSN. ### Recommendations - Confirm Jetty compatibility with `sentry-spring-boot-starter-jakarta` during the first `./mvnw clean package` run. If there are classpath conflicts, the fallback is `sentry-spring-jakarta` (manual setup, no starter). - If `enable-tracing: true` is kept, document in the issue whether GlitchTip's hosted version supports Sentry performance data — partial tracing support is a known limitation of GlitchTip vs. full Sentry SaaS. - The architecture diagram update (L1 or L2 context) should be in scope for whichever issue delivers the full GlitchTip stack, not necessarily this one. Flag it as a dependency, not a blocker for this PR. - The `application.yaml` config block fits cleanly in the base config (not dev-profile-only) because `SENTRY_DSN` defaults to empty. This is correct — Sentry is disabled when the var is absent, so there's no "dev only" concern.
marcel commented 2026-05-14 15:29:03 +02:00
Author
Owner
Copy Link

🔐 Nora "NullX" Steiner — Application Security Engineer

Observations

  • send-default-pii: false is correctly set. This prevents Sentry from capturing usernames, IP addresses, cookies, and HTTP headers in error events. For a family archive containing personal documents, this setting is non-negotiable. Good call to include it explicitly.
  • Stack traces in GlitchTip contain developer messages from DomainException. The DomainException developer messages in this codebase include things like "Not found: " + id (UUIDs) and "Failed to upload file: " + e.getMessage() (which may contain file paths or storage endpoint URLs). Since DomainException instances are suppressed via ignored-exceptions-for-type, these won't reach GlitchTip — but the handleGeneric catch-all for unexpected exceptions could include stack frames containing user-controlled data (e.g., filenames in S3 errors). This is a smell to watch, not a definite vulnerability.
  • DSN is a semi-secret. A GlitchTip DSN is not a credential in the traditional sense (it has no write access to GlitchTip beyond error submission), but it identifies the project and organization. Storing it in .env (gitignored) and injecting via env var is the correct pattern — it's already how the issue specifies it. Confirm .env is in .gitignore.
  • GlitchTip receives exception data including the full stack trace and exception message. If any exception message includes user-supplied input (e.g., a filename a user uploaded, a document title), that text reaches GlitchTip. With send-default-pii: false, Sentry won't add HTTP request bodies, but exception messages are always included. Audit throw DomainException.internal(...) call sites to confirm developer messages do not embed user-controlled strings verbatim.
  • No new attack surface on the backend. This is outbound-only: the backend POSTs to GlitchTip's ingest endpoint. No inbound port is opened.

Recommendations

  • Audit the 8 DomainException.internal(...) call sites and the OCR/file error catch blocks where raw e.getMessage() is embedded in the exception message (e.g., DocumentController.java:152, DocumentService.java:619). Those e.getMessage() strings could contain S3 endpoint URLs, bucket names, or file paths. Since these exceptions are caught by handleGeneric and would reach Sentry, consider stripping the raw cause message: "Failed to upload file — see logs for details" rather than "Failed to upload file: " + e.getMessage(). This is a minor hardening step, not a blocking issue.
  • Confirm .env is in .gitignore (it should be, given the existing pattern with other secrets). The SENTRY_DSN= line in .env.example is correct — an empty default exposes nothing.
  • No GDPR concern from Sentry SDK itself given send-default-pii: false, but if GlitchTip is self-hosted in the EU (Hetzner), data residency is fine regardless.
## 🔐 Nora "NullX" Steiner — Application Security Engineer ### Observations - **`send-default-pii: false` is correctly set.** This prevents Sentry from capturing usernames, IP addresses, cookies, and HTTP headers in error events. For a family archive containing personal documents, this setting is non-negotiable. Good call to include it explicitly. - **Stack traces in GlitchTip contain developer messages from `DomainException`.** The `DomainException` developer messages in this codebase include things like `"Not found: " + id` (UUIDs) and `"Failed to upload file: " + e.getMessage()` (which may contain file paths or storage endpoint URLs). Since `DomainException` instances are suppressed via `ignored-exceptions-for-type`, these won't reach GlitchTip — but the `handleGeneric` catch-all for unexpected exceptions could include stack frames containing user-controlled data (e.g., filenames in S3 errors). This is a smell to watch, not a definite vulnerability. - **DSN is a semi-secret.** A GlitchTip DSN is not a credential in the traditional sense (it has no write access to GlitchTip beyond error submission), but it identifies the project and organization. Storing it in `.env` (gitignored) and injecting via env var is the correct pattern — it's already how the issue specifies it. Confirm `.env` is in `.gitignore`. - **GlitchTip receives exception data including the full stack trace and exception message.** If any exception message includes user-supplied input (e.g., a filename a user uploaded, a document title), that text reaches GlitchTip. With `send-default-pii: false`, Sentry won't add HTTP request bodies, but exception messages are always included. Audit `throw DomainException.internal(...)` call sites to confirm developer messages do not embed user-controlled strings verbatim. - **No new attack surface on the backend.** This is outbound-only: the backend POSTs to GlitchTip's ingest endpoint. No inbound port is opened. ### Recommendations - Audit the 8 `DomainException.internal(...)` call sites and the OCR/file error catch blocks where raw `e.getMessage()` is embedded in the exception message (e.g., `DocumentController.java:152`, `DocumentService.java:619`). Those `e.getMessage()` strings could contain S3 endpoint URLs, bucket names, or file paths. Since these exceptions are caught by `handleGeneric` and would reach Sentry, consider stripping the raw cause message: `"Failed to upload file — see logs for details"` rather than `"Failed to upload file: " + e.getMessage()`. This is a minor hardening step, not a blocking issue. - Confirm `.env` is in `.gitignore` (it should be, given the existing pattern with other secrets). The `SENTRY_DSN=` line in `.env.example` is correct — an empty default exposes nothing. - No GDPR concern from Sentry SDK itself given `send-default-pii: false`, but if GlitchTip is self-hosted in the EU (Hetzner), data residency is fine regardless.
marcel commented 2026-05-14 15:29:15 +02:00
Author
Owner
Copy Link

🧪 Sara Holt — QA Engineer & Test Strategist

Observations

  • The acceptance criteria are well-structured and testable. The happy-path ("event appears in GlitchTip") and negative-path ("DomainException.notFound does NOT appear") are both called out explicitly. This is good requirements practice.
  • The ACs that require a live GlitchTip DSN (SENTRY_DSN set to a real value) are manual verification only — they cannot be automated in unit or integration tests by design. This is acceptable for infrastructure wiring, but it should be acknowledged explicitly in the PR description so future contributors know these ACs aren't covered by the automated suite.
  • The ACs that don't require GlitchTip (startup log contains Sentry is disabled, ./mvnw test passes) are automatically verifiable in CI. They are already implicitly covered by the existing test suite running with an empty DSN.
  • There is no AC for the tracing configuration (traces-sample-rate: 1.0, enable-tracing: true). If these settings cause performance overhead or unexpected behavior in tests, it should be caught. A smoke test confirming the /actuator/health endpoint still responds within an acceptable time after the change would provide some coverage.
  • The issue does not specify a test for the 5xx DomainException case (e.g., DomainException.internal(...)). As Felix noted, these are also suppressed. If the intended behavior is that 5xx DomainException instances should also be suppressed (because they are "handled"), an explicit AC should say so. If they should be captured, a separate AC is needed.

Recommendations

  • Add an explicit AC: "When SENTRY_DSN is empty, ./mvnw test produces no Sentry-related errors or warnings." This is trivially automatable and gives CI confidence.
  • Mark the GlitchTip-dependent ACs as "manual verification against GlitchTip staging DSN" in the PR description so they're not treated as test gaps.
  • Consider adding a one-liner note in the PR: "These ACs are verified manually against GlitchTip; automated coverage is limited to SENTRY_DSN=empty startup behavior." This sets expectations for reviewers.
  • The JaCoCo branch coverage gate (88%) should not be affected by this change since it's configuration-only. Confirm no new code paths are introduced that would require test coverage to keep the gate green.
## 🧪 Sara Holt — QA Engineer & Test Strategist ### Observations - The acceptance criteria are well-structured and testable. The happy-path ("event appears in GlitchTip") and negative-path ("DomainException.notFound does NOT appear") are both called out explicitly. This is good requirements practice. - The ACs that require a live GlitchTip DSN (`SENTRY_DSN` set to a real value) are **manual verification only** — they cannot be automated in unit or integration tests by design. This is acceptable for infrastructure wiring, but it should be acknowledged explicitly in the PR description so future contributors know these ACs aren't covered by the automated suite. - The ACs that don't require GlitchTip (startup log contains `Sentry is disabled`, `./mvnw test` passes) are automatically verifiable in CI. They are already implicitly covered by the existing test suite running with an empty DSN. - There is no AC for the tracing configuration (`traces-sample-rate: 1.0`, `enable-tracing: true`). If these settings cause performance overhead or unexpected behavior in tests, it should be caught. A smoke test confirming the `/actuator/health` endpoint still responds within an acceptable time after the change would provide some coverage. - The issue does not specify a test for the **5xx `DomainException` case** (e.g., `DomainException.internal(...)`). As Felix noted, these are also suppressed. If the intended behavior is that 5xx `DomainException` instances should also be suppressed (because they are "handled"), an explicit AC should say so. If they should be captured, a separate AC is needed. ### Recommendations - Add an explicit AC: "When `SENTRY_DSN` is empty, `./mvnw test` produces no Sentry-related errors or warnings." This is trivially automatable and gives CI confidence. - Mark the GlitchTip-dependent ACs as "manual verification against GlitchTip staging DSN" in the PR description so they're not treated as test gaps. - Consider adding a one-liner note in the PR: "These ACs are verified manually against GlitchTip; automated coverage is limited to `SENTRY_DSN=empty` startup behavior." This sets expectations for reviewers. - The JaCoCo branch coverage gate (88%) should not be affected by this change since it's configuration-only. Confirm no new code paths are introduced that would require test coverage to keep the gate green.
marcel commented 2026-05-14 15:29:30 +02:00
Author
Owner
Copy Link

🚀 Tobias Wendt — DevOps & Platform Engineer

Observations

  • The docker-compose.yml change (SENTRY_DSN: ${SENTRY_DSN:-}) is correct and follows the existing env-var injection pattern used by all other secrets in the Compose file. The dash-default (:-) means the container starts cleanly with an empty string when .env doesn't define SENTRY_DSN. No risk here.
  • The .env.example line (SENTRY_DSN=) needs to actually be added — the issue body notes "confirm this line exists" but it was not found in the current .env.example. This is a required deliverable, not optional.
  • Version pinning: The issue specifies sentry-spring-boot-starter-jakarta at version 8.5.0 with a comment to "check for latest stable 8.x". This is the right pattern — pin to a specific version and let Renovate bump it. The comment is informational, not a process blocker.
  • The Sentry SDK will make outbound HTTPS calls to the GlitchTip ingest endpoint (typically https://glitchtip.your-domain.com/api/<project>/store/). This is a new outbound dependency for the backend container. Confirm the GlitchTip ingestion endpoint is accessible from the Docker internal network (or from the backend container's perspective). If Caddy proxies GlitchTip on the same VPS, the backend container will need to reach it via the internal Docker network or via the public hostname.
  • The application.yaml sentry.environment: ${SPRING_PROFILES_ACTIVE:dev} is clever — it reuses the existing profiles env var to tag Sentry events. This means events from the dev Docker Compose stack will be tagged dev,e2e (the current SPRING_PROFILES_ACTIVE value in docker-compose.yml). That's useful for filtering in GlitchTip.
  • No new Docker service is required. No new volume. No new port. This is a zero-infrastructure-cost change.

Recommendations

  • Add the SENTRY_DSN= line to .env.example — it's not there yet. This is a required deliverable per the issue's own AC.
  • Confirm the GlitchTip ingest URL is reachable from the archive-backend container. If GlitchTip runs on the same Compose stack or same VPS, test with docker exec archive-backend curl -s <glitchtip-ingest-url> during integration.
  • The docs/infrastructure/self-hosted-catalogue.md (referenced in Tobias's domain knowledge) should document the GlitchTip DSN pattern so future operators know where to get it and how to rotate it. This is a recommendation for the GlitchTip infrastructure issue, not this one.
  • No CI workflow changes are needed — the existing Maven test job runs with SENTRY_DSN absent (empty), so the Sentry is disabled path is exercised automatically on every CI run.
## 🚀 Tobias Wendt — DevOps & Platform Engineer ### Observations - The `docker-compose.yml` change (`SENTRY_DSN: ${SENTRY_DSN:-}`) is correct and follows the existing env-var injection pattern used by all other secrets in the Compose file. The dash-default (`:-`) means the container starts cleanly with an empty string when `.env` doesn't define `SENTRY_DSN`. No risk here. - The `.env.example` line (`SENTRY_DSN=`) needs to actually be added — the issue body notes "confirm this line exists" but it was **not found** in the current `.env.example`. This is a required deliverable, not optional. - **Version pinning**: The issue specifies `sentry-spring-boot-starter-jakarta` at version `8.5.0` with a comment to "check for latest stable 8.x". This is the right pattern — pin to a specific version and let Renovate bump it. The comment is informational, not a process blocker. - The Sentry SDK will make outbound HTTPS calls to the GlitchTip ingest endpoint (typically `https://glitchtip.your-domain.com/api/<project>/store/`). This is a new outbound dependency for the backend container. Confirm the GlitchTip ingestion endpoint is accessible from the Docker internal network (or from the backend container's perspective). If Caddy proxies GlitchTip on the same VPS, the backend container will need to reach it via the internal Docker network or via the public hostname. - The `application.yaml` `sentry.environment: ${SPRING_PROFILES_ACTIVE:dev}` is clever — it reuses the existing profiles env var to tag Sentry events. This means events from the dev Docker Compose stack will be tagged `dev,e2e` (the current `SPRING_PROFILES_ACTIVE` value in `docker-compose.yml`). That's useful for filtering in GlitchTip. - No new Docker service is required. No new volume. No new port. This is a zero-infrastructure-cost change. ### Recommendations - **Add the `SENTRY_DSN=` line to `.env.example`** — it's not there yet. This is a required deliverable per the issue's own AC. - Confirm the GlitchTip ingest URL is reachable from the `archive-backend` container. If GlitchTip runs on the same Compose stack or same VPS, test with `docker exec archive-backend curl -s <glitchtip-ingest-url>` during integration. - The `docs/infrastructure/self-hosted-catalogue.md` (referenced in Tobias's domain knowledge) should document the GlitchTip DSN pattern so future operators know where to get it and how to rotate it. This is a recommendation for the GlitchTip infrastructure issue, not this one. - No CI workflow changes are needed — the existing Maven test job runs with `SENTRY_DSN` absent (empty), so the `Sentry is disabled` path is exercised automatically on every CI run.
marcel commented 2026-05-14 15:29:41 +02:00
Author
Owner
Copy Link

📋 Elicit — Requirements Engineer

Observations

  • The issue is well-specified for a configuration-only change. It identifies the library, the configuration properties, the environment variable wiring, and gives clear acceptance criteria covering both the happy path and the negative case.
  • One unresolved ambiguity: The issue states "Only unexpected 5xx errors should be captured." The codebase has DomainException.internal(...) — a DomainException subtype that results in HTTP 500. The ignored-exceptions-for-type rule suppresses all DomainException instances, including these 5xx ones. The requirement as stated and the implementation as specified are in conflict. This needs an explicit decision:
    • Option A: "Suppress all DomainException regardless of HTTP status" — simplest, but misses real OCR/file-upload failures that are operationally significant.
    • Option B: "Suppress only 4xx DomainException; capture 5xx ones" — matches the stated intent, requires a BeforeSend hook or a separate exception hierarchy.
  • The dependency on the GlitchTip infrastructure issue is noted in the body but not tracked as a formal Gitea "blocked by" link. This means the PR could be merged before a DSN exists to verify against.
  • The traces-sample-rate: 1.0 + enable-tracing: true settings are included but not covered by any acceptance criterion. There is no AC for "tracing data appears in GlitchTip" and no AC for "tracing does not degrade performance." These settings add scope without corresponding verification.

Recommendations

  • Resolve the 5xx DomainException ambiguity before implementation starts. Add an explicit AC: either "DomainException.internal(...) results in a GlitchTip event" or "DomainException.internal(...) is also suppressed (document the rationale)."
  • Add a "blocked by" link to the GlitchTip infrastructure issue in Gitea so the dependency is formally tracked.
  • Either add ACs for tracing or remove those config lines from this issue's scope. Tracing is a separate concern from exception reporting. Delivering it in this issue without verification criteria is scope creep. Recommended: remove enable-tracing: true and traces-sample-rate: 1.0 from this issue, track them in a follow-up issue once GlitchTip tracing support is confirmed.
  • The Definition of Done is complete and actionable. The AC format is testable. The issue body is implementation-ready once the ambiguities above are resolved.
## 📋 Elicit — Requirements Engineer ### Observations - The issue is well-specified for a configuration-only change. It identifies the library, the configuration properties, the environment variable wiring, and gives clear acceptance criteria covering both the happy path and the negative case. - **One unresolved ambiguity**: The issue states "Only unexpected 5xx errors should be captured." The codebase has `DomainException.internal(...)` — a `DomainException` subtype that results in HTTP 500. The `ignored-exceptions-for-type` rule suppresses **all** `DomainException` instances, including these 5xx ones. The requirement as stated and the implementation as specified are in conflict. This needs an explicit decision: - Option A: "Suppress all `DomainException` regardless of HTTP status" — simplest, but misses real OCR/file-upload failures that are operationally significant. - Option B: "Suppress only 4xx `DomainException`; capture 5xx ones" — matches the stated intent, requires a `BeforeSend` hook or a separate exception hierarchy. - The dependency on the GlitchTip infrastructure issue is noted in the body but not tracked as a formal Gitea "blocked by" link. This means the PR could be merged before a DSN exists to verify against. - The `traces-sample-rate: 1.0` + `enable-tracing: true` settings are included but not covered by any acceptance criterion. There is no AC for "tracing data appears in GlitchTip" and no AC for "tracing does not degrade performance." These settings add scope without corresponding verification. ### Recommendations - **Resolve the 5xx DomainException ambiguity** before implementation starts. Add an explicit AC: either "DomainException.internal(...) results in a GlitchTip event" or "DomainException.internal(...) is also suppressed (document the rationale)." - **Add a "blocked by" link** to the GlitchTip infrastructure issue in Gitea so the dependency is formally tracked. - **Either add ACs for tracing or remove those config lines from this issue's scope.** Tracing is a separate concern from exception reporting. Delivering it in this issue without verification criteria is scope creep. Recommended: remove `enable-tracing: true` and `traces-sample-rate: 1.0` from this issue, track them in a follow-up issue once GlitchTip tracing support is confirmed. - The Definition of Done is complete and actionable. The AC format is testable. The issue body is implementation-ready once the ambiguities above are resolved.
marcel commented 2026-05-14 15:29:54 +02:00
Author
Owner
Copy Link

🗳️ Decision Queue — Action Required

2 decisions need your input before implementation starts.

Architecture / Requirements

  • How to handle DomainException.internal(...) (HTTP 500) — The ignored-exceptions-for-type: DomainException config suppresses ALL DomainException instances, including the 8+ places in the codebase where DomainException.internal(...) signals real operational failures (OCR unavailable, file upload failures, invite code generation). The stated goal is "only unexpected 5xx errors should be captured" — but these 5xx DomainException instances are "expected by the domain" yet still operationally significant. Choose one:
    • Option A (simple): Accept that all DomainException — including .internal() — are suppressed. Add an explicit AC noting this, and rely on logs for OCR/file-upload failure visibility.
    • Option B (precise): Use a Sentry BeforeSend configuration bean to re-allow DomainException instances where getStatus().is5xxServerError() is true.
      (Raised by: Felix, Elicit, Sara)

Scope

  • Whether to include distributed tracing settings in this issue — The config includes enable-tracing: true and traces-sample-rate: 1.0, but there are no acceptance criteria for tracing, and GlitchTip's self-hosted tracing support is partial compared to full Sentry SaaS. These settings add unverified scope. Choose one:
    • Option A (trim scope): Remove tracing settings from this issue; deliver only exception reporting. Track tracing in a follow-up once GlitchTip tracing is confirmed to work.
    • Option B (keep, document): Keep the settings but add an explicit note in the issue that tracing is best-effort and not covered by ACs in this milestone.
      (Raised by: Felix, Markus, Elicit)
## 🗳️ Decision Queue — Action Required _2 decisions need your input before implementation starts._ ### Architecture / Requirements - **How to handle `DomainException.internal(...)` (HTTP 500)** — The `ignored-exceptions-for-type: DomainException` config suppresses ALL `DomainException` instances, including the 8+ places in the codebase where `DomainException.internal(...)` signals real operational failures (OCR unavailable, file upload failures, invite code generation). The stated goal is "only unexpected 5xx errors should be captured" — but these 5xx `DomainException` instances are "expected by the domain" yet still operationally significant. Choose one: - **Option A (simple):** Accept that all `DomainException` — including `.internal()` — are suppressed. Add an explicit AC noting this, and rely on logs for OCR/file-upload failure visibility. - **Option B (precise):** Use a Sentry `BeforeSend` configuration bean to re-allow `DomainException` instances where `getStatus().is5xxServerError()` is true. _(Raised by: Felix, Elicit, Sara)_ ### Scope - **Whether to include distributed tracing settings in this issue** — The config includes `enable-tracing: true` and `traces-sample-rate: 1.0`, but there are no acceptance criteria for tracing, and GlitchTip's self-hosted tracing support is partial compared to full Sentry SaaS. These settings add unverified scope. Choose one: - **Option A (trim scope):** Remove tracing settings from this issue; deliver only exception reporting. Track tracing in a follow-up once GlitchTip tracing is confirmed to work. - **Option B (keep, document):** Keep the settings but add an explicit note in the issue that tracing is best-effort and not covered by ACs in this milestone. _(Raised by: Felix, Markus, Elicit)_
marcel commented 2026-05-15 08:12:10 +02:00
Author
Owner
Copy Link

Implementation complete — PR #592: #592

What was changed:

  • backend/pom.xml — added sentry-spring-boot-starter-jakarta 8.5.0
  • backend/src/main/resources/application.yaml — added sentry: block with DSN from env, profile-based environment name, 100% trace sampling, PII disabled, and DomainException on the ignore list
  • docker-compose.yml — added SENTRY_DSN: ${SENTRY_DSN:-} to the backend service (empty default keeps SDK disabled)
  • .env.example — SENTRY_DSN= was already present, no change needed

./mvnw clean package -DskipTests passes (BUILD SUCCESS).

Implementation complete — PR #592: https://git.raddatz.cloud/marcel/familienarchiv/pulls/592 **What was changed:** - `backend/pom.xml` — added `sentry-spring-boot-starter-jakarta` 8.5.0 - `backend/src/main/resources/application.yaml` — added `sentry:` block with DSN from env, profile-based environment name, 100% trace sampling, PII disabled, and `DomainException` on the ignore list - `docker-compose.yml` — added `SENTRY_DSN: ${SENTRY_DSN:-}` to the backend service (empty default keeps SDK disabled) - `.env.example` — `SENTRY_DSN=` was already present, no change needed `./mvnw clean package -DskipTests` passes (BUILD SUCCESS).
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
P0-critical
Blocks a core user journey, causes data loss, or is a security/accessibility barrier. Work on this before P1+.
 P1-high
Significant friction on a main user journey; workaround exists but is non-obvious. Next up after P0.
 P2-medium
Noticeable improvement; doesn't block anything; schedule opportunistically.
 P3-later
Cosmetic or parking-lot work; fine to stay open indefinitely.
 audit
Read-only audit / assessment work; produces a report rather than changing code
 bug
Something isn't working
 cleanup
Removal of dead code, vague comments, naming violations, and scratch artifacts
 collaboration
We want to extend the family archive to have more collaboration tools
 conversation
descoped
We will do that later
 devops
documentation
README, ARCHITECTURE, GLOSSARY, CONTRIBUTING, per-domain docs
 epic
Marker for epic-level issues that group multiple child issues
 feature
file-upload
legibility
Codebase Legibility Refactor — preparing the codebase for human evaluation and bus-factor reduction
 notification
person
phase-1: security
Security hygiene — must be done first
 phase-2: container-images
Production-ready multi-stage Docker images
 phase-3: prod-compose
Production compose overlay + Caddy reverse proxy
 phase-4: spring-prod-profile
Spring Boot production configuration profile
 phase-5: backups
Database and object storage backup strategy
 phase-6: deployment-docs
.env.example, DEPLOYMENT.md, runbook
 phase-7: monitoring
Prometheus, Loki, Grafana, Alertmanager
 refactor
Code restructuring without behaviour change
 security
test
ui
UI/UX and accessibility issues
 user
No Label P2-medium feature phase-7: monitoring
Milestone
No items
No Milestone Observability Stack — Grafana LGTM + GlitchTip
Projects
Clear projects
No project
Assignees
Clear assignees
Jens marcel
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#580
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?