chore(cdp): drop the warpstream-calculated-events producer - #89622
chore(cdp): drop the warpstream-calculated-events producer#89622gustavohstrassburger wants to merge 1 commit into
Conversation
The CDP producer registry connects every registered producer at pod boot and throws if any connect fails. WARPSTREAM_CALCULATED_EVENTS_PRODUCER has no CDP output routing to it, but it still opens a connection to the warpstream-calculated-events cluster on every app that inherits the shared CDP config. Once that cluster's broker list goes away, the config falls back to the default and the connect times out, exiting the process. Remove the producer entirely: the registry registration, the name and type, the config map, the env-var type, and the defaults, plus the test registry slot. Deployment env vars for the removed prefix become inert, since unknown env vars are ignored by config parsing. Generated-By: PostHog Desktop Task-Id: 054a4733-22c6-48a8-9de6-c2e58d916ade
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
@greptileai please review |
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes. |
|
Reviews (1): Last reviewed commit: "chore(cdp): drop the warpstream-calculat..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Removes the now-dead warpstream-calculated-events CDP producer from the Node.js CDP producer registry so CDP pods no longer attempt to connect to a decommissioned Kafka/WarpStream cluster during startup.
Changes:
- Drop the
WARPSTREAM_CALCULATED_EVENTS_PRODUCERconstant/type and its env-var config map/defaults from the CDP producers module. - Deregister the producer from
createCdpProducerRegistryand remove the corresponding test registry slot. - Remove the deleted env-config type/defaults from the global
PluginsServerConfigand default config assembly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| nodejs/tests/helpers/cdp.ts | Removes the deleted producer from the test producer registry map. |
| nodejs/src/types.ts | Drops the deleted producer env-config type from PluginsServerConfig composition. |
| nodejs/src/common/config/config.ts | Stops including deleted producer env defaults in getDefaultConfig(). |
| nodejs/src/cdp/outputs/producers.ts | Deletes the producer name/type and its env-var config map + defaults; narrows CdpProducerName. |
| nodejs/src/cdp/outputs/producer-registry.ts | Removes the producer registration and updates the producer list comment accordingly. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from each directory's |
|
Reviews (2): Last reviewed commit: "chore(cdp): drop the warpstream-calculat..." | Re-trigger Greptile |
Problem
A CDP pod that inherits the shared CDP config today opens a Kafka connection to the
warpstream-calculated-eventscluster at boot, even though no CDP output sends anything to it. The producer registry connects every registered producer during startup and throws if any connect fails.That makes the dead producer a boot-time dependency on a cluster that is being decommissioned. When the cluster's broker list is removed from deployment config, the value falls back to its default, the connect times out, and the process exits — on every app inheriting the shared CDP config, not only the one consumer people associate with that cluster.
Refs #82624
Changes
warpstream-calculated-eventscluster at boot, so removing that cluster's broker list can no longer crash them.Deploy order
KAFKA_WARPSTREAM_CALCULATED_EVENTS_PRODUCER_*block and the cluster definition.Reversing 1 and 2 breaks every CDP pod. The producer would still be registered while its broker list is gone, so
metadata.broker.listfalls back to thekafka:9092zod default innodejs/src/common/outputs/kafka-producer-config.ts. That does not resolve in production,build()rejects, and the process exits on every app inheriting the shared CDP config.How did you test this code?
Automated only; the agent did not exercise a running pod.
pnpm typescript:checkinnodejs/— no errors in the touched files. The remaining errors are pre-existing and come from workspace packages (@posthog/hogvm,@posthog/replay-anonymizer) that were not built in this environment.npx eslinton the touched files — clean.npx jest src/common/outputs— the producer registry and builder suites pass.No tests were added. The removal is covered by the type system: the registry builder accumulates each registered producer's config keys into a type parameter that
build()checks against the config object, andCdpProducerNameis what every producer-selecting config field is typed as. A route written in source and left pointing at the removed producer would fail typecheck. That does not cover a route supplied as an env-var string:overrideWithEnvassigns the raw value with no membership check, so a stale*_PRODUCERenv var would resolve to no producer at runtime rather than failing to compile.Not checked: that no deployed environment sets a producer-selecting env var (for example
HOG_INVOCATION_RESULTS_PRODUCER) to the removed name. A search of deployment config found the removed prefix only in env-var definitions, never as a routing value, and every in-repo default routes elsewhere.Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written by Claude Code in a PostHog Desktop cloud task, working from the decommission sequence in the linked issue, which names this as a hard prerequisite for the cluster teardown step.
The issue asks only to deregister the producer from
createCdpProducerRegistry. Full removal was chosen over deleting the single.register()line: the definition, config map, and env-var type have no remaining consumer, and leaving them behind invites the next person to re-register a producer for a cluster that will not exist. Before removing the env-var type, deployment config was checked to confirm the cluster only supplies producer settings and never appears as a routing value, and that unknown env vars are ignored rather than rejected.Created with PostHog Desktop