feat(plugin-server): handle process_person=false

[bretthoerner]

Problem

We need to support $process_person=false by not handling person properties, groups, and by dropping some fields from the event.

Note that we also need to skip some plugins, but that doesn't depend on this code and will be coming separately.

Changes

First, I'm not in love with where I had to squeeze things in. We want to get this out soon, so I can't afford to do the refactoring that I'd like to do. I do think a lot of this will become cleaner in the next phase, when (I think) the person object becomes optional and so we aren't passing around the extra bool to most steps, we just do work based on whether or not a real person object exists or not.

Anyway, changes:

• Early on in event processing (runner.ts) extract the proper processPerson boolean that is used for the rest of event processing
• Pass that processPerson down so that we:
• • Don't lookup or update person properties in PG
• • Don't add person properties to the event
• • Drop $set and related properties
• • Drop certain events like $identify with an ingestion warning
• • Don't update groups or add group related columns

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

Yes.

How did you test this code?

Updated existing and added more.

[bretthoerner]

Oh, this one was unrelated but seems costly. I think by my count we were proactively serializing the whole event 6 times per full processing loop? I'm hoping I'm JSing wrong again. :)
e15a798

[tiina303]

Just to confirm my understanding: we're removing group properties from the event here to match what we do for $set and friends, correct?
But we keep group info, then we'll want to have these here.
I'd propose that we move everything groups into a separate step and handle it all there and not do anything within normalizeEventStep - because later we might want to make that also look at their add-on selection - i.e. skip group properties if they have disabled that add-on

[bretthoerner]

I agree we should refactor to have a group processing step.

But I'm not sure I follow these parts:

But we keep group info, then we'll want to have these here.
i.e. skip group properties if they have disabled that add-on

I thought for $process_person=false we want don't want to process Group stuff at all? So I'm avoiding doing any Group lookups or adding Group fields. I can't tell if you're saying we want to do (some?) Group stuff for $process_person=false events?

[tiina303]

This is a separate follow-up I think we should do, sorry for the confusion here.
What I'm thinking is

1. we move all group stuff into a separate step
2. we make that step be ignored if $process_person=false or group analytics addon is disabled

[bretthoerner]

Ah OK, agreed!

[tiina303]

is event.$unset a thing also?

[bretthoerner]

I was going by what's on the type, but it appears we don't expect one. And we only read it from within properties.

I can delete event['$unset'] to be safe if we want.

[tiina303]

oh I see, might be good to do that for consistency indeed (in case someone changes something later somewhere else)

[tiina303]

Maybe we either always keep it or always remove it, atm we keep if false and remove otherwise

[bretthoerner]

That's what I chose to do yeah, I'm up for either. My thinking was it feels weird to add properties to every event when they aren't sending it, and true is the default. (We also have the person_mode column.)

If we'd rather always add it to every event, I can definitely do that. Will it not throw current users off to see a new $process_person=true property on every event?

[tiina303]

how about we just always remove it?