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: Support for client-assigned feature flags #1519

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
64 changes: 64 additions & 0 deletions src/__tests__/featureflags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,70 @@ describe('featureflags', () => {
})
})
})
describe('clientAssignedFeatureFlags', () => {
it('should match the correct variant', () => {
const flag = { key: 'test-flag', variants: { 'variant-1': 50, 'variant-2': 30, 'variant-3': 20 } }
// Repeating the hash function should produce the same output
expect(featureFlags._getMatchingVariant(flag)).toBe('variant-1')
expect(featureFlags._getMatchingVariant(flag)).toBe('variant-1')
expect(featureFlags._getMatchingVariant(flag)).toBe('variant-1')
expect(featureFlags._getMatchingVariant(flag)).toBe('variant-1')
})

it('should compute the correct lookup tables', () => {
const variants1 = {
'variant-1': 50,
'variant-2': 30,
'variant-3': 20,
}
expect(featureFlags._variantLookupTable(variants1)).toEqual([
{ value_min: 0, value_max: 50, key: 'variant-1' },
{ value_min: 50, value_max: 80, key: 'variant-2' },
{ value_min: 80, value_max: 100, key: 'variant-3' },
])

const variants2 = {
'variant-1': 33,
'variant-2': 33,
'variant-3': 34,
}
expect(featureFlags._variantLookupTable(variants2)).toEqual([
{ value_min: 0, value_max: 33, key: 'variant-1' },
{ value_min: 33, value_max: 66, key: 'variant-2' },
{ value_min: 66, value_max: 100, key: 'variant-3' },
])
})

it('should compute the correct hash values', () => {
const testFlag = 'test-flag'

// Repeating the hash function should produce the same output
// Same as: import hashlib; hashlib.sha1("test-flag.distinct_id_1".encode("utf-8")).hexdigest()[:15]
expect(featureFlags._hash('test-flag.distinct_id_1')).toBe('59f5e7274a66f06')
expect(featureFlags._hash('test-flag.distinct_id_1')).toBe('59f5e7274a66f06')
// A different input should produce a different hash
// Same as: import hashlib; hashlib.sha1("test-flag.distinct_id_2".encode("utf-8")).hexdigest()[:15]
expect(featureFlags._hash('test-flag.distinct_id_2')).toBe('59589dd697c3745')

// Same identifier should get same hash
// distinct_id_1 + test-flag = 0.35140843114131903
expect(featureFlags._get_hash(testFlag, 'distinct_id_1')).toBeCloseTo(0.35140843114131903)
expect(featureFlags._get_hash(testFlag, 'distinct_id_1')).toBeCloseTo(0.35140843114131903)

// Different identifiers should get different hashes
// distinct_id_2 + test-flag = 0.34900843133051557
expect(featureFlags._get_hash(testFlag, 'distinct_id_2')).toBeCloseTo(0.34900843133051557)

// Different salt should produce different hash
// distinct_id_1 + test-flag + salt = 0.05659409091269017
expect(featureFlags._get_hash(testFlag, 'distinct_id_1', 'salt')).toBeCloseTo(0.05659409091269017)

// Different flag keys should produce different hashes
const differentFlag = 'different-flag'
// distinct_id_1 + different-flag = 0.5078604702829128
expect(featureFlags._get_hash(differentFlag, 'distinct_id_1')).toBeCloseTo(0.5078604702829128)
})
})
})

describe('parseFeatureFlagDecideResponse', () => {
Expand Down
27 changes: 27 additions & 0 deletions src/__tests__/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,33 @@ describe('posthog core', () => {
})
})

describe('client assigned feature flags', () => {
it('onFeatureFlags should be called immediately if client assigned feature flags are defined', () => {
let called = false
const posthog = posthogWith({
bootstrap: {
clientAssignedFeatureFlags: [{ key: 'test-flag', variants: { test: 0.5, control: 0.5 } }],
},
})

posthog.featureFlags.onFeatureFlags(() => (called = true))
expect(called).toEqual(true)
})

it('onFeatureFlags should not be called immediately if client assigned feature flags bootstrap is empty', () => {
let called = false

const posthog = posthogWith({
bootstrap: {
clientAssignedFeatureFlags: [],
},
})

posthog.featureFlags.onFeatureFlags(() => (called = true))
expect(called).toEqual(false)
})
})

