From ee5033ad32735c3f53fdbe26c31250e7d53fa052 Mon Sep 17 00:00:00 2001
From: Tiina Turban <tiina303@gmail.com>
Date: Wed, 12 Jun 2024 14:13:19 +0200
Subject: [PATCH] feat: Track which person property value changes cause updates
 (#22900)

---
 plugin-server/src/utils/db/utils.ts           |  5 +++-
 .../src/worker/ingestion/person-state.ts      | 30 +++++++++++++++++--
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/plugin-server/src/utils/db/utils.ts b/plugin-server/src/utils/db/utils.ts
index 4825e4272b8aa..3286fea388342 100644
--- a/plugin-server/src/utils/db/utils.ts
+++ b/plugin-server/src/utils/db/utils.ts
@@ -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',
@@ -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 {
diff --git a/plugin-server/src/worker/ingestion/person-state.ts b/plugin-server/src/worker/ingestion/person-state.ts
index 24e279fd981da..955969b1b6804 100644
--- a/plugin-server/src/worker/ingestion/person-state.ts
+++ b/plugin-server/src/worker/ingestion/person-state.ts
@@ -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'
@@ -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
@@ -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
@@ -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
     }