Skip to content

fix: serve analytics from our own origin so pageviews aren't lost - #31

Merged
KMKoushik merged 1 commit into
mainfrom
chore/umami-first-party
Oct 14, 2026
Merged

KMKoushik merged 1 commit into
mainfrom
chore/umami-first-party

Conversation

@KMKoushik

Copy link
Copy Markdown
Contributor

Loading the tracker from umami.kdawg.dev makes it a third-party script, and a good share of visitors never request it — the pageview is lost before it leaves the browser. #30 shipped it that way; this moves it to our own origin.

The tracker now loads from /stats/script.js. Umami derives its collect endpoint from the directory the script came from, so the browser posts to /stats/api/send. Both are rewrites onto API routes.

Why API routes and not a plain rewrite to the upstream

The hop now happens server-side, so without help Umami sees our address for every visitor. Its session key is uuid(website, ip, userAgent, …), so every visitor would fold into one session. Two things fall out of that:

  • /stats/api/send forwards the real client IP as x-splitpro-client-ip, and the Umami service has CLIENT_IP_HEADER=x-splitpro-client-ip set (already applied, see below). It also forwards the user agent, which Umami uses for device/browser/OS and for its bot filter, and replays x-umami-cache.
  • The upstream is Railway's own domain, not umami.kdawg.dev. That hostname is proxied by Cloudflare, which overwrites cf-ipcountry with the country of whoever connects — us. Umami prefers that header over its GeoIP lookup, so going through it put every visitor in one country. Verified: a forwarded hit claiming a US address was recorded as AU.

Also here

getClientIp existed in two copies (tRPC context and the NextAuth route) and I needed a third, so it moved to src/server/clientIp.ts. Same precedence as before (cf-connecting-ip, then x-forwarded-for, then the socket), now also ignoring empty header values instead of returning ''.

Verified locally

Against a throwaway Umami site, through next dev and the rewrites, then deleted:

  • GET /stats/script.js → 200, byte-identical to upstream, text/javascript, cached an hour.
  • POST /stats/api/send with x-forwarded-for: 8.8.8.8 and an iPhone UA → 200, recorded as country US, device mobile, correct path. Without the IP header it came back AU (my address) with everything collapsed into one visitor.

tsc --noEmit clean, next lint clean for these files, prettier clean. (13 files are prettier-dirty on main already; untouched here.)

Notes

  • CLIENT_IP_HEADER is already set on the Umami service, which is safe to do ahead of this merge: Umami only honours it when the header is present, and today's direct browser posts don't send it. So nothing changes until this deploys.
  • That setting does mean a forged x-splitpro-client-ip on a direct post to the Umami host would be believed. The collect endpoint is already public and unauthenticated and the website ID is public, so forged events were possible regardless — this doesn't widen it.
  • Blockers that filter first-party analytics will still block this, by design. Expect a real undercount either way; the numbers are a trend line, not a census.
  • UMAMI_UPSTREAM_URL defaults to the Railway domain in env.js, so no new Railway variable is needed on split-pro.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 13 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d25afe97-3321-44bc-813a-fb6da1db9b12
📥 Commits

Reviewing files that changed from the base of the PR and between 740dcc1 and 0c81bb6.

📒 Files selected for processing: 8
  • next.config.js
  • src/components/Site/constants.ts
  • src/env.js
  • src/pages/api/auth/[...nextauth].ts
  • src/pages/api/stats/script.ts
  • src/pages/api/stats/send.ts
  • src/server/api/trpc.ts
  • src/server/clientIp.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@KMKoushik
KMKoushik merged commit 5946d5a into main Oct 14, 2026
2 checks passed
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.

1 participant