describe('init()', () => {
jest.spyOn(window, 'window', 'get')

Expand Down
28 changes: 28 additions & 0 deletions src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,26 @@ export class PostHog {
})
}

if (this._hasClientAssignedFeatureFlags()) {
const clientAssignedFeatureFlags: Record<string, string | boolean> = {}
const clientAssignedFeatureFlagPayloads: Record<string, JsonType> = {}

for (const flag of this.config.bootstrap?.clientAssignedFeatureFlags || []) {
const variant = this.featureFlags._getMatchingVariant(flag)
if (variant) {
clientAssignedFeatureFlags[flag.key] = variant
if (flag.payload) {
clientAssignedFeatureFlagPayloads[flag.key] = flag.payload
}
}
}

this.featureFlags.receivedFeatureFlags({
featureFlags: clientAssignedFeatureFlags,
featureFlagPayloads: clientAssignedFeatureFlagPayloads,
})
}

if (this._hasBootstrappedFeatureFlags()) {
const activeFlags = Object.keys(config.bootstrap?.featureFlags || {})
.filter((flag) => !!config.bootstrap?.featureFlags?.[flag])
Expand Down Expand Up @@ -746,6 +766,14 @@ export class PostHog {
execute(capturing_calls, this)
}

_hasClientAssignedFeatureFlags(): boolean {
return (
(this.config.bootstrap?.clientAssignedFeatureFlags &&
Object.keys(this.config.bootstrap?.clientAssignedFeatureFlags).length > 0) ||
false
)
}

_hasBootstrappedFeatureFlags(): boolean {
return (
(this.config.bootstrap?.featureFlags && Object.keys(this.config.bootstrap?.featureFlags).length > 0) ||
Expand Down
115 changes: 115 additions & 0 deletions src/posthog-featureflags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
Properties,
JsonType,
Compression,
ClientAssignedFeatureFlag,
} from './types'
import { PostHogPersistence } from './posthog-persistence'

Expand Down Expand Up @@ -483,4 +484,118 @@ export class PostHogFeatureFlags {
this.instance.unregister(STORED_GROUP_PROPERTIES_KEY)
}
}

_getMatchingVariant(featureFlag: ClientAssignedFeatureFlag): string | null {
const lookupTable = this._variantLookupTable(featureFlag.variants)
const hash = this._get_hash(featureFlag.key, this.instance.get_distinct_id(), 'variant')

for (const variant of lookupTable) {
if (hash >= variant.value_min && hash < variant.value_max) {
return variant.key
}
}
return null
}

// TODO how should this behave for erroneous values?
_variantLookupTable(variants: Record<string, number>): { value_min: number; value_max: number; key: string }[] {
const lookupTable: { value_min: number; value_max: number; key: string }[] = []
let valueMin = 0

for (const [variant, percentage] of Object.entries(variants)) {
const valueMax = valueMin + percentage
lookupTable.push({
value_min: valueMin,
value_max: valueMax,
key: variant,
})
valueMin = valueMax
}
return lookupTable
}

_get_hash(featureFlagKey: string, distinctId: string, salt: string = ''): number {
const hashKey = `${featureFlagKey}.${distinctId}${salt}`
const hashHex = this._hash(hashKey)
// TODO do we care about IE11 support for BigInt?
const hashInt = BigInt(`0x${hashHex}`)
const LONG_SCALE = BigInt('0xFFFFFFFFFFFFFFF')
return Number(hashInt) / Number(LONG_SCALE) // Normalize the hash to a value between 0 and 1
}

// TODO how much do we trust sonnet to write a hashing function?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😆

_hash(input: string): string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use sha1 in both the python and rust implementations when creating hash keys; any reason why we wouldn't do the same here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason why we wouldn't do the same here?

@dmarticus Tell me more? I ran into some IE11 linting issue with crypto, but maybe we can actually use it.

Copy link
Contributor

@dmarticus dmarticus Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argh, should've known there's some browser compat issue – there's some prior art (slack, github around separating our bundles into es5 (for things that explicitly need IE11 support) and es6 (for everything else), but I'm not sure what our policy on adding new behavior that explicitly doesn't support IE11 is.

It might be a pain to support it here though, even if crypto is technically "supported" on IE11, it seems like the support pegged to a specific version of that library, and I'm not sure if taking on the overhead of making sure we have IE11 support is worth using the library. Then again, using a standard for crypto is probably safer than rolling our owning hashing algo. TBH – the decision is up to you, I just wanted to provide my 2 cents. I definitely didn't realize this was a browser compat thing beforehand.

function rotateLeft(n: number, s: number): number {
return ((n << s) | (n >>> (32 - s))) >>> 0
}

let H0 = 0x67452301
let H1 = 0xefcdab89
let H2 = 0x98badcfe
let H3 = 0x10325476
let H4 = 0xc3d2e1f0

// Convert string to byte array
const bytes: number[] = []
for (let i = 0; i < input.length; i++) {
const char = input.charCodeAt(i)
bytes.push(char & 0xff)
}

// Add padding
bytes.push(0x80)
while ((bytes.length * 8) % 512 !== 448) {
bytes.push(0)
}

const bitLen = input.length * 8
bytes.push(0, 0, 0, 0) // JavaScript bitwise ops are 32-bit
bytes.push((bitLen >>> 24) & 0xff)
bytes.push((bitLen >>> 16) & 0xff)
bytes.push((bitLen >>> 8) & 0xff)
bytes.push(bitLen & 0xff)

// Process blocks
for (let i = 0; i < bytes.length; i += 64) {
const w = new Array(80)
for (let j = 0; j < 16; j++) {
w[j] =
(bytes[i + j * 4] << 24) |
(bytes[i + j * 4 + 1] << 16) |
(bytes[i + j * 4 + 2] << 8) |
bytes[i + j * 4 + 3]
}

for (let j = 16; j < 80; j++) {
w[j] = rotateLeft(w[j - 3] ^ w[j - 8] ^ w[j - 14] ^ w[j - 16], 1)
}

let [a, b, c, d, e] = [H0, H1, H2, H3, H4]

for (let j = 0; j < 80; j++) {
const f =
j < 20 ? (b & c) | (~b & d) : j < 40 ? b ^ c ^ d : j < 60 ? (b & c) | (b & d) | (c & d) : b ^ c ^ d

const k = j < 20 ? 0x5a827999 : j < 40 ? 0x6ed9eba1 : j < 60 ? 0x8f1bbcdc : 0xca62c1d6

const temp = (rotateLeft(a, 5) + f + e + k + w[j]) >>> 0
e = d
d = c
c = rotateLeft(b, 30)
b = a
a = temp
}

H0 = (H0 + a) >>> 0
H1 = (H1 + b) >>> 0
H2 = (H2 + c) >>> 0
H3 = (H3 + d) >>> 0
H4 = (H4 + e) >>> 0
}

return [H0, H1, H2, H3, H4]
.map((n) => n.toString(16).padStart(8, '0'))
.join('')
.slice(0, 15)
}
}
6 changes: 6 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,16 @@ export interface AutocaptureConfig {

capture_copied_text?: boolean
}
export type ClientAssignedFeatureFlag = {
key: string
variants: Record<string, number>
payload?: JsonType
}

export interface BootstrapConfig {
distinctID?: string
isIdentifiedID?: boolean
clientAssignedFeatureFlags?: ClientAssignedFeatureFlag[]
featureFlags?: Record<string, boolean | string>
featureFlagPayloads?: Record<string, JsonType>
/**
Expand Down
Loading