-
Notifications
You must be signed in to change notification settings - Fork 56
feat: add beforeSend hook #255
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
base: main
Are you sure you want to change the base?
Conversation
| - Application lifecycle events (`Application Opened`, etc.) | ||
| - etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
events captured by the captureApplicationLifecycleEvents config
| - feat: add `beforeSend` callback to `PostHogConfig` for dropping or modifying events before they are sent to PostHog ([#255](https://github.com/PostHog/posthog-flutter/pull/255)) | ||
| - **Limitation**: | ||
| - Does NOT intercept native-initiated events such as: | ||
| - Session replay events (`$snapshot`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, mention the sessionReplay flag
|
|
||
| - feat: add `beforeSend` callback to `PostHogConfig` for dropping or modifying events before they are sent to PostHog ([#255](https://github.com/PostHog/posthog-flutter/pull/255)) | ||
| - **Limitation**: | ||
| - Does NOT intercept native-initiated events such as: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should mention events captured by sendFeatureFlagEvents
| /// }; | ||
| /// ``` | ||
| /// | ||
| /// **Limitations:** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments in the changelog
| /// Callback to intercept and modify events before they are sent to PostHog. | ||
| /// | ||
| /// Return a possibly modified event to send it, or return `null` to drop it. | ||
| typedef BeforeSendCallback = PostHogEvent? Function(PostHogEvent event); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this type should be `FutureOr<PostHogEvent?> since people might need to use async methods to filter stuff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this one since we've talked about eventually moving away from async method channels. This would complicate the capture pipeline potentially. Plus all the other sdks are sync hooks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, but right now even our sdk is async, so if you call isFeatureFlag something is async, which maybe its an use case to be used for filtering things, so we need to make it FutureOr which allows to make it async or not.
Ps I used many async methods in the past to filter out events.
I think when we redesign to be sync first, we'd figure this out differently.
| /// Represents an event that can be modified or dropped by the [BeforeSendCallback]. | ||
| /// | ||
| /// This class is used in the beforeSend callback to allow modification of events before they are sent to PostHog. | ||
| class PostHogEvent { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at least on the other SDKs, this event has a specific map for $set and $set_once
i think we should do the same
see https://github.com/PostHog/posthog-js/pull/2931/files#diff-5415311d4de5614e8deee6eddaa62716f3528992c20787a89433c2b99d2cdc9a capture event
not a must, just wanted to share this
| /// **Note:** | ||
| /// - This callback runs synchronously on the Dart side | ||
| /// - Exceptions in the callback will cause the event to be sent unchanged | ||
| BeforeSendCallback? beforeSend; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the other SDKs we allow one function or a list of tunction, we dont support the list of functions here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good point
| properties != null ? PropertyNormalizer.normalize(properties) : null; | ||
| final normalizedProperties = processedEvent.properties != null | ||
| ? PropertyNormalizer.normalize( | ||
| processedEvent.properties!.cast<String, Object>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
never use !., fallback to empty array or check nullability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
create a CLAUDE.md if needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a null check right above but yes better avoid this
💡 Motivation and Context
Closes #155
Docs: PostHog/posthog.com#14482
Adds a
beforeSendcallback toPostHogConfigthat lets you drop or modify events before they're sent to PostHog.Initially tried implementing this on the native side with method channels calling back to Dart in an attempt to also capture native initiated events, but hit timing issues since method channels are async but the beforeSend hook is called synchronously from native side. Also explored FFI but it added way too much complexity for the benefit.
For now, decided to keep it dart only so that it stays fast, synchronous and reliable. Should be good for the majority of the use cases.
Limitations in changelog
💚 How did you test it?
📝 Checklist