You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Why does this change exist?
What problem does it solve, and why is this the right approach?
Fixes .md page analytics which were incorrect and most dropped and posthog removal and page-leave data logged which posthog had and now goes to existing setup.
Manual verification: Done on staging and checked on staging
Edge cases covered -
md page in browser triggers Website Page View
content negotiation header prefers md works as before
curl fetches a .md page triggers Bot Page Request with custom_prefers_markdown: true (this was false before)
curl fetches a normal HTML page triggers Bot Page Request with custom_prefers_markdown: false
normal browser browsing triggers via PageViewTracker, no regression
browser reads a page, scrolls, and navigates away triggers a Website Page Leave with the time on page (23.5s), scroll depth (70%), and the same session id as the page view.
headless browsers trigger as Bot Page View with no page-leave tracking
⚠️ Risk & Impact Assessment
What could break? How do we recover?
Blast radius: website analytics
Potential regressions:
Website Page View counts will rise slightly because .md views are now counted
Known caveat: a .md URL that 404s still logs a page view (the middleware runs before the response status is known) so browser-UA crawler sweeping invented .md paths would inflate server-side views.
Reviewed with the signoz-website-frontend-pr-review skill against contributing/site-code.md. Scope: proxy.ts, utils/logEvent.ts, utils/userClient.ts, hooks/useLogEvent.ts, components/Analytics/PageViewTracker.tsx, deletions (PostHog + /log route), and new tests.
Read project conventions
Review analytics changes
Review deletions (PostHog, /log route)
Review new tests
Post inline findings + summary
Overall this is a clean, well-tested change. The PostHog removal is complete (no lingering references outside the intentional legacy-migration keys in userClient.ts), the tunnel-only send path is simpler, and coverage for the new server-side .md page view, session migration, and keepalive transport is solid.
Code findings
Medium
pageLocation keeps the .md suffix (proxy.ts:174, inline) — server logs /docs/introduction.md while the client tracker logs /docs/introduction, so the same page splits into two rows in pageLocation breakdowns and won't line up in funnels keyed on the canonical path. pageTypePath (already computed one line above with .md stripped) is a drop-in fix; custom_content_type still distinguishes markdown fetches. If the split is intentional, worth a note in the PR.
Low
transport: 'fetch' | 'beacon' is now a slight misnomer (utils/logEvent.ts:38-47) — with sendBeacon gone, 'beacon' only toggles keepalive. Not blocking; a keepalive?: boolean option would read more truthfully, but the current shape keeps the call sites unchanged and is fine to leave.
Residual risks / testing gaps (no action required)
404 .md inflation — already documented in the PR: a browser-UA client sweeping invented .md paths logs a Website Page View because the middleware runs before the response status is known. Acceptable given the acknowledged blast radius; flagging so it's not a surprise in the data.
custom_content_type: 'text/markdown' is hardcoded but safe here — the only other markdownRewritePath branch (apiRefYamlRewrite) requires isBot === true, and this block is gated on !isBot, so it can never mislabel an OpenAPI-spec fetch.
I did not run yarn build / yarn lint (review-only); the added Vitest/node tests cover the changed logic well, but a CI build confirmation before merge is worth it given the DO NOT MERGE label.
Nothing here is blocking correctness — the pageLocation item is the one worth a decision before merge.
· fix/analytics-md
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
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.
Pull Request
📄 Summary
Fixes
.mdpage analytics which were incorrect and most dropped and posthog removal and page-leave data logged which posthog had and now goes to existing setup.Screenshots / Screen Recordings (if applicable)
NA, event verification manually done.
Issues closed by this PR
Closes https://github.com/SigNoz/growth-pod/issues/1254
✅ Change Type
Select all that apply
🧪 Testing Strategy
Website Page View.mdpage triggersBot Page Requestwithcustom_prefers_markdown: true(this wasfalsebefore)Bot Page Requestwithcustom_prefers_markdown: falseWebsite Page Leavewith the time on page (23.5s), scroll depth (70%), and the same session id as the page view.Bot Page Viewwith no page-leave trackingWebsite Page Viewcounts will rise slightly because.mdviews are now counted.mdURL that 404s still logs a page view (the middleware runs before the response status is known) so browser-UA crawler sweeping invented.mdpaths would inflate server-side views.Website Page Leaveis a new event📋 Checklist
👀 Notes for Reviewers