Skip to content

Commit

Permalink
chore(plugin-server): $process_person -> $process_person_profile
Browse files Browse the repository at this point in the history
  • Loading branch information
bretthoerner committed Apr 16, 2024
1 parent 1fb3644 commit 344e4ee
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ test.concurrent(`event ingestion: can $set and update person properties`, async
})

test.concurrent(
`event ingestion: $process_person=false drops expected fields, doesn't include person properties`,
`event ingestion: $process_person_profile=false drops expected fields, doesn't include person properties`,
async () => {
const teamId = await createTeam(organizationId)
const distinctId = new UUIDT().toString()
Expand All @@ -229,7 +229,7 @@ test.concurrent(
uuid: properylessUuid,
event: 'custom event',
properties: {
$process_person: false,
$process_person_profile: false,
$group_0: 'group_key',
$set: {
c: 3,
Expand All @@ -251,7 +251,7 @@ test.concurrent(
expect(event).toEqual(
expect.objectContaining({
person_properties: {},
properties: { uuid: properylessUuid, $sent_at: expect.any(String), $process_person: false },
properties: { uuid: properylessUuid, $sent_at: expect.any(String), $process_person_profile: false },
person_mode: 'propertyless',
})
)
Expand Down
8 changes: 4 additions & 4 deletions plugin-server/src/utils/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ export function convertToIngestionEvent(event: RawClickHouseEvent, skipElementsC
}
}

/// Does normalization steps involving the $process_person property. This is currently a separate
/// Does normalization steps involving the $process_person_profile property. This is currently a separate
/// function because `normalizeEvent` is called from multiple places, some early in the pipeline,
/// and we want to have one trusted place where `$process_person` is handled and passed through
/// and we want to have one trusted place where `$process_person_profile` is handled and passed through
/// all of the processing steps.
///
/// If `formPipelineEvent` is removed this can easily be combined with `normalizeEvent`.
Expand All @@ -132,11 +132,11 @@ export function normalizeProcessPerson(event: PluginEvent, processPerson: boolea
delete properties.$unset
// Recorded for clarity and so that the property exists if it is ever sent elsewhere,
// e.g. for migrations.
properties.$process_person = false
properties.$process_person_profile = false
} else {
// Removed as it is the default, note that we have record the `person_mode` column
// in ClickHouse for all events.
delete properties.$process_person
delete properties.$process_person_profile
}

event.properties = properties
Expand Down
12 changes: 6 additions & 6 deletions plugin-server/src/worker/ingestion/event-pipeline/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ export class EventPipelineRunner {
const kafkaAcks: Promise<void>[] = []

let processPerson = true // The default.
if (event.properties && '$process_person' in event.properties) {
const propValue = event.properties.$process_person
if (event.properties && '$process_person_profile' in event.properties) {
const propValue = event.properties.$process_person_profile
if (propValue === true) {
// This is the default, and `true` is one of the two valid values.
} else if (propValue === false) {
Expand All @@ -136,7 +136,7 @@ export class EventPipelineRunner {
captureIngestionWarning(
this.hub.db.kafkaProducer,
event.team_id,
'invalid_event_when_process_person_is_false',
'invalid_event_when_process_person_profile_is_false',
{
eventUuid: event.uuid,
event: event.event,
Expand All @@ -159,13 +159,13 @@ export class EventPipelineRunner {
captureIngestionWarning(
this.hub.db.kafkaProducer,
event.team_id,
'invalid_process_person',
'invalid_process_person_profile',
{
eventUuid: event.uuid,
event: event.event,
distinctId: event.distinct_id,
$process_person: propValue,
message: 'Only a boolean value is valid for the $process_person property',
$process_person_profile: propValue,
message: 'Only a boolean value is valid for the $process_person_profile property',
},
{ alwaysSend: false }
)
Expand Down
2 changes: 1 addition & 1 deletion plugin-server/src/worker/ingestion/person-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export class PersonState {
private teamId: number,
private distinctId: string,
private timestamp: DateTime,
private processPerson: boolean, // $process_person flag from the event
private processPerson: boolean, // $process_person_profile flag from the event
private db: DB,
private personOverrideWriter?: DeferredPersonOverrideWriter,
uuid: UUIDT | undefined = undefined
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('normalizeEventStep()', () => {
a: 5,
},
$browser: 'Chrome',
$process_person: true, // This is dropped, as it is implied
$process_person_profile: true, // This is dropped, as it is implied
},
$set: {
someProp: 'value',
Expand Down Expand Up @@ -54,7 +54,7 @@ describe('normalizeEventStep()', () => {
expect(timestamp).toEqual(DateTime.fromISO(event.timestamp!, { zone: 'utc' }))
})

it('normalizes $process_person=false events by dropping $set and related', async () => {
it('normalizes $process_person_profile=false events by dropping $set and related', async () => {
await resetTestDatabase()
const [hub, _] = await createHub()
const organizationId = await createOrganization(hub.db.postgres)
Expand Down Expand Up @@ -94,7 +94,7 @@ describe('normalizeEventStep()', () => {
...event,
properties: {
$browser: 'Chrome',
$process_person: false,
$process_person_profile: false,
},
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,12 +286,12 @@ describe('EventPipelineRunner', () => {
})
})

describe('EventPipelineRunner $process_person=false', () => {
it('drops events that are not allowed when $process_person=false', async () => {
describe('EventPipelineRunner $process_person_profile=false', () => {
it('drops events that are not allowed when $process_person_profile=false', async () => {
for (const eventName of ['$identify', '$create_alias', '$merge_dangerously', '$groupidentify']) {
const event = {
...pipelineEvent,
properties: { $process_person: false },
properties: { $process_person_profile: false },
event: eventName,
team_id: 9,
}
Expand All @@ -311,7 +311,7 @@ describe('EventPipelineRunner $process_person=false', () => {
JSON.parse(hub.db.kafkaProducer.queueMessage.mock.calls[0][0].kafkaMessage.messages[0].value)
).toMatchObject({
team_id: 9,
type: 'invalid_event_when_process_person_is_false',
type: 'invalid_event_when_process_person_profile_is_false',
details: JSON.stringify({ eventUuid: 'uuid1', event: eventName, distinctId: 'my_id' }),
})
}
Expand Down
10 changes: 5 additions & 5 deletions plugin-server/tests/worker/ingestion/person-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ describe('PersonState.update()', () => {
expect(distinctIds).toEqual(expect.arrayContaining(['new-user']))
})

it('creates person if they are new and $process_person=false', async () => {
// Note that eventually $process_person=false will be optimized so that the person is
it('creates person if they are new and $process_person_profile=false', async () => {
// Note that eventually $process_person_profile=false will be optimized so that the person is
// *not* created here.
const event_uuid = new UUIDT().toString()
const processPerson = false
Expand All @@ -183,7 +183,7 @@ describe('PersonState.update()', () => {
event: '$pageview',
distinct_id: 'new-user',
uuid: event_uuid,
properties: { $process_person: false, $set: { a: 1 }, $set_once: { b: 2 } },
properties: { $process_person_profile: false, $set: { a: 1 }, $set_once: { b: 2 } },
},
hub,
processPerson
Expand Down Expand Up @@ -216,7 +216,7 @@ describe('PersonState.update()', () => {
expect(distinctIds).toEqual(expect.arrayContaining(['new-user']))
})

it('does not attach existing person properties to $process_person=false events', async () => {
it('does not attach existing person properties to $process_person_profile=false events', async () => {
const originalEventUuid = new UUIDT().toString()
const person = await personState({
event: '$pageview',
Expand Down Expand Up @@ -256,7 +256,7 @@ describe('PersonState.update()', () => {
}).update()
expect(personVerifyProps.properties).toEqual({ $creator_event_uuid: originalEventUuid, c: 420 })

// But they don't when $process_person=false
// But they don't when $process_person_profile=false
const processPersonFalseResult = await personState(
{
event: '$pageview',
Expand Down
2 changes: 1 addition & 1 deletion plugin-server/tests/worker/ingestion/process-event.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ describe('EventsProcessor#createEvent()', () => {
)
})

it('when $process_person=false, emits event with without person properties or groups', async () => {
it('when $process_person_profile=false, emits event with without person properties or groups', async () => {
const processPerson = false
await eventsProcessor.createEvent(
{ ...preIngestionEvent, properties: { $group_0: 'group_key' } },
Expand Down

0 comments on commit 344e4ee

Please sign in to comment.