feat(observability): add Sentry error tracking across backend, web, and mobile#25
Merged
Conversation
…nd mobile - Backend: SentryMiddleware (sentry-go v0.46.2 + sentry-go/gin), registered first in Gin middleware chain, opt-in via SENTRY_DSN env var - Web: @sentry/nextjs v10.57.0 with client/server/edge config files, next.config wrapped with withSentryConfig, opt-in via NEXT_PUBLIC_SENTRY_DSN - Mobile: sentry-android v8.14.0 initialised in MainActivity.onCreate, guarded by BuildConfig.SENTRY_DSN (empty by default) - Docs: created observability.md for all three layers; fixed stale bootstrap, routing, middleware, environment docs; added auth.md
github-actions
Bot
added
area: backend
|
Warning Review limit reached
More reviews will be available in 41 minutes and 8 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds Sentry error-tracking integration across the Go backend, Next.js web app, and Android mobile app. Each platform reads a DSN from an environment variable, conditionally initializes Sentry (no-op when the DSN is absent), wires the SDK into the existing middleware/config pipeline, and adds unit tests and documentation. ChangesBackend Sentry Integration
Web (Next.js) Sentry Integration
Mobile (Android) Sentry Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/docs/environment.md (1)
22-36:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd SENTRY_DSN to environment variable table.
Per coding guidelines, "every new environment variable in Go
loadConfig()must appear inbackend/.env.examplewith a safe placeholder value and a comment explaining the format." TheSENTRY_DSNvariable is not documented in this file.Add a row to the environment variables table (after
RATE_LIMIT_BURSTor near other observability config):| `SENTRY_DSN` | `bootstrap.go`, `internal/server/server.go` | — | Sentry DSN for error tracking. Middleware is no-op when empty (optional). Format: `https://key@o0.ingest.sentry.io/projectid` |Also ensure
SENTRY_DSNappears inbackend/.env.examplewith a safe placeholder value and comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/docs/environment.md` around lines 22 - 36, The SENTRY_DSN environment variable is not documented in the environment variables table, which violates the coding guidelines requiring all environment variables in loadConfig() to be documented. Add a new row to the environment variables table in backend/docs/environment.md after RATE_LIMIT_BURST with the SENTRY_DSN entry showing it is used in bootstrap.go and internal/server/server.go, has no default value, and explains it is for Sentry error tracking with optional usage. Additionally, add SENTRY_DSN to backend/.env.example with a safe placeholder value (such as a commented example URL format) and a descriptive comment explaining its purpose and optional nature.Source: Coding guidelines
backend/docs/middleware.md (1)
15-35:⚠️ Potential issue | 🟠 MajorUpdate
Registration ordersection to includeSentryMiddlewareas the first middleware.The code in
internal/transport/handlers/routes.goregistersSentryMiddlewareas the first handler in the chain, but theRegistration ordersection ofmiddleware.mdomits it entirely. Update lines 15–35 to reflect the actual order:Updated registration order
// 1. Sentry (first in chain; no-op when SENTRY_DSN is unset) r.Use(middleware.SentryMiddleware(sentryDSN)) // 2. Recovery + logger (debug: gin.Logger, release: middleware.Logger) r.Use(gin.Recovery(), middleware.Logger()) // 3. Rate limiter (no-op when RPS <= 0) r.Use(middleware.RateLimit(rps, burst)) // 4. CORS r.Use(cors.New(...)) // Global routes (no auth): r.GET("/", h.HelloWorldHandler) r.GET("/health", h.HealthHandler) r.GET("/swagger/*any", ginSwagger.WrapHandler(swaggerFiles.Handler)) // Protected group — FirebaseAuth applied when verifier != nil: api := r.Group("/api/v1") if verifier != nil { api.Use(middleware.FirebaseAuth(verifier)) } api.GET("/me", h.MeHandler)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/docs/middleware.md` around lines 15 - 35, The Registration order section in middleware.md is missing SentryMiddleware, which is registered first in the actual codebase. Update the section to add SentryMiddleware as item 1 with the comment indicating it is the first in chain and no-op when SENTRY_DSN is unset, then renumber the subsequent middleware steps (Recovery + logger becomes 2, Rate limiter becomes 3, CORS becomes 4) to match the actual registration order in internal/transport/handlers/routes.go.backend/docs/bootstrap.md (1)
42-51:⚠️ Potential issue | 🟡 MinorUpdate bootstrap.md to reflect SentryDSN in Config and clarify Sentry middleware initialization location.
The
Configstruct in the documentation (lines 27–38) is missing theSentryDSNfield, which is present in the code and read inloadConfig(). Additionally, theRun sequencedoes not acknowledge Sentry at all, even as a no-op when the DSN is absent.Since
SentryMiddlewareis initialized inserver.NewServer()viaRegisterRoutes()(not inbootstrap.Run()), update the documentation to:
- Add
SentryDSNto the Config struct listing (afterFirebaseServiceAccountJSON)- Either note in the Run sequence that Sentry DSN is loaded but middleware is applied later, or add a note after step 7 clarifying: "Sentry middleware is attached during HTTP server routing in
server.NewServer(), not in bootstrap."Current code shows SentryDSN is loaded and applied here:
bootstrap.Run: cfg := loadConfig() reads SENTRY_DSN env var server.NewServer: h.RegisterRoutes(..., app.Config.SentryDSN) applies middleware🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/docs/bootstrap.md` around lines 42 - 51, Update the bootstrap.md documentation to include missing Sentry configuration details. First, add the SentryDSN field to the Config struct listing (after FirebaseServiceAccountJSON) to document that this environment variable is loaded during configuration. Second, clarify in the Run sequence that while loadConfig() reads the SENTRY_DSN environment variable as part of step 1, the actual SentryMiddleware initialization occurs later in server.NewServer() during HTTP server routing, not within bootstrap.Run() itself. You can either add this as a note after step 7 or mention it inline with step 1 to indicate that the DSN is loaded but the middleware attachment happens separately in RegisterRoutes().
🧹 Nitpick comments (2)
web/__tests__/sentry.test.ts (1)
8-20: ⚡ Quick winStrengthen these tests to verify Sentry init contract, not just import success.
Right now, imports can succeed even if config wiring breaks. Assert that
initis called with expected fields (dsn,tracesSampleRate,debug) per module and environment setup.Suggested test shape
import { vi, describe, it, expect } from "vitest"; -vi.mock("`@sentry/nextjs`", () => ({ - init: vi.fn(), -})); +const initMock = vi.fn(); +vi.mock("`@sentry/nextjs`", () => ({ init: initMock })); describe("Sentry config", () => { + it("server config calls init with expected options", async () => { + process.env.NEXT_PUBLIC_SENTRY_DSN = "https://example@sentry.io/123"; + initMock.mockClear(); + await import("../sentry.server.config"); + expect(initMock).toHaveBeenCalledWith( + expect.objectContaining({ + dsn: "https://example@sentry.io/123", + tracesSampleRate: 1.0, + debug: false, + }), + ); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/__tests__/sentry.test.ts` around lines 8 - 20, The current tests for the Sentry configuration modules (sentry.client.config, sentry.server.config, sentry.edge.config) only verify that imports succeed without checking the actual Sentry initialization contract. Replace the simple import resolution checks with tests that mock the Sentry init function and verify it is called with the expected configuration fields. For each of the three test cases (client, server, edge), mock the Sentry module's init function before importing the config module, then assert that init was called with an object containing the required fields: dsn, tracesSampleRate, and debug. This ensures the configuration modules not only load but actually initialize Sentry correctly with the proper setup.web/sentry.server.config.ts (1)
5-5: Use environment-driven trace sampling instead of hardcoded1.0for production.Both Sentry configs hardcode
tracesSampleRate: 1.0, which captures every transaction and can lead to excessive telemetry volume and cost in production. Sentry recommends 10–20% (0.1–0.2) for production environments.Update both files to read the sampling rate from an environment variable or apply a safer default for production:
web/sentry.server.config.ts#L5web/sentry.edge.config.ts#L5Consider using a
tracesSamplerfunction for more granular control across different transaction types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/sentry.server.config.ts` at line 5, The hardcoded tracesSampleRate: 1.0 in both web/sentry.server.config.ts (line 5) and web/sentry.edge.config.ts (line 5) captures every transaction, leading to excessive telemetry volume and cost in production. Replace the hardcoded 1.0 value at both locations with an environment-driven sampling rate read from an environment variable (e.g., SENTRY_TRACES_SAMPLE_RATE) with a safer production default like 0.1 or 0.2. Alternatively, consider implementing a tracesSampler function to provide more granular control based on transaction types across both configuration files.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/.env.example`:
- Around line 14-16: The FIREBASE_SERVICE_ACCOUNT_JSON and FIREBASE_PROJECT_ID
environment variables use inline comments (=# format) which standard dotenv
parsers interpret as literal values rather than comments. Remove the inline
comment text from these variables, leaving them with empty values (=), and move
the guidance information to separate comment-only lines placed above each
variable definition, matching the correct format already used by SENTRY_DSN.
In `@backend/docs/bootstrap.md`:
- Around line 30-37: The Config struct documentation is incomplete and missing
an entry for the SentryDSN field that exists in the actual Config struct. Add
the SentryDSN field to the documentation list with the same formatting and
structure as the other fields (Port, Env, DB, RedisURL, RateLimitRPS,
RateLimitBurst, FirebaseProjectID, FirebaseServiceAccountJSON), specifying it as
a string type to match the implementation.
In `@backend/docs/observability.md`:
- Around line 66-68: The markdown code fence containing the SENTRY_DSN
configuration example is missing a language label, which violates markdown lint
rules. Add "dotenv" as the language specifier immediately after the opening
triple backticks (```) to properly label this as a dotenv environment file
format.
In `@backend/docs/routing.md`:
- Line 54: The RegisterRoutes function signature documented in the routing.md
file is missing the sentryDSN parameter that is present in the actual
implementation. Update the documented RegisterRoutes method signature to include
the sentryDSN string parameter as the fourth parameter (after verifier).
Additionally, add documentation explaining that this sentryDSN parameter is
passed to middleware.SentryMiddleware(sentryDSN) in the middleware chain to
ensure the documentation accurately reflects the current implementation.
In `@backend/internal/transport/handlers/routes.go`:
- Around line 20-24: The SentryMiddleware function is discarding the error from
sentry.Init(...) and RegisterRoutes has no error return path, which silently
swallows Sentry initialization failures. Update the SentryMiddleware function
signature at backend/internal/transport/middleware/sentry.go:16 to return
(gin.HandlerFunc, error) instead of just gin.HandlerFunc, capturing and
returning the error from sentry.Init(...). Then update the RegisterRoutes
function signature at backend/internal/transport/handlers/routes.go:20 to return
(http.Handler, error) instead of just http.Handler, and propagate the error
returned by SentryMiddleware when it is called. Finally, update the call site in
backend/internal/server/server.go that invokes RegisterRoutes to handle the
returned error instead of ignoring it.
In `@backend/internal/transport/middleware/sentry.go`:
- Around line 16-17: The error returned by sentry.Init() in the initialization
block is being silently discarded with `_`, violating the principle of not
swallowing errors. Since the function returns gin.HandlerFunc and cannot return
an error, address this by either: (1) adding a logger parameter and logging the
actual error from sentry.Init() to surface configuration issues, (2) adding a
comment explaining why silent fallback is intentional as a tradeoff for graceful
degradation of observability, or (3) moving the Sentry initialization and
validation logic to server.NewServer where you have access to *slog.Logger and
can properly validate the initialization before wiring the middleware. Choose
the approach that aligns with your error handling strategy.
In `@mobile/app/build.gradle.kts`:
- Line 22: The buildConfigField for SENTRY_DSN on line 22 is hardcoded to an
empty string instead of reading from local.properties. Replace the hardcoded
empty value by reading the SENTRY_DSN property from the gradle properties (which
loads from local.properties), using a safe pattern like project.findProperty()
with an appropriate default value if the property is not defined. This allows
the DSN to be injected at build time from local.properties without modifying
tracked files.
In `@web/docs/_index.md`:
- Line 13: The documentation index table in the Observability row contains a
stale reference to `.env.local.example` which no longer exists. Replace this
filename with `web/.env.example` in the table entry on line 13 to point to the
correct environment configuration example file and ensure the documentation
reference is accurate.
In `@web/docs/observability.md`:
- Around line 49-51: The fenced code block containing the Sentry DSN
configuration is missing a language identifier after the opening triple
backticks, which violates markdown linting rule MD040. Add a language identifier
to the code fence at line 49 by changing the opening ``` to ```env (or ```bash)
to specify that this is environment variable configuration and satisfy lint
compliance.
---
Outside diff comments:
In `@backend/docs/bootstrap.md`:
- Around line 42-51: Update the bootstrap.md documentation to include missing
Sentry configuration details. First, add the SentryDSN field to the Config
struct listing (after FirebaseServiceAccountJSON) to document that this
environment variable is loaded during configuration. Second, clarify in the Run
sequence that while loadConfig() reads the SENTRY_DSN environment variable as
part of step 1, the actual SentryMiddleware initialization occurs later in
server.NewServer() during HTTP server routing, not within bootstrap.Run()
itself. You can either add this as a note after step 7 or mention it inline with
step 1 to indicate that the DSN is loaded but the middleware attachment happens
separately in RegisterRoutes().
In `@backend/docs/environment.md`:
- Around line 22-36: The SENTRY_DSN environment variable is not documented in
the environment variables table, which violates the coding guidelines requiring
all environment variables in loadConfig() to be documented. Add a new row to the
environment variables table in backend/docs/environment.md after
RATE_LIMIT_BURST with the SENTRY_DSN entry showing it is used in bootstrap.go
and internal/server/server.go, has no default value, and explains it is for
Sentry error tracking with optional usage. Additionally, add SENTRY_DSN to
backend/.env.example with a safe placeholder value (such as a commented example
URL format) and a descriptive comment explaining its purpose and optional
nature.
In `@backend/docs/middleware.md`:
- Around line 15-35: The Registration order section in middleware.md is missing
SentryMiddleware, which is registered first in the actual codebase. Update the
section to add SentryMiddleware as item 1 with the comment indicating it is the
first in chain and no-op when SENTRY_DSN is unset, then renumber the subsequent
middleware steps (Recovery + logger becomes 2, Rate limiter becomes 3, CORS
becomes 4) to match the actual registration order in
internal/transport/handlers/routes.go.
---
Nitpick comments:
In `@web/__tests__/sentry.test.ts`:
- Around line 8-20: The current tests for the Sentry configuration modules
(sentry.client.config, sentry.server.config, sentry.edge.config) only verify
that imports succeed without checking the actual Sentry initialization contract.
Replace the simple import resolution checks with tests that mock the Sentry init
function and verify it is called with the expected configuration fields. For
each of the three test cases (client, server, edge), mock the Sentry module's
init function before importing the config module, then assert that init was
called with an object containing the required fields: dsn, tracesSampleRate, and
debug. This ensures the configuration modules not only load but actually
initialize Sentry correctly with the proper setup.
In `@web/sentry.server.config.ts`:
- Line 5: The hardcoded tracesSampleRate: 1.0 in both
web/sentry.server.config.ts (line 5) and web/sentry.edge.config.ts (line 5)
captures every transaction, leading to excessive telemetry volume and cost in
production. Replace the hardcoded 1.0 value at both locations with an
environment-driven sampling rate read from an environment variable (e.g.,
SENTRY_TRACES_SAMPLE_RATE) with a safer production default like 0.1 or 0.2.
Alternatively, consider implementing a tracesSampler function to provide more
granular control based on transaction types across both configuration files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ab240e5d-2a9d-4e9a-80f2-17e8ce64f4e3
⛔ Files ignored due to path filters (2)
backend/go.sumis excluded by!**/*.sumweb/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
backend/.env.examplebackend/docs/_index.mdbackend/docs/auth.mdbackend/docs/bootstrap.mdbackend/docs/environment.mdbackend/docs/middleware.mdbackend/docs/observability.mdbackend/docs/routing.mdbackend/go.modbackend/internal/bootstrap/bootstrap.gobackend/internal/bootstrap/bootstrap_test.gobackend/internal/server/server.gobackend/internal/transport/handlers/routes.gobackend/internal/transport/middleware/sentry.gobackend/internal/transport/middleware/sentry_test.gomobile/app/build.gradle.ktsmobile/app/src/main/java/com/company/template/MainActivity.ktmobile/app/src/test/java/com/company/template/SentryInitTest.ktmobile/docs/_index.mdmobile/docs/observability.mdmobile/gradle/libs.versions.tomlweb/.env.exampleweb/.gitignoreweb/__tests__/sentry.test.tsweb/docs/_index.mdweb/docs/observability.mdweb/next.config.tsweb/package.jsonweb/sentry.client.config.tsweb/sentry.edge.config.tsweb/sentry.server.config.ts
- backend/.env.example: separate inline comments from FIREBASE var values so dotenv parsers don't treat # as a literal value - backend/docs/bootstrap.md: add missing SentryDSN field to Config struct docs - backend/docs/routing.md: update RegisterRoutes signature and NewServer wiring snippet to include sentryDSN param - backend/docs/observability.md: add dotenv language tag to code fence - middleware/sentry.go: log sentry.Init errors instead of discarding with _; degrade to no-op on failure - mobile/build.gradle.kts: read SENTRY_DSN from local.properties (gitignored) instead of hardcoding empty string - web/docs: fix stale .env.local.example references to .env.example; add language tag to code fence Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sentry-go v0.46.2+sentry-go/gin:SentryMiddleware(dsn)registered first in the Gin chain (no-op whenSENTRY_DSNis unset); panics are re-raised after capture so the existing Recovery middleware still handles them@sentry/nextjs v10.57.0: per-runtime config files (client / server / edge),next.config.tswrapped withwithSentryConfig, opt-in viaNEXT_PUBLIC_SENTRY_DSNsentry-android v8.14.0: initialised inMainActivity.onCreatebeforesetContent, guarded byBuildConfig.SENTRY_DSN(empty string default = disabled); DSN is injected at build time, never in sourceobservability.mdfor all three layers; stalebootstrap,routing,middleware,environmentdocs corrected for Firebase additions; newauth.mdcreatedActivating Sentry
Set the DSN in the appropriate env file — all three default to disabled:
SENTRY_DSNbackend/.envNEXT_PUBLIC_SENTRY_DSNweb/.env.localSENTRY_DSN(build-time)buildConfigFieldoverride or-PSENTRY_DSN=...CLI flagFill in
organdprojectinweb/next.config.tsto enable source map uploads.Test plan
go test ./internal/transport/middleware/...—TestSentryMiddleware_NoDSN_IsNoOpandTestSentryMiddleware_WithDSN_ReturnsHandlerpassgo test ./internal/bootstrap/...—TestLoadConfig_SentryDSN_SetandTestLoadConfig_SentryDSN_Unsetpasspnpm test— 3 Sentry config import tests pass;pnpm buildclean./gradlew test—SentryInitTest(3 cases) passes;./gradlew assembleDebugand./gradlew lintcleanSummary by CodeRabbit
New Features
Documentation
Chores