fix(otel): remove clickhouse event repo feature flag support from v3, now v4 only

[ericallam]

No description provided.

[changeset-bot]

⚠️ No Changeset found

Latest commit: a81a7fb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Walkthrough

• Adds getV3EventRepository(parentStore) in apps/webapp/app/v3/eventRepository/index.server.ts. It selects the repository and store based on parentStore or env.EVENT_REPOSITORY_DEFAULT_STORE, supporting "clickhouse" or defaulting to the standard repository with getTaskEventStore().
• Updates apps/webapp/app/v3/services/triggerTaskV1.server.ts to import and use getV3EventRepository instead of getEventRepository. No other logic changes.

Estimated code review effort

Small scope across two files; one new selector function mirroring existing logic and one import/call-site rename. Low heterogeneity and minimal logic density.

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The provided title claims to remove clickhouse event repo feature flag support from v3 and restrict it to v4, but the changes actually introduce a new getV3EventRepository function and update its usage without removing any feature flag logic. Therefore the title does not accurately represent the main change.	Please revise the PR title to clearly describe the introduction of the getV3EventRepository function and related import updates rather than removal of clickhouse feature flag support.
Description Check	⚠️ Warning	The pull request lacks any description, so it does not follow the repository’s template which requires sections for the issue reference, checklist, testing steps, changelog, and screenshots.	Please add a detailed description following the repository template including the issue number, completion of the checklist, testing steps, a brief changelog entry, and any relevant screenshots.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ea-branch-93

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

apps/webapp/app/v3/eventRepository/index.server.ts (1)

41-57: LGTM! V3-specific event repository correctly bypasses feature flags.

The implementation correctly removes feature flag and rollout percentage logic, directly using env.EVENT_REPOSITORY_DEFAULT_STORE. This aligns with the PR objective to make clickhouse feature flag support V4-only.

Optional: Extract common parentStore handling logic.

Lines 44-50 duplicate the parentStore handling from getEventRepository (lines 24-30). Consider extracting this into a helper function to reduce duplication:

function resolveRepositoryFromParentStore(parentStore: string): { repository: IEventRepository; store: string } | null {
  if (typeof parentStore === "string") {
    if (parentStore === "clickhouse") {
      return { repository: clickhouseEventRepository, store: "clickhouse" };
    } else {
      return { repository: eventRepository, store: getTaskEventStore() };
    }
  }
  return null;
}

export async function getV3EventRepository(
  parentStore: string | undefined
): Promise<{ repository: IEventRepository; store: string }> {
  const fromParent = resolveRepositoryFromParentStore(parentStore);
  if (fromParent) return fromParent;

  if (env.EVENT_REPOSITORY_DEFAULT_STORE === "clickhouse") {
    return { repository: clickhouseEventRepository, store: "clickhouse" };
  } else {
    return { repository: eventRepository, store: getTaskEventStore() };
  }
}

Note: This refactor is optional as the duplication is limited and both functions may evolve independently.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b3b2553 and a81a7fb.

📒 Files selected for processing (2)

• apps/webapp/app/v3/eventRepository/index.server.ts (1 hunks)
• apps/webapp/app/v3/services/triggerTaskV1.server.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations

Files:

• apps/webapp/app/v3/eventRepository/index.server.ts
• apps/webapp/app/v3/services/triggerTaskV1.server.ts

{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

We use zod a lot in packages/core and in the webapp

Files:

• apps/webapp/app/v3/eventRepository/index.server.ts
• apps/webapp/app/v3/services/triggerTaskV1.server.ts

apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

When importing from @trigger.dev/core in the webapp, never import the root package path; always use one of the documented subpath exports from @trigger.dev/core’s package.json

Files:

• apps/webapp/app/v3/eventRepository/index.server.ts
• apps/webapp/app/v3/services/triggerTaskV1.server.ts

{apps/webapp/app/**/*.server.{ts,tsx},apps/webapp/app/routes/**/*.ts}

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Access environment variables only via the env export from app/env.server.ts; do not reference process.env directly

Files:

• apps/webapp/app/v3/eventRepository/index.server.ts
• apps/webapp/app/v3/services/triggerTaskV1.server.ts

apps/webapp/app/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)

Modules intended for test consumption under apps/webapp/app/**/*.ts must not read environment variables; accept configuration via options instead

Files:

• apps/webapp/app/v3/eventRepository/index.server.ts
• apps/webapp/app/v3/services/triggerTaskV1.server.ts

🧬 Code graph analysis (2)

apps/webapp/app/v3/eventRepository/index.server.ts (4)

apps/webapp/app/v3/eventRepository/clickhouseEventRepositoryInstance.server.ts (1)

• clickhouseEventRepository (6-9)

apps/webapp/app/v3/eventRepository/eventRepository.server.ts (1)

• eventRepository (1475-1475)

apps/webapp/app/v3/taskEventStore.server.ts (1)

• getTaskEventStore (53-55)

apps/webapp/app/env.server.ts (1)

• env (1203-1203)

apps/webapp/app/v3/services/triggerTaskV1.server.ts (1)

apps/webapp/app/v3/eventRepository/index.server.ts (1)

• getV3EventRepository (41-57)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)

• GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
• GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
• GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
• GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
• GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
• GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
• GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
• GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
• GitHub Check: typecheck / typecheck
• GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (2)

apps/webapp/app/v3/services/triggerTaskV1.server.ts (2)

25-25: LGTM! Import updated to V3-specific event repository.

The import correctly switches to getV3EventRepository, which removes feature flag dependency from the V3 code path as intended.

---

294-298: LGTM! Call site correctly updated.

The function call correctly uses getV3EventRepository with the parentStore parameter (derived from dependent/parent task runs), removing the feature flags parameter as intended by this PR.