diff --git a/nodejs/src/common/utils/db/utils.ts b/nodejs/src/common/utils/db/utils.ts index 119b1f0f41dd..b1f0be269196 100644 --- a/nodejs/src/common/utils/db/utils.ts +++ b/nodejs/src/common/utils/db/utils.ts @@ -111,15 +111,11 @@ export function personInitialAndUTMProperties(properties: Properties): Propertie // For the purposes of $initial properties, $os_name is treated as a fallback alias of $os, starting August 2024 // It's a special case due to _some_ SDKs using $os_name: https://github.com/PostHog/posthog-js-lite/issues/244 + // normalizeOsAlias fills the event's own $os upstream, so the loop above already lifted $os into $set. const osName = properties.$os_name if (osName !== undefined && !skipServerHostOs) { - if (!('$os' in properties)) { - properties.$os = osName - } - if (!('$os' in $set)) { - $set.$os = osName - } - // Same null rule as above: a null $os_name must not claim $initial_os for good. + // Only reachable when $os is null, since the loop skips $initial_os for a null value; the + // non-null check on $os_name is the same first-touch rule as above. if (osName !== null && !('$initial_os' in $set_once!)) { $set_once!.$initial_os = osName } diff --git a/nodejs/src/common/utils/event.test.ts b/nodejs/src/common/utils/event.test.ts index b3519ad66abe..0c7983fc085a 100644 --- a/nodejs/src/common/utils/event.test.ts +++ b/nodejs/src/common/utils/event.test.ts @@ -62,6 +62,69 @@ describe('normalizeEvent()', () => { }) }) + describe('$os_name alias', () => { + test.each([ + { + name: 'fills a missing $os', + event: 'some_event', + properties: { $os_name: 'Windows' }, + expected: 'Windows', + }, + { + name: 'leaves an existing $os alone', + event: 'some_event', + properties: { $os: 'Mac OS X', $os_name: 'Windows' }, + expected: 'Mac OS X', + }, + { + name: 'carries a null $os_name through to $os', + event: 'some_event', + properties: { $os_name: null }, + expected: null, + }, + { + name: 'skips a $snapshot event, which skips person property lifting too', + event: '$snapshot', + properties: { $os_name: 'Windows' }, + expected: undefined, + }, + { + name: 'skips a $performance_event, which skips person property lifting too', + event: '$performance_event', + properties: { $os_name: 'Windows' }, + expected: undefined, + }, + ])('$name', ({ event, properties, expected }) => { + const normalized = normalizeEvent({ event, distinct_id: 'user1', properties } as any) + expect(normalized.properties!['$os']).toBe(expected) + }) + + it('gives the person $os and drops the $os_name alias', () => { + const normalized = normalizeEvent({ + event: 'some_event', + distinct_id: 'user1', + properties: { $os_name: 'iOS' }, + } as any) + + expect(normalized.properties!['$set']).toEqual({ $os: 'iOS' }) + expect(normalized.properties!['$set_once']).toEqual({ $initial_os: 'iOS' }) + }) + + it('keeps a server host OS out of $os and $initial_os', () => { + const normalized = normalizeEvent({ + event: 'some_event', + distinct_id: 'user1', + properties: { $is_server: true, $os_name: 'Linux' }, + } as any) + + expect(normalized.properties!['$os']).toBeUndefined() + // The host OS still reaches the person under the $os_name keys: personInitialAndUTMProperties + // skips only $os and $os_version for a server event, and $set_once makes $initial_os_name sticky. + expect(normalized.properties!['$set']).toEqual({ $os_name: 'Linux' }) + expect(normalized.properties!['$set_once']).toEqual({ $initial_os_name: 'Linux' }) + }) + }) + it('combines .properties $set and $set_once with top-level $set and $set_once', () => { const event = { event: 'some_event', diff --git a/nodejs/src/common/utils/event.ts b/nodejs/src/common/utils/event.ts index 21123314f47c..51efc20588ad 100644 --- a/nodejs/src/common/utils/event.ts +++ b/nodejs/src/common/utils/event.ts @@ -1,4 +1,4 @@ -import { PluginEvent } from '~/plugin-scaffold' +import { PluginEvent, Properties } from '~/plugin-scaffold' import { ClickHouseEvent, PipelineEvent, PostIngestionEvent, RawClickHouseEvent } from '~/types' import { personInitialAndUTMProperties, sanitizeString } from './db/utils' @@ -136,6 +136,30 @@ export function sanitizeEvent(event: T): return event } +/** + * Fills the event's own $os from the $os_name some SDKs report instead: + * https://github.com/PostHog/posthog-js-lite/issues/244. + * + * This describes the event, not the person. Call it after transformations: the user agent populator + * plugin skips an event that already carries $os, so filling $os earlier would stop it deriving + * $browser, $device, and the rest. + * + * Server events are skipped so a server's host OS does not land in the event's own $os. The person + * side is handled separately: personInitialAndUTMProperties skips lifting $os and $os_version and + * skips folding $os_name into the sticky $initial_os for $is_server events. The host OS still reaches + * the person under the raw $os_name and $initial_os_name keys. + */ +export function normalizeOsAlias(properties: Properties): void { + if (properties['$is_server'] === true) { + return + } + + const osName = properties['$os_name'] + if (osName !== undefined && !('$os' in properties)) { + properties['$os'] = osName + } +} + /** * Full event normalization including person property mapping. * This should only be called ONCE per event, after any transformations. @@ -146,6 +170,7 @@ export function normalizeEvent(event: T): event = sanitizeEvent(event) if (!['$snapshot', '$performance_event'].includes(event.event)) { + normalizeOsAlias(event.properties!) event.properties = personInitialAndUTMProperties(event.properties!) } diff --git a/nodejs/tests/utils/db/utils.test.ts b/nodejs/tests/utils/db/utils.test.ts index cc8630ddd937..41c3cd909a69 100644 --- a/nodejs/tests/utils/db/utils.test.ts +++ b/nodejs/tests/utils/db/utils.test.ts @@ -101,23 +101,28 @@ describe('personInitialAndUTMProperties()', () => { $set: { $current_url: 'https://test.com' }, }) }) - it('treats $os_name as fallback for $os', () => { - const propertiesOsNameOnly = { $os_name: 'Android' } - expect(personInitialAndUTMProperties(propertiesOsNameOnly)).toEqual({ - $os: 'Android', - $os_name: 'Android', - $set_once: { $initial_os: 'Android' }, - $set: { $os: 'Android' }, - }) - // Test that $os takes precedence, with $os_name preserved (although this should not happen in the wild) - const propertiesBothOsKeys = { $os: 'Windows', $os_name: 'Android' } - expect(personInitialAndUTMProperties(propertiesBothOsKeys)).toEqual({ + it('keeps $os over $os_name and drops the alias from person properties', () => { + // normalizeOsAlias fills the event's own $os upstream, so this function always sees $os + // alongside $os_name; the divergent values here never occur in the wild. + const properties = { $os: 'Windows', $os_name: 'Android' } + expect(personInitialAndUTMProperties(properties)).toEqual({ $os: 'Windows', $os_name: 'Android', $set_once: { $initial_os: 'Windows' }, $set: { $os: 'Windows' }, }) }) + it('falls back to $os_name for $initial_os when the event carries a null $os', () => { + // The never-write-null rule skips $initial_os for a null $os, leaving $os_name as the only + // first-touch value available. + const properties = { $os: null, $os_name: 'Android' } + expect(personInitialAndUTMProperties(properties)).toEqual({ + $os: null, + $os_name: 'Android', + $set_once: { $initial_os: 'Android' }, + $set: { $os: null }, + }) + }) it('does not lift device context from an $is_server event, but keeps campaign params', () => { // A server SDK (is_server: true) stamps its host $os; it must not reach $set/$set_once or // it permanently poisons $initial_os. utm is real attribution and still flows through. @@ -156,7 +161,7 @@ describe('personInitialAndUTMProperties()', () => { { gclid: null, msclkid: null }, {}, ], - ['a null $os_name does not claim $initial_os', { $os_name: null }, { $os: null }, {}], + ['a null $os_name does not claim $initial_os', { $os: null, $os_name: null }, { $os: null }, {}], ])('never writes null first-touch values: %s', (_desc, input, expectedSet, expectedSetOnce) => { const result = personInitialAndUTMProperties({ ...input }) expect(result.$set).toEqual(expectedSet) @@ -165,7 +170,6 @@ describe('personInitialAndUTMProperties()', () => { it.each([ ['client event maps $os', { $lib: 'web', $os: 'Linux' }, 'Linux'], - ['client mobile via $os_name', { $lib: 'posthog-ios', $os_name: 'iOS' }, 'iOS'], ['no $is_server is treated as client', { $os: 'Linux' }, 'Linux'], ['$is_server false is treated as client', { $os: 'Linux', $is_server: false }, 'Linux'], ['$is_server true skips $os', { $os: 'Linux', $is_server: true }, undefined], diff --git a/rust/feature-flags/src/flags/flag_matching_utils.rs b/rust/feature-flags/src/flags/flag_matching_utils.rs index 92dbef67e242..acf03d365296 100644 --- a/rust/feature-flags/src/flags/flag_matching_utils.rs +++ b/rust/feature-flags/src/flags/flag_matching_utils.rs @@ -166,8 +166,10 @@ pub fn populate_missing_initial_properties(properties: &mut HashMap `$os` at write time -/// (`personInitialAndUTMProperties` in nodejs/src/utils/db/utils.ts), but that -/// only covers newly-written rows — historical and un-normalized person rows can +/// (`normalizeOsAlias` in nodejs/src/common/utils/event.ts for the event's own +/// `$os`, `personInitialAndUTMProperties` in nodejs/src/common/utils/db/utils.ts +/// for the person row), but that only covers newly-written rows, and it excludes +/// `$is_server` events; historical and un-normalized person rows can /// carry only one of the two keys. Flag matching does exact-key lookups, so /// without this a mobile person whose row has only `$os_name` never matches an /// `$os` filter (and vice versa). The mirror is bidirectional so either filter