Skip to content

feat(sentry): segment spans and errors by anon/authed state - #925

Open
thostetler wants to merge 3 commits into
adsabs:masterfrom
thostetler:feat/sentry-auth-segmentation
Open

feat(sentry): segment spans and errors by anon/authed state#925
thostetler wants to merge 3 commits into
adsabs:masterfrom
thostetler:feat/sentry-auth-segmentation

Conversation

@thostetler

@thostetler thostetler commented Sep 2, 2026

Copy link
Copy Markdown
Member

Sentry had no user data. The old setUser call carried only an anonymous flag, which Sentry drops for lacking an indexed key, so there was no way to tell whether an error or slow pageload hit a logged-in researcher or an anonymous visitor.

  • Tag spans and events with a low-cardinality auth value (authed/anon), set from a cookie at SDK init and kept in sync server-side
  • Read SENTRY_ENVIRONMENT in the Node config
  • sendDefaultPii stays false; this tag has no identity, but it is still new data on every event and needs ADS to confirm before merge

@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.15686% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.0%. Comparing base (4c73059) to head (6476d2b).

Files with missing lines Patch % Lines
src/middleware.ts 87.0% 3 Missing ⚠️
src/lib/sentryAuthTag.ts 94.2% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #925     +/-   ##
========================================
+ Coverage    67.8%   68.0%   +0.2%     
========================================
  Files         378     380      +2     
  Lines       44164   44192     +28     
  Branches     2377    2384      +7     
========================================
+ Hits        29937   30030     +93     
+ Misses      14179   14113     -66     
- Partials       48      49      +1     
Files with missing lines Coverage Δ
sentry.client.config.ts 96.4% <100.0%> (ø)
sentry.server.config.ts 96.7% <100.0%> (ø)
src/ssr-utils.ts 76.1% <100.0%> (+1.3%) ⬆️
src/lib/sentryAuthTag.ts 94.2% <94.2%> (ø)
src/middleware.ts 94.3% <87.0%> (-0.2%) ⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sentry had no user data at all. providers.tsx called Sentry.setUser with
only an anonymous flag, and Sentry drops a user object carrying no indexed
key, so the call never did anything — there was no way to tell whether an
error or a slow pageload hit a logged-in researcher or an anonymous visitor.

- Add an auth tag, values anon and authed, set from a client-readable
  scix_auth cookie at SDK init so the pageload span carries it
- Tag the per-request isolation scope in updateUserStateSSR and in
  middleware so http.server and edge spans are segmented too
- Replace the dead Sentry.setUser call with a setTag that corrects the
  value after a client-side login or logout
- Read SENTRY_ENVIRONMENT in the server config
The client-side tag used `anonymous === false`, which disagrees with
isAuthenticated() in two ways: a session with anonymous true and a username
other than anonymous@ads is authenticated, and an unhydrated store has
anonymous undefined. Both cases resolved to anon and overwrote the correct
tag that middleware had already set from the session.

- Drop authTagForUser and call isAuthenticated from the Telemetry effect,
  keeping auth-utils off the Sentry init path
- Skip the effect entirely while the store user is unhydrated, so unknown
  never overwrites the cookie value
- Export SENTRY_AUTH_TAG_NAME instead of repeating the tag key four times
- Match the session cookie's sameSite strict on scix_auth
- Cover the home-page early-return branch in the edge span tag test
@thostetler
thostetler force-pushed the feat/sentry-auth-segmentation branch from 9cad54e to ff8ebc8 Compare September 3, 2026 03:01
@thostetler
thostetler marked this pull request as ready for review September 3, 2026 15:09
Copilot AI lite review requested due to automatic review settings September 3, 2026 15:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There’s a blocker-level issue in the new Sentry config tests due to resetModules() ordering that can prevent observing Sentry.init calls reliably.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Risk summary: Medium risk. Changes touch request-time middleware + SSR utilities + client telemetry initialization; incorrect tagging or a middleware regression could impact observability across most navigations.

