Skip to content

Commit

Permalink
feat: Track which person property value changes cause updates (#22900)
Browse files Browse the repository at this point in the history
  • Loading branch information
tiina303 authored Jun 12, 2024
1 parent 1a2bb8d commit ee5033a
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
5 changes: 4 additions & 1 deletion plugin-server/src/utils/db/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export function timeoutGuard(
}

// when changing this set, be sure to update the frontend as well (taxonomy.tsx (eventToPersonProperties))
const eventToPersonProperties = new Set([
export const eventToPersonProperties = new Set([
// mobile params
'$app_build',
'$app_name',
Expand Down Expand Up @@ -89,6 +89,9 @@ const eventToPersonProperties = new Set([
'igshid', // instagram
'ttclid', // tiktok
])
export const initialEventToPersonProperties = new Set(
Array.from(eventToPersonProperties, (key) => `$initial_${key.replace('$', '')}`)
)

/** If we get new UTM params, make sure we set those **/
export function personInitialAndUTMProperties(properties: Properties): Properties {
Expand Down
30 changes: 28 additions & 2 deletions plugin-server/src/worker/ingestion/person-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { KAFKA_PERSON_OVERRIDE } from '../../config/kafka-topics'
import { InternalPerson, Person, PropertyUpdateOperation, TimestampFormat } from '../../types'
import { DB } from '../../utils/db/db'
import { PostgresRouter, PostgresUse, TransactionClient } from '../../utils/db/postgres'
import { timeoutGuard } from '../../utils/db/utils'
import { eventToPersonProperties, initialEventToPersonProperties, timeoutGuard } from '../../utils/db/utils'
import { PeriodicTask } from '../../utils/periodic-task'
import { promiseRetry } from '../../utils/retries'
import { status } from '../../utils/status'
Expand All @@ -34,6 +34,12 @@ export const mergeTxnSuccessCounter = new Counter({
labelNames: ['call', 'oldPersonIdentified', 'newPersonIdentified', 'poEEmbraceJoin'],
})

export const personPropertyKeyUpdateCounter = new Counter({
name: 'person_property_key_update_total',
help: 'Number of person updates triggered by this property value changing.',
labelNames: ['key'],
})

// used to prevent identify from being used with generic IDs
// that we can safely assume stem from a bug or mistake
// used to prevent identify from being used with generic IDs
Expand Down Expand Up @@ -264,6 +270,21 @@ export class PersonState {
return [person, Promise.resolve()]
}

// For tracking what property keys cause us to update persons
// tracking all properties we add from the event, 'geoip' for '$geoip_*' or '$initial_geoip_*' and 'other' for anything outside of those
private getMetricKey(key: string): string {
if (key.startsWith('$geoip_') || key.startsWith('$initial_geoip_')) {
return 'geoIP'
}
if (eventToPersonProperties.has(key)) {
return key
}
if (initialEventToPersonProperties.has(key)) {
return key
}
return 'other'
}

/**
* @param personProperties Properties of the person to be updated, these are updated in place.
* @returns true if the properties were changed, false if they were not
Expand All @@ -277,25 +298,30 @@ export class PersonState {
: Object.keys(unsetProps || {}) || []

let updated = false
// tracking as set because we only care about if other or geoip was the cause of the update, not how many properties got updated
const metricsKeys = new Set<string>()
Object.entries(propertiesOnce).map(([key, value]) => {
if (typeof personProperties[key] === 'undefined') {
updated = true
metricsKeys.add(this.getMetricKey(key))
personProperties[key] = value
}
})
Object.entries(properties).map(([key, value]) => {
if (personProperties[key] !== value) {
updated = true
metricsKeys.add(this.getMetricKey(key))
personProperties[key] = value
}
})
unsetProperties.forEach((propertyKey) => {
if (propertyKey in personProperties) {
updated = true
metricsKeys.add(this.getMetricKey(propertyKey))
delete personProperties[propertyKey]
}
})

metricsKeys.forEach((key) => personPropertyKeyUpdateCounter.labels({ key: key }).inc())
return updated
}

Expand Down

0 comments on commit ee5033a

Please sign in to comment.