feat: migrate analytics from Cloudflare Worker to SvelteKit hooks#2809
Merged
TorstenDittmann merged 10 commits intomainfrom Mar 18, 2026
Merged
feat: migrate analytics from Cloudflare Worker to SvelteKit hooks#2809TorstenDittmann merged 10 commits intomainfrom
TorstenDittmann merged 10 commits intomainfrom
Conversation
Removes the standalone log-collector Cloudflare Worker and replaces it with a native SvelteKit server hook implementation for Profound Analytics. This change simplifies the infrastructure by: - Eliminating the separate log-collector Worker deployment - Moving analytics logic directly into the app request lifecycle - Leveraging SvelteKit's handle hook for fire-and-forget logging Changes: - Delete log-collector/ directory (Worker code, config, and dependencies) - Add src/lib/server/profound-analytics.ts as a SvelteKit handle - Integrate profoundAnalytics into src/hooks.server.ts sequence The analytics middleware captures: - Request metadata (host, method, pathname, query params) - Client info (IP, user agent, referer) - Response metrics (status code, bytes sent) Analytics only activates when PROFOUND_API_URL and PROFOUND_API_KEY environment variables are configured, silently skipping otherwise. BREAKING CHANGE: The log-collector Cloudflare Worker is no longer used. Deployments should remove Worker routes and configure environment variables in the SvelteKit application instead.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR removes the standalone Cloudflare Worker "log-collector" (deleted: its package.json, tsconfig.json, wrangler.json, src/index.ts, and reduced .gitignore entries) and integrates server-side analytics into the SvelteKit app. It adds a new Handle hook at src/lib/server/profound-analytics.ts that batches and asynchronously POSTs log entries to PROFOUND_API_URL with PROFOUND_API_KEY, registers the hook in src/hooks.server.ts, updates environment examples and CI to include those secrets, and makes several prerender-related and small route signature adjustments plus a couple of package script tweaks. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can get early access to new features in CodeRabbit.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/server/profound-analytics.ts (2)
4-4: Environment variables read at module initialization time.Reading
$env/dynamic/privateat module scope (outside the handler function) captures values once when the module loads. While this typically works, it can cause issues if:
- The environment variables are set after the server starts
- You're using adapters that may evaluate modules before env is fully available
Consider moving the destructuring inside the handle function for guaranteed fresh reads on each request, or verify this pattern is acceptable for your deployment setup.
♻️ Suggested refactor to read env at request time
import type { Handle } from '@sveltejs/kit'; import { env } from '$env/dynamic/private'; -const { PROFOUND_API_URL, PROFOUND_API_KEY } = env; - export const profoundAnalytics: Handle = async ({ event, resolve }) => { + const { PROFOUND_API_URL, PROFOUND_API_KEY } = env; + // Short circuit if analytics is not configured if (!PROFOUND_API_URL || !PROFOUND_API_KEY) { return resolve(event); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/profound-analytics.ts` at line 4, The module currently destructures PROFOUND_API_URL and PROFOUND_API_KEY from env at module scope (const { PROFOUND_API_URL, PROFOUND_API_KEY } = env), which can stale if env is populated later; move that destructuring into the per-request handler (the function that processes each request—e.g., handle or whatever request entrypoint in this file) so the variables are read fresh on every request, updating any references in the file to use the newly-scoped constants; ensure no other module-level logic depends on those values remaining at load time.
14-23: Header size calculation may underestimate actual bytes sent.The calculation adds 4 bytes per header (
key.length + value.length + 4) to account for:and\r\n. However, this doesn't include:
- The HTTP status line (e.g.,
HTTP/1.1 200 OK\r\n)- The final
\r\nthat terminates headersAdditionally,
content-lengthmay be absent for streaming or chunked responses, resulting inbodySize = 0even when a body is sent. This is acceptable for analytics approximation, but worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/profound-analytics.ts` around lines 14 - 23, The header size calculation undercounts bytes: update the logic that computes headerSize/totalBytesSent to include the HTTP status line length (e.g., assume "HTTP/1.1 " + response.status + " " + response.statusText + "\r\n") and the final terminating "\r\n" after headers; keep adding each header's key + value + 4 for ": " and "\r\n". For body size, keep the current content-length parsing but, when contentLength is missing and response.body is available and not yet consumed, attempt to derive bodySize by cloning the response and using clone().arrayBuffer().byteLength (falling back to 0 if body is consumed or clone fails) so totalBytesSent becomes headerSize + 2 (final CRLF) + statusLineLength + computed bodySize.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/server/profound-analytics.ts`:
- Line 4: The module currently destructures PROFOUND_API_URL and
PROFOUND_API_KEY from env at module scope (const { PROFOUND_API_URL,
PROFOUND_API_KEY } = env), which can stale if env is populated later; move that
destructuring into the per-request handler (the function that processes each
request—e.g., handle or whatever request entrypoint in this file) so the
variables are read fresh on every request, updating any references in the file
to use the newly-scoped constants; ensure no other module-level logic depends on
those values remaining at load time.
- Around line 14-23: The header size calculation undercounts bytes: update the
logic that computes headerSize/totalBytesSent to include the HTTP status line
length (e.g., assume "HTTP/1.1 " + response.status + " " + response.statusText +
"\r\n") and the final terminating "\r\n" after headers; keep adding each
header's key + value + 4 for ": " and "\r\n". For body size, keep the current
content-length parsing but, when contentLength is missing and response.body is
available and not yet consumed, attempt to derive bodySize by cloning the
response and using clone().arrayBuffer().byteLength (falling back to 0 if body
is consumed or clone fails) so totalBytesSent becomes headerSize + 2 (final
CRLF) + statusLineLength + computed bodySize.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3baa97ae-19db-4ed8-a5ad-286b27817c4f
📒 Files selected for processing (7)
log-collector/.gitignorelog-collector/package.jsonlog-collector/src/index.tslog-collector/tsconfig.jsonlog-collector/wrangler.jsonsrc/hooks.server.tssrc/lib/server/profound-analytics.ts
💤 Files with no reviewable changes (5)
- log-collector/.gitignore
- log-collector/package.json
- log-collector/src/index.ts
- log-collector/wrangler.json
- log-collector/tsconfig.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/server/profound-analytics.ts (2)
109-109: Consider compliance implications of logging query parameters.Query parameters may contain PII such as email addresses, tokens, or user identifiers. Logging
query_paramsverbatim could have GDPR/CCPA compliance implications depending on what data flows through URLs.Consider whether sensitive parameters should be redacted or excluded.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/profound-analytics.ts` at line 109, The code currently logs query_params built via Object.fromEntries(event.url.searchParams) which can leak PII/tokens; update the logic in profound-analytics.ts where query_params is constructed to filter or redact sensitive keys before logging (e.g., detect common sensitive names like "token", "access_token", "auth", "password", "email", "session_id" and replace values with "[REDACTED]" or omit them), implement a small helper (e.g., sanitizeQueryParams or redactParams) and call it instead of Object.fromEntries(event.url.searchParams) so only non-sensitive parameters are logged.
81-82: Singleton starts interval timer even when analytics is disabled.The
LogBatcherconstructor callsstart()which creates asetIntervaltimer. This happens at module load time regardless of whetherPROFOUND_API_URLandPROFOUND_API_KEYare configured. Whileflush()short-circuits when disabled, the timer still runs every 10 seconds unnecessarily.Consider lazy initialization or checking the environment variables before starting.
♻️ Proposed fix for lazy initialization
-// Singleton instance -const logBatcher = new LogBatcher(); +// Singleton instance (lazy initialized) +let logBatcher: LogBatcher | null = null; + +function getLogBatcher(): LogBatcher { + if (!logBatcher) { + logBatcher = new LogBatcher(); + } + return logBatcher; +}Then in the handle function:
// Non-blocking: add to queue and return immediately - logBatcher.add(logData); + getLogBatcher().add(logData);This ensures the timer only starts when analytics is actually used.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/profound-analytics.ts` around lines 81 - 82, The LogBatcher singleton currently starts its interval timer in its constructor (LogBatcher.start()), which runs at module load even when analytics are disabled; change to lazy initialization so the timer only starts when analytics are enabled: remove/disable the automatic start() call from the LogBatcher constructor, add a factory/getter (e.g., getLogBatcher or initializeLogBatcher) that checks PROFOUND_API_URL and PROFOUND_API_KEY and only creates and calls start() on the LogBatcher when those env vars are present, and update the consumer (the handle function that calls logBatcher.flush()/enqueue) to obtain the instance via that getter so flush() short-circuits and no interval is created when analytics are disabled. Ensure references to LogBatcher, start(), flush(), and the handle caller are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/server/profound-analytics.ts`:
- Around line 59-77: The flush() implementation removes the batch immediately
via this.queue.splice(0) so failed sends are permanently lost; change the logic
to preserve and retry failed batches by either (a) copying the batch with const
batch = this.queue.slice(0) and only splice/remove items from this.queue after a
successful response, or (b) if you must splice first, push the batch back onto
this.queue when the fetch fails (including non-ok responses), and implement
retry logic with exponential backoff inside flush() (use a retry loop or helper
that retries fetch(PROFOUND_API_URL, ...) a few times with backoff before giving
up) so that failures are retried and not dropped.
- Line 110: The direct call to event.getClientAddress() can throw in adapters
that don't implement it; change the ip assignment in profound-analytics to
safely call it (e.g., check typeof event.getClientAddress === 'function' before
invoking or wrap the call in try/catch) and fall back to undefined or an empty
string on error so the request won't fail; update the code that sets ip:
event.getClientAddress() to use this safe-call pattern (referencing
event.getClientAddress()) and ensure any caught error is swallowed or logged but
not rethrown.
---
Nitpick comments:
In `@src/lib/server/profound-analytics.ts`:
- Line 109: The code currently logs query_params built via
Object.fromEntries(event.url.searchParams) which can leak PII/tokens; update the
logic in profound-analytics.ts where query_params is constructed to filter or
redact sensitive keys before logging (e.g., detect common sensitive names like
"token", "access_token", "auth", "password", "email", "session_id" and replace
values with "[REDACTED]" or omit them), implement a small helper (e.g.,
sanitizeQueryParams or redactParams) and call it instead of
Object.fromEntries(event.url.searchParams) so only non-sensitive parameters are
logged.
- Around line 81-82: The LogBatcher singleton currently starts its interval
timer in its constructor (LogBatcher.start()), which runs at module load even
when analytics are disabled; change to lazy initialization so the timer only
starts when analytics are enabled: remove/disable the automatic start() call
from the LogBatcher constructor, add a factory/getter (e.g., getLogBatcher or
initializeLogBatcher) that checks PROFOUND_API_URL and PROFOUND_API_KEY and only
creates and calls start() on the LogBatcher when those env vars are present, and
update the consumer (the handle function that calls logBatcher.flush()/enqueue)
to obtain the instance via that getter so flush() short-circuits and no interval
is created when analytics are disabled. Ensure references to LogBatcher,
start(), flush(), and the handle caller are updated accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e1842eaa-0ff7-46a5-b0c0-3eac77584462
📒 Files selected for processing (1)
src/lib/server/profound-analytics.ts
HarshMN2345
approved these changes
Mar 16, 2026
…com/appwrite/website into feat/migrate-analytics-to-sveltekit
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
package.json (1)
12-12: Consider adding a dedicated reinstall script for developer ergonomics.
cleannow behaves more predictably. If you still want the old one-shot behavior, add an explicit companion script (e.g.,clean:install) to avoid workflow surprises.♻️ Proposed follow-up script split
"scripts": { - "clean": "rm -rf node_modules && rm -rf .svelte_kit", + "clean": "rm -rf node_modules && rm -rf .svelte_kit", + "clean:install": "bun run clean && bun install", ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 12, The package.json "clean" script currently only removes node_modules and .svelte_kit; add a dedicated reinstall helper by creating a new npm script (e.g., "clean:install") that runs the one-shot behavior you previously had (clean then install) so developers have an explicit command for clean+install while keeping "clean" deterministic; reference the existing "clean" script name and add the new "clean:install" script entry to package.json.src/lib/server/profound-analytics.ts (1)
82-83: Consider lazy initialization for the singleton.The
LogBatcherconstructor starts the flush interval immediately on module import. Whileflush()short-circuits when env vars are missing, this still creates an unnecessary timer in environments where analytics isn't configured.♻️ Optional: Lazy initialization pattern
-// Singleton instance -const logBatcher = new LogBatcher(); +// Lazy singleton instance +let logBatcher: LogBatcher | null = null; + +function getLogBatcher(): LogBatcher { + if (!logBatcher) { + logBatcher = new LogBatcher(); + } + return logBatcher; +}Then use
getLogBatcher().add(logData)in the handler.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/profound-analytics.ts` around lines 82 - 83, The module currently creates a LogBatcher singleton at import time which starts its flush interval; change this to lazy initialization by replacing the eager const logBatcher = new LogBatcher() with a getLogBatcher() function that returns a single shared instance, creating it on first call; ensure the LogBatcher constructor or its start/flush timer is only invoked when the instance is created (so move timer startup into the constructor-if-still-needed-or-into an explicit start triggered by getLogBatcher()), and update call sites to use getLogBatcher().add(...) (preserving existing flush() behavior).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/server/profound-analytics.ts`:
- Around line 94-97: The code calls resolve(event) twice causing a new response
to be produced; change the non-HTML branch to return the already-resolved
response variable instead of calling resolve(event) again—i.e., after reading
const contentType = response.headers.get('content-type'); if it doesn't include
'text/html' return the response object you stored (response) rather than
invoking resolve(event) a second time; update the logic in the same function
where response, contentType, and resolve(event) are used.
---
Nitpick comments:
In `@package.json`:
- Line 12: The package.json "clean" script currently only removes node_modules
and .svelte_kit; add a dedicated reinstall helper by creating a new npm script
(e.g., "clean:install") that runs the one-shot behavior you previously had
(clean then install) so developers have an explicit command for clean+install
while keeping "clean" deterministic; reference the existing "clean" script name
and add the new "clean:install" script entry to package.json.
In `@src/lib/server/profound-analytics.ts`:
- Around line 82-83: The module currently creates a LogBatcher singleton at
import time which starts its flush interval; change this to lazy initialization
by replacing the eager const logBatcher = new LogBatcher() with a
getLogBatcher() function that returns a single shared instance, creating it on
first call; ensure the LogBatcher constructor or its start/flush timer is only
invoked when the instance is created (so move timer startup into the
constructor-if-still-needed-or-into an explicit start triggered by
getLogBatcher()), and update call sites to use getLogBatcher().add(...)
(preserving existing flush() behavior).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a1bcce74-69d2-4c2e-a7c0-b5771672de84
⛔ Files ignored due to path filters (1)
src/lib/generated/github-stars.jsonis excluded by!**/generated/**
📒 Files selected for processing (10)
package.jsonsrc/lib/server/profound-analytics.tssrc/routes/(init)/init/+page.server.tssrc/routes/(marketing)/+page.tssrc/routes/+layout.tssrc/routes/blog/[[page]]/+page.tssrc/routes/integrations/+page.tssrc/routes/llms-full.txt/+server.tssrc/routes/partners/catalog/+page.tssrc/routes/threads/+layout.ts
💤 Files with no reviewable changes (6)
- src/routes/(marketing)/+page.ts
- src/routes/blog/[[page]]/+page.ts
- src/routes/integrations/+page.ts
- src/routes/(init)/init/+page.server.ts
- src/routes/threads/+layout.ts
- src/routes/partners/catalog/+page.ts
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
2 participants
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.
Removes the standalone log-collector Cloudflare Worker and replaces it with a native SvelteKit server hook implementation for Profound Analytics.
This change simplifies the infrastructure by:
Changes:
The analytics middleware captures:
Analytics only activates when PROFOUND_API_URL and PROFOUND_API_KEY environment variables are configured, silently skipping otherwise.
BREAKING CHANGE: The log-collector Cloudflare Worker is no longer used. Deployments should remove Worker routes and configure environment variables in the SvelteKit application instead.
Summary by CodeRabbit
New Features
Refactor
Chores