Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: CDP Internal events processor #27007

Merged
merged 51 commits into from
Dec 27, 2024
Merged

Conversation

benjackwhite
Copy link
Contributor

@benjackwhite benjackwhite commented Dec 18, 2024

Problem

Needs https://github.com/PostHog/posthog-cloud-infra/pull/3376

We want to trigger CDP destinations via "internal events".
This sets up the internal events stream and service to process it.

Screenshot 2024-12-19 at 15 07 50 Screenshot 2024-12-19 at 15 07 56

Changes

  • New consumer to consume from new topic
  • Proper event validation with Zod
  • Uses new hog function types for loading from the DB
  • Hooked up activity log (simple version)
  • Disables a bunch of messaging stuff for now (I will fix in follow up but given that is all pre-alpha code I don't want to waste time on it right now)
  • Infra PR for new topics
  • Removes sub-templates from the config area - this is now only doable via a direct load to the template
  • Internal destinations should have different filter validation

Follow up

  • Expose activity log notifications in the UI with inline editor
  • Allow properties to be marked as "advanced" so they don't show up by default
  • Add url and "human description" to server side activity log to make message delivery easier
  • Some sort of UI for seeing all configured internal notifcations
  • Make it so we can keep functions closed-source (only allow modifying inputs + filters) with the execution code pulled from the template

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Does this work well for both Cloud and self-hosted?

How did you test this code?

@benjackwhite benjackwhite changed the title feat: Started adding internal events topic feat: CDP Internal events processor Dec 18, 2024
@benjackwhite benjackwhite marked this pull request as ready for review December 18, 2024 15:19
export type HogFunctionTypeType =
| 'destination'
| 'transformation'
| 'internal-destination'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we say that the type would be more specific (e.g. error_tracking_alert) and just make the Kafka topic generic. I'm easy either way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I changed my mind :D There isn't any plus side that I could find tbh. But also I'm still playing with it to find the right mix of namings

plugin-server/src/cdp/schema.ts Outdated Show resolved Hide resolved
@benjackwhite benjackwhite requested a review from a team December 19, 2024 14:07
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

  • chromium: 0 added, 3 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

# Conflicts:
#	frontend/src/lib/constants.tsx
#	posthog/migrations/max_migration.txt
# Conflicts:
#	posthog/migrations/max_migration.txt
# Conflicts:
#	posthog/migrations/0535_alter_hogfunction_type.py
@benjackwhite benjackwhite changed the base branch from master to feat/drop-hog-function-type December 20, 2024 12:31
Base automatically changed from feat/drop-hog-function-type to master December 20, 2024 13:15
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@benjackwhite benjackwhite merged commit dd74b2c into master Dec 27, 2024
99 checks passed
@benjackwhite benjackwhite deleted the feat/cdp-internal-events branch December 27, 2024 09:03
Copy link

sentry-io bot commented Dec 27, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ TypeError: Object of type datetime is not JSON serializable /api/projects/{parent_lookup_project_id}/survey... View Issue
  • ‼️ NoBrokersAvailable: NoBrokersAvailable /api/projects/{parent_lookup_team_id}/insights/ View Issue

Did you find this useful? React with a 👍 or 👎

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.

4 participants