Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(flags): Add local props and flags to all calls #149

Merged
merged 22 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions posthog-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ export abstract class PostHogCoreStateless {
private captureMode: 'form' | 'json'
private removeDebugCallback?: () => void
private debugMode: boolean = false
private pendingPromises: Record<string, Promise<any>> = {}
private disableGeoip: boolean = true

private _optoutOverride: boolean | undefined
Expand All @@ -65,6 +64,7 @@ export abstract class PostHogCoreStateless {
protected _events = new SimpleEventEmitter()
protected _flushTimer?: any
protected _retryOptions: RetriableOptions
protected pendingPromises: Record<string, Promise<any>> = {}

// Abstract methods to be overridden by implementations
abstract fetch(url: string, options: PostHogFetchOptions): Promise<PostHogFetchResponse>
Expand Down Expand Up @@ -249,7 +249,7 @@ export abstract class PostHogCoreStateless {
return this.fetchWithRetry(url, fetchOptions)
.then((response) => response.json() as Promise<PostHogDecideResponse>)
.catch((error) => {
console.error('Error fetching feature flags', error)
this._events.emit('error', error)
return undefined
})
}
Expand Down Expand Up @@ -565,6 +565,10 @@ export abstract class PostHogCoreStateless {
})
)
)
// flush again to make sure we send all events, some of which might've been added
// while we were waiting for the pending promises to resolve
// For example, see sendFeatureFlags in posthog-node/src/posthog-node.ts::capture
await this.flushAsync()
} catch (e) {
if (!isPostHogFetchError(e)) {
throw e
Expand Down
4 changes: 2 additions & 2 deletions posthog-node/src/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class FeatureFlagsPoller {
console.debug(`InconclusiveMatchError when computing flag locally: ${key}: ${e}`)
}
} else if (e instanceof Error) {
console.error(`Error computing flag locally: ${key}: ${e}`)
this.onError?.(new Error(`Error computing flag locally: ${key}: ${e}`))
}
}
}
Expand Down Expand Up @@ -383,7 +383,7 @@ class FeatureFlagsPoller {

const responseJson = await res.json()
if (!('flags' in responseJson)) {
console.error(`Invalid response when getting feature flags: ${JSON.stringify(responseJson)}`)
this.onError?.(new Error(`Invalid response when getting feature flags: ${JSON.stringify(responseJson)}`))
}

this.featureFlags = responseJson.flags || []
Expand Down
137 changes: 118 additions & 19 deletions posthog-node/src/posthog-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { PostHogMemoryStorage } from '../../posthog-core/src/storage-memory'
import { EventMessageV1, GroupIdentifyMessage, IdentifyMessageV1, PostHogNodeV1 } from './types'
import { FeatureFlagsPoller } from './feature-flags'
import fetch from './fetch'
import { generateUUID } from 'posthog-core/src/utils'

export type PostHogOptions = PosthogCoreOptions & {
persistence?: 'memory'
Expand Down Expand Up @@ -106,22 +107,68 @@ export class PostHog extends PostHogCoreStateless implements PostHogNodeV1 {
}

if (sendFeatureFlags) {
super.getFeatureFlagsStateless(distinctId, groups, undefined, undefined, disableGeoip).then((flags) => {
const featureVariantProperties: Record<string, string | boolean> = {}
if (flags) {
for (const [feature, variant] of Object.entries(flags)) {
if (variant !== false) {
featureVariantProperties[`$feature/${feature}`] = variant
const promiseUUID = generateUUID()
// :TRICKY: If we flush, or need to shut down, to not lose events we want this promise to resolve before we flush
const promise = super.getFeatureFlagsStateless(distinctId, groups, undefined, undefined, disableGeoip)
this.pendingPromises[promiseUUID] = promise

promise
.then((flags) => {
neilkakkar marked this conversation as resolved.
Show resolved Hide resolved
const featureVariantProperties: Record<string, string | boolean> = {}
if (flags) {
for (const [feature, variant] of Object.entries(flags)) {
if (variant !== false) {
featureVariantProperties[`$feature/${feature}`] = variant
}
}
}
}
const activeFlags = Object.keys(flags || {}).filter((flag) => flags?.[flag] !== false)
const flagProperties = {
$active_feature_flags: activeFlags || undefined,
...featureVariantProperties,
}
_capture({ ...properties, $groups: groups, ...flagProperties })
const activeFlags = Object.keys(flags || {}).filter((flag) => flags?.[flag] !== false)
const flagProperties = {
$active_feature_flags: activeFlags || undefined,
...featureVariantProperties,
}
_capture({ ...properties, $groups: groups, ...flagProperties })
})
.catch(() => {
_capture({ ...properties, $groups: groups })
})
} else if ((this.featureFlagsPoller?.featureFlags?.length || 0) > 0) {
const groupsWithStringValues: Record<string, string> = {}
for (const [key, value] of Object.entries(groups || {})) {
groupsWithStringValues[key] = String(value)
}

const promiseUUID = generateUUID()
// :TRICKY: If we flush, or need to shut down, to not lose events we want this promise to resolve before we flush
const promise = this.getAllFlags(distinctId, {
groups: groupsWithStringValues,
disableGeoip,
onlyEvaluateLocally: true,
})
this.pendingPromises[promiseUUID] = promise
promise
.then((flags) => {
const featureVariantProperties: Record<string, string | boolean> = {}
if (flags) {
for (const [feature, variant] of Object.entries(flags)) {
featureVariantProperties[`$feature/${feature}`] = variant
}
}
const activeFlags = Object.keys(flags || {}).filter((flag) => flags?.[flag] !== false)
let flagProperties: Record<string, any> = {
...featureVariantProperties,
}
if (activeFlags.length > 0) {
flagProperties = {
...flagProperties,
$active_feature_flags: activeFlags,
}
}
_capture({ ...flagProperties, ...properties, $groups: groups })
})
.catch(() => {
_capture({ ...properties, $groups: groups })
})
} else {
_capture({ ...properties, $groups: groups })
}
Expand Down Expand Up @@ -156,8 +203,18 @@ export class PostHog extends PostHogCoreStateless implements PostHogNodeV1 {
disableGeoip?: boolean
}
): Promise<string | boolean | undefined> {
const { groups, personProperties, groupProperties, disableGeoip } = options || {}
let { onlyEvaluateLocally, sendFeatureFlagEvents } = options || {}
const { groups, disableGeoip } = options || {}
let { onlyEvaluateLocally, sendFeatureFlagEvents, personProperties, groupProperties } = options || {}

const adjustedProperties = this.addLocalPersonAndGroupProperties(
distinctId,
groups,
personProperties,
groupProperties
)

personProperties = adjustedProperties.allPersonProperties
groupProperties = adjustedProperties.allGroupProperties

// set defaults
if (onlyEvaluateLocally == undefined) {
Expand Down Expand Up @@ -232,8 +289,19 @@ export class PostHog extends PostHogCoreStateless implements PostHogNodeV1 {
disableGeoip?: boolean
}
): Promise<JsonType | undefined> {
const { groups, personProperties, groupProperties, disableGeoip } = options || {}
let { onlyEvaluateLocally, sendFeatureFlagEvents } = options || {}
const { groups, disableGeoip } = options || {}
let { onlyEvaluateLocally, sendFeatureFlagEvents, personProperties, groupProperties } = options || {}

const adjustedProperties = this.addLocalPersonAndGroupProperties(
distinctId,
groups,
personProperties,
groupProperties
)

personProperties = adjustedProperties.allPersonProperties
groupProperties = adjustedProperties.allGroupProperties

let response = undefined

// Try to get match value locally if not provided
Expand Down Expand Up @@ -324,8 +392,18 @@ export class PostHog extends PostHogCoreStateless implements PostHogNodeV1 {
disableGeoip?: boolean
}
): Promise<PosthogFlagsAndPayloadsResponse> {
const { groups, personProperties, groupProperties, disableGeoip } = options || {}
let { onlyEvaluateLocally } = options || {}
const { groups, disableGeoip } = options || {}
let { onlyEvaluateLocally, personProperties, groupProperties } = options || {}

const adjustedProperties = this.addLocalPersonAndGroupProperties(
distinctId,
groups,
personProperties,
groupProperties
)

personProperties = adjustedProperties.allPersonProperties
groupProperties = adjustedProperties.allGroupProperties

// set defaults
if (onlyEvaluateLocally == undefined) {
Expand Down Expand Up @@ -385,4 +463,25 @@ export class PostHog extends PostHogCoreStateless implements PostHogNodeV1 {
this.featureFlagsPoller?.stopPoller()
return super.shutdownAsync()
}

private addLocalPersonAndGroupProperties(
distinctId: string,
groups?: Record<string, string>,
personProperties?: Record<string, string>,
groupProperties?: Record<string, Record<string, string>>
): { allPersonProperties: Record<string, string>; allGroupProperties: Record<string, Record<string, string>> } {
const allPersonProperties = { $current_distinct_id: distinctId, ...(personProperties || {}) }

const allGroupProperties: Record<string, Record<string, string>> = {}
if (groups) {
for (const groupName of Object.keys(groups)) {
allGroupProperties[groupName] = {
$group_key: groups[groupName],
...(groupProperties?.[groupName] || {}),
}
}
}

return { allPersonProperties, allGroupProperties }
}
}
19 changes: 17 additions & 2 deletions posthog-node/test/feature-flags.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,17 @@ export const apiImplementation = ({
}) as any
}

if ((url as any).includes('batch/')) {
return Promise.resolve({
status: 200,
text: () => Promise.resolve('ok'),
json: () =>
Promise.resolve({
status: 'ok',
}),
}) as any
}

return Promise.resolve({
status: 400,
text: () => Promise.resolve('ok'),
Expand Down Expand Up @@ -342,7 +353,11 @@ describe('local evaluation', () => {
token: 'TEST_API_KEY',
distinct_id: 'some-distinct-id_outside_rollout?',
groups: {},
person_properties: { region: 'USA', email: '[email protected]' },
person_properties: {
$current_distinct_id: 'some-distinct-id_outside_rollout?',
region: 'USA',
email: '[email protected]',
},
group_properties: {},
geoip_disable: true,
}),
Expand All @@ -361,7 +376,7 @@ describe('local evaluation', () => {
token: 'TEST_API_KEY',
distinct_id: 'some-distinct-id',
groups: {},
person_properties: { doesnt_matter: '1' },
person_properties: { $current_distinct_id: 'some-distinct-id', doesnt_matter: '1' },
group_properties: {},
geoip_disable: true,
}),
Expand Down
Loading