This PR adds a low-cardinality Sentry tag (auth=authed|anon) to segment spans/events by anonymous vs authenticated state across edge middleware, SSR, and the client SDK init path, and wires SENTRY_ENVIRONMENT into the Node Sentry config.

Changes:

  • Add scix_auth cookie + edge/SSR tagging to keep Sentry auth tag in sync with session state.
  • Tag initial client Sentry scope from cookie at SDK init, and update tag on client-side auth state changes.
  • Add SENTRY_ENVIRONMENT support for Node runtime and extend tests around Sentry config + middleware behavior.

Findings (priority order):

  • blockersentry.config.test.ts module reset ordering can break the test’s ability to observe Sentry.init calls

    • Impact: Config tests may become flaky or consistently fail because the config imports can call a different mocked init than the one inspected.
    • Location: sentry.config.test.ts:13-21
    • Minimal fix: Call vi.resetModules() before importing @sentry/nextjs, then import the config; optionally guard against missing calls.
    • Confidence: high
  • medium — SSR tagging passes an optional session flag without boolean coercion

    • Impact: If session.isAuthenticated is ever unset/undefined (e.g., unexpected session initialization gaps), tagging becomes implicit/ambiguous and can skew segmentation.
    • Location: src/ssr-utils.ts:26
    • Minimal fix: Coerce to a strict boolean (e.g., ctx.req.session.isAuthenticated === true) before passing to authTagForSession.
    • Confidence: medium
File summaries
File Description
src/ssr-utils.ts Tags SSR request scope with auth based on session state.
src/providers.tsx Updates Sentry auth tag after client-side auth changes once store is hydrated.
src/middleware.ts Writes scix_auth cookie and tags edge isolation scope for all middleware-handled requests.
src/middlewares/tests/middleware.routes.integration.test.ts Adds integration coverage for cookie emission and edge tagging without clobbering session cookies.
src/lib/sentryAuthTag.ts Centralizes auth tag name, cookie name, and mapping helpers.
src/lib/sentryAuthTag.test.ts Unit tests for tag mapping and cookie parsing behavior.
src/tests/ssr-utils.test.ts Adds SSR test coverage verifying the Sentry tag is set per session state.
sentry.client.config.ts Tags initial client scope from scix_auth cookie at SDK init.
sentry.server.config.ts Reads SENTRY_ENVIRONMENT for Node runtime Sentry environment.
sentry.config.test.ts New tests validating server environment and client cookie-based tagging.
.env.local.sample Documents SENTRY_ENVIRONMENT env var.
Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread sentry.config.test.ts
Comment on lines +13 to +21
const loadInitOptions = async (configPath: string): Promise<Record<string, unknown>> => {
const Sentry = await import('@sentry/nextjs');
vi.mocked(Sentry.init).mockClear();
vi.resetModules();
await import(configPath);

const [options] = vi.mocked(Sentry.init).mock.calls[0];
return { ...options };
};
Comment thread src/ssr-utils.ts
Next's ResponseCookies snapshots the set-cookie header when the response is
constructed and rewrites the whole header from that snapshot on every set,
while iron-session appends its session cookie straight to the headers.
Writing scix_auth through response.cookies.set() after initSession therefore
erased scix_session, so middleware stopped seeing anyone as authenticated —
eight middleware e2e tests failed on the missing cookie and the redirects
that followed from it.

- Append the scix_auth set-cookie header directly instead
- Add a regression test that appends a session cookie the way iron-session
  does, confirmed to fail against the previous implementation
- Read raw set-cookie headers in the auth cookie tests, since res.cookies
  only reflects cookies written through the ResponseCookies API
- Treat an undefined session isAuthenticated as anon in authTagForSession,
  since the iron-session shim types it optional
- Reset modules before importing the Sentry mock in the config tests, so the
  assertions cannot read a stale mock instance
@thostetler
thostetler force-pushed the feat/sentry-auth-segmentation branch from ff8ebc8 to 6476d2b Compare September 3, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants