Skip to content

feat: make posthog analytics configurable#3707

Merged
joeyorlando merged 4 commits intomainfrom
codex/configurable-posthog
Apr 11, 2026
Merged

feat: make posthog analytics configurable#3707
joeyorlando merged 4 commits intomainfrom
codex/configurable-posthog

Conversation

@joeyorlando
Copy link
Copy Markdown
Contributor

@joeyorlando joeyorlando commented Apr 10, 2026

Summary

  • Adds backend analytics config for PostHog using ARCHESTRA_ANALYTICS_POSTHOG_KEY and ARCHESTRA_ANALYTICS_POSTHOG_HOST, while preserving the existing hosted PostHog defaults.
  • Exposes the analytics PostHog key/host through the unauthenticated public config route so the frontend no longer hardcodes them.
  • Updates the PostHog provider to initialize from public config and keeps analytics disabled when ARCHESTRA_ANALYTICS=disabled.

@joeyorlando joeyorlando changed the title [codex] make posthog analytics configurable feat: make posthog analytics configurable Apr 10, 2026
@joeyorlando joeyorlando marked this pull request as ready for review April 10, 2026 18:36
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 10, 2026

Claude finished @joeyorlando's task in 3m 7s —— View job


PR Review: make posthog analytics configurable

  • Read repository CLAUDE.md guidelines
  • Analyze backend config changes
  • Analyze frontend posthog provider changes
  • Review tests for completeness
  • Review docs and deployment changes
  • Post final review feedback

Summary

Overall this is a clean, well-tested refactor. It correctly moves PostHog key/host out of the hardcoded frontend bundle and into the backend's /api/config/public endpoint, which is the right direction: it lets self-hosted deployments point analytics at their own PostHog instance without rebuilding the frontend, and removes the awkward build-time/runtime env var split on the frontend. Tests and docs are updated accordingly.

A few observations / suggestions below, ordered by significance. Nothing blocking.


Suggestions

1. The frontend "empty key" guard is effectively dead code given current backend behaviorplatform/frontend/src/app/_parts/posthog-provider.tsx:32

if (
  !isPublicConfigLoading &&
  analytics?.enabled &&
  analytics.posthog.key &&   // <— can never be falsy today
  !isPostHogInitializedRef.current
) {

getAnalyticsConfig() in platform/backend/src/config.ts:449 always returns a non-empty key because of ?.trim() || DEFAULT_POSTHOG_KEY. So analytics.posthog.key is guaranteed non-empty over the wire. The new test "does not initialize when analytics is enabled but key is empty" (posthog-provider.test.tsx:326) exercises a state the backend cannot produce.

Two reasonable choices:

  • Drop the analytics.posthog.key check (and the test), since the schema + backend already guarantee it.
  • Or, on the backend, allow key to be an empty string when the operator explicitly sets ARCHESTRA_ANALYTICS_POSTHOG_KEY="", and treat that as "no analytics" (see feat: get gmail MCP Server flow working #3 below). This would make the defensive check meaningful.

I'd lean toward the second option — it makes the "disable by unsetting key" path work intuitively. Fix this →

2. Ambiguity between "enabled + empty key" vs ARCHESTRA_ANALYTICS=disabledplatform/backend/src/config.ts:449

Right now there are two ways an operator could plausibly try to opt out:

  • ARCHESTRA_ANALYTICS=disabled (documented, supported)
  • ARCHESTRA_ANALYTICS_POSTHOG_KEY="" (silently falls back to Archestra's hosted project)

The latter is almost certainly surprising: a user explicitly setting an empty key does NOT expect their telemetry to be sent to Archestra's hosted project. Consider either:

  • Documenting that the key is ignored when empty and you must use ARCHESTRA_ANALYTICS=disabled, OR
  • Treating empty key as "disabled" (enabled: !!posthogKey && process.env.ARCHESTRA_ANALYTICS !== "disabled").

3. config.analytics is a snapshot of env at module load timeplatform/backend/src/config.ts:527

analytics: getAnalyticsConfig(),

This is consistent with how the rest of config.ts works, but note the getAnalyticsConfig export is only useful for unit tests — the route at platform/backend/src/routes/config.ts:42 returns config.analytics (the snapshot), not getAnalyticsConfig(). That's fine; just flagging because the getAnalyticsConfig export reads a bit like it would be re-evaluated per request. Non-issue, just a readability note.

4. PostHog init now races with the public-config query

Previously PostHog was initialized synchronously at mount from the bundled config. Now initialization waits on usePublicConfig() to resolve. That means any events or pageviews that happen before the query returns won't be captured. For most dashboards this is acceptable (the initial pageview delay is small), but worth being aware of — if early-boot analytics matter, you may want to inline the key via an SSR/env injection path rather than a network round-trip.

Since phc_ keys are public by design, one alternative would be to keep a build-time default in the frontend bundle and only hit the public-config endpoint to override it at runtime for custom-key deployments. Not required, just a thought.

5. Minor: section ordering in config.tsplatform/backend/src/config.ts:41-42

Per the repo's "Module Code Order" convention (CLAUDE.md), internal constants belong at the bottom of the file. DEFAULT_POSTHOG_KEY / DEFAULT_POSTHOG_HOST are added at the top of the file, far from getAnalyticsConfig (line 449). Either move them to the "Internal helpers" section at the bottom, or co-locate them right above getAnalyticsConfig. Trivial.

6. Backend test: consider asserting against the exact default keyplatform/backend/src/config.test.ts:58

key: expect.stringMatching(/^phc_/),

This is fine, but the regex is loose enough that a refactor that accidentally drops the default value and leaves a stray phc_ somewhere would still pass. Exact-string comparison against DEFAULT_POSTHOG_KEY (exported or re-declared in the test) would be more robust. Minor.

7. Test gap: no explicit "enabled" env var caseplatform/backend/src/config.test.ts

Tests cover undefined (default) and "disabled". A third test where ARCHESTRA_ANALYTICS is set to any non-"disabled" value (e.g. "enabled", "true", or garbage) would pin the current behavior ("anything but 'disabled' means enabled"). This is also worth calling out explicitly in docs/pages/platform-deployment.md so users don't assume ARCHESTRA_ANALYTICS=false would work.

8. Tiltfile.dev change looks correctplatform/dev/Tiltfile.dev:82

Now that the backend is the source of truth, setting ARCHESTRA_ANALYTICS=disabled on the backend process is the right knob. Good cleanup.

9. docker-entrypoint.sh / supervisord.conf cleanup is correctplatform/docker/scripts/docker-entrypoint.sh and platform/docker/supervisord/supervisord.conf:56

Removing the sed that propagated ARCHESTRA_ANALYTICS into NEXT_PUBLIC_ARCHESTRA_ANALYTICS is consistent with killing the frontend env-var path. Good.

10. types.gen.ts is generated — just confirm codegen matches

The diff adds analytics to GetPublicConfigResponses. Since this file is generated (pnpm codegen:api-client), please verify it was produced by running codegen rather than hand-edited, so the next regeneration doesn't churn it.


Non-issues / things done well

  • PostHog project keys (prefixed phc_) are designed to be public, so bundling/exposing via an unauthenticated endpoint is fine from a security standpoint.
  • Test was properly updated to mock usePublicConfig instead of the previous hack that redefined config.posthog.enabled via Object.defineProperty. Much cleaner.
  • config.test.ts correctly trims whitespace in the "custom env vars" test — nice touch verifying the .trim() behavior.
  • Strict Zod schema on the route response matches what the frontend consumes.
  • Documentation correctly updated in docs/pages/platform-deployment.md per the CLAUDE.md env-var guidelines.

Verdict

LGTM with minor suggestions. The two things I'd most like to see addressed before merge are:

  1. Clarify the "empty key" vs "disabled" behavior (gmail oauth wip #1 and Add correct email to the License #2).
  2. Verify types.gen.ts is from codegen (test #10).

Everything else is optional polish.
| Branch: codex/configurable-posthog

@joeyorlando joeyorlando merged commit 7ae9101 into main Apr 11, 2026
39 checks passed
@joeyorlando joeyorlando deleted the codex/configurable-posthog branch April 11, 2026 03:05
joeyorlando added a commit that referenced this pull request Apr 11, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.2.10](platform-v1.2.9...platform-v1.2.10)
(2026-04-11)


### Features

* make posthog analytics configurable
([#3707](#3707))
([7ae9101](7ae9101))


### Bug Fixes

* `/llm/costs` table scrolling
([#3722](#3722))
([6a42ba8](6a42ba8))
* apply MCP OAuth lifetime for gateway slugs
([#3711](#3711))
([362aaec](362aaec))
* Bedrock tool name encoding
([#3706](#3706))
([0e2c2d1](0e2c2d1))
* costs timeframes and surface limit reset settings
([#3709](#3709))
([6e4154b](6e4154b))
* jira oauth discovery overrides
([#3721](#3721))
([2c4cf8f](2c4cf8f))
* OIDC discovery trusted origins for IdP registration
([#3714](#3714))
([adb5f5e](adb5f5e))
* preserve shared chat agents on fork
([#3715](#3715))
([252edfc](252edfc))
* reranker model dropdown labels
([#3704](#3704))
([ebd1c8a](ebd1c8a))
* session logs loading state
([#3712](#3712))
([ffba126](ffba126))


### Miscellaneous Chores

* **ci:** add ID-JAG MCP e2e test
([#3702](#3702))
([1a5078a](1a5078a))
* **deps:** bump next from 16.1.7 to 16.2.3 in /platform/frontend
([#3708](#3708))
([d47967c](d47967c))
* use neutral token prefixes with legacy support
([#3719](#3719))
([db5929c](db5929c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: archestra-ci[bot] <222894074+archestra-ci[bot]@users.noreply.github.com>
Co-authored-by: Joey Orlando <joey@archestra.ai>
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