feat(ingestion): conversion buffer producer#9427
Conversation
yakkomajuri
changed the title
| distinctId: string | ||
| properties: Properties | ||
| timestamp: DateTime | string | ||
| elementsList: Element[] |
There was a problem hiding this comment.
what is this and why do we need it?
There was a problem hiding this comment.
this is used in a lot of places.
before we used to pass all these as args to createEvent. Now however we need to return these, send these to the buffer, etc. I could check other types though and see if there's significant overlap to reuse
| await hub.hookCannon.findAndFireHooks(event, person, siteUrl, actionMatches) | ||
|
|
||
| // eventId is undefined for CH deployments | ||
| // CH deployments calculate actions on the fly |
There was a problem hiding this comment.
we don't have non-CH deployments anymore.
There was a problem hiding this comment.
this comment already existed but sure can remove
| return actionMatches | ||
| } | ||
|
|
||
| // TODO: Handle new persons? |
There was a problem hiding this comment.
So the rules for the buffer are specified here:
One of the rules is that events for persons that didn't exist before should go to the buffer. However, while building this, I realized:
- It'd be a major refactor to cover that
- The conversion seconds anyway handles it (person created within 60sec)
The one edge case is if we lower that buffer value a lot (say to 1sec), but actually I've decided to punt on this problem
There was a problem hiding this comment.
commented on that issue, sorry I missed it somehow before
| const isAnonymousEvent = | ||
| event.properties && event.properties['$device_id'] && event.distinctId === event.properties['$device_id'] | ||
| const isRecentPerson = !person || DateTime.now().diff(person.created_at).seconds < hub.BUFFER_CONVERSION_SECONDS | ||
| const ingestEventDirectly = isAnonymousEvent || event.event === '$identify' || !isRecentPerson |
There was a problem hiding this comment.
Wouldn't it be better to send anonymous events to the buffer & ingest non-anonymouse events directly?
Works for first sign-up and good for later pre-login events too?
We probably want to treat alias the same way as identify for when it's ingested.
There was a problem hiding this comment.
We thought about the buffer a lot before making decisions on the rules.
But anyway, no, that doesn't make sense. We will merge on the anonymous person ID, so the ID for those events will never change. The buffer is used for events where the person ID might change.
The reason for sending anonymous events there would be if we merged into the identified person ID, but under that the whole system crumbles. How long does it take for an anonymous user to become identified? Is there a guarantee that they will?
2 participants
Problem
#9182
Changes
This should be a no-op change for now given the buffer is disabled.
It splits out the event creation from the
processEventpath in the worker so that we can selectively create an event or not. If the event shouldn't be immediately created, it will be produced to the buffer, which will in turn trigger ingestion at a later point.How did you test this code?
Added tests