Skip to content

Commit

Permalink
chore(flags): Add new debugging property $used_bootstrap_value, `$f…
Browse files Browse the repository at this point in the history
…eature_flag_bootstrapped_response` and `$feature_flag_bootstrapped_payload` to `$feature_flag_called` event (#1567)

* chore(flags): Add new debugging property `$feature_flag_bootstrapped_response` and `$feature_flag_bootstrapped_payload` to `$feature_flag_called` event

* add test

* Add $used_bootstrap_value evt property, update tests

* bump testcafe result check timeout to 20m from 10m

* bump testcafe result check timeout to 60m just to get a passing run

* bump

* temp bump 120m timeout for checking results

* bump again to get unblocked

* tweak
  • Loading branch information
havenbarnes authored Dec 12, 2024
1 parent c5ac0de commit cf9f91e
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 16 deletions.
45 changes: 44 additions & 1 deletion cypress/e2e/capture.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,50 @@ describe('Event capture', () => {

cy.get('[data-cy-feature-flag-button]').click()

cy.phCaptures().should('include', '$feature_flag_called')
cy.phCaptures({ full: true }).then((captures) => {
const flagCallEvents = captures.filter((capture) => capture.event === '$feature_flag_called')
expect(flagCallEvents.length).to.eq(1)
const flagCallEvent = flagCallEvents[0]
expect(flagCallEvent.properties.$feature_flag_bootstrapped_response).to.not.exist
expect(flagCallEvent.properties.$feature_flag_bootstrapped_payload).to.not.exist
expect(flagCallEvent.properties.$used_bootstrap_value).to.equal(false)
})
})

it('captures $feature_flag_called with bootstrapped value properties', () => {
start({
options: {
bootstrap: {
featureFlags: {
'some-feature': 'some-value',
},
featureFlagPayloads: {
'some-feature': 'some-payload',
},
},
advanced_disable_feature_flags: true,
},
waitForDecide: false,
})

cy.intercept(
'/decide/*',
{ times: 1 },
{
forceNetworkError: true,
}
)

cy.get('[data-cy-feature-flag-button]').click()

cy.phCaptures({ full: true }).then((captures) => {
const flagCallEvents = captures.filter((capture) => capture.event === '$feature_flag_called')
expect(flagCallEvents.length).to.eq(1)
const flagCallEvent = flagCallEvents[0]
expect(flagCallEvent.properties.$feature_flag_bootstrapped_response).to.equal('some-value')
expect(flagCallEvent.properties.$feature_flag_bootstrapped_payload).to.equal('some-payload')
expect(flagCallEvent.properties.$used_bootstrap_value).to.equal(true)
})
})

it('captures rage clicks', () => {
Expand Down
19 changes: 10 additions & 9 deletions src/__tests__/featureflags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('featureflags', () => {
get_property: (key) => instance.persistence.props[key],
capture: () => {},
decideEndpointWasHit: false,
receivedFlagValues: false,
_send_request: jest.fn().mockImplementation(({ callback }) =>
callback({
statusCode: 200,
Expand Down Expand Up @@ -67,7 +68,7 @@ describe('featureflags', () => {
})

it('should return flags from persistence even if decide endpoint was not hit', () => {
featureFlags.instance.decideEndpointWasHit = false
featureFlags.instance.receivedFlagValues = false

expect(featureFlags.getFlags()).toEqual([
'beta-feature',
Expand All @@ -80,7 +81,7 @@ describe('featureflags', () => {

it('should warn if decide endpoint was not hit and no flags exist', () => {
;(window as any).POSTHOG_DEBUG = true
featureFlags.instance.decideEndpointWasHit = false
featureFlags.instance.receivedFlagValues = false
instance.persistence.unregister('$enabled_feature_flags')
instance.persistence.unregister('$active_feature_flags')

Expand All @@ -101,7 +102,7 @@ describe('featureflags', () => {
})

it('should return the right feature flag and call capture', () => {
featureFlags.instance.decideEndpointWasHit = false
featureFlags.instance.receivedFlagValues = false

expect(featureFlags.getFlags()).toEqual([
'beta-feature',
Expand Down Expand Up @@ -132,7 +133,7 @@ describe('featureflags', () => {
})

it('should call capture for every different flag response', () => {
featureFlags.instance.decideEndpointWasHit = true
featureFlags.instance.receivedFlagValues = true

instance.persistence.register({
$enabled_feature_flags: {
Expand All @@ -156,13 +157,13 @@ describe('featureflags', () => {
instance.persistence.register({
$enabled_feature_flags: {},
})
featureFlags.instance.decideEndpointWasHit = false
featureFlags.instance.receivedFlagValues = false
expect(featureFlags.getFlagVariants()).toEqual({})
expect(featureFlags.isFeatureEnabled('beta-feature')).toEqual(undefined)
// no extra capture call because flags haven't loaded yet.
expect(instance.capture).toHaveBeenCalledTimes(1)

featureFlags.instance.decideEndpointWasHit = true
featureFlags.instance.receivedFlagValues = true
instance.persistence.register({
$enabled_feature_flags: { x: 'y' },
})
Expand All @@ -185,7 +186,7 @@ describe('featureflags', () => {
})

it('should return the right feature flag and not call capture', () => {
featureFlags.instance.decideEndpointWasHit = true
featureFlags.instance.receivedFlagValues = true

expect(featureFlags.isFeatureEnabled('beta-feature', { send_event: false })).toEqual(true)
expect(instance.capture).not.toHaveBeenCalled()
Expand Down Expand Up @@ -315,7 +316,7 @@ describe('featureflags', () => {
})

it('onFeatureFlags callback should be called immediately if feature flags were loaded', () => {
featureFlags.instance.decideEndpointWasHit = true
featureFlags.instance.receivedFlagValues = true
let called = false
featureFlags.onFeatureFlags(() => (called = true))
expect(called).toEqual(true)
Expand All @@ -324,7 +325,7 @@ describe('featureflags', () => {
})

it('onFeatureFlags should not return flags that are off', () => {
featureFlags.instance.decideEndpointWasHit = true
featureFlags.instance.receivedFlagValues = true
let _flags = []
let _variants = {}
featureFlags.onFeatureFlags((flags, variants) => {
Expand Down
4 changes: 3 additions & 1 deletion src/decide.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const logger = createLogger('[Decide]')
export class Decide {
constructor(private readonly instance: PostHog) {
// don't need to wait for `decide` to return if flags were provided on initialisation
this.instance.decideEndpointWasHit = this.instance._hasBootstrappedFeatureFlags()
this.instance.receivedFlagValues = this.instance._hasBootstrappedFeatureFlags()
}

private _loadRemoteConfigJs(cb: (config?: RemoteConfig) => void): void {
Expand Down Expand Up @@ -119,6 +119,8 @@ export class Decide {
this.instance.featureFlags.receivedFeatureFlags(response ?? {}, errorsLoading)
}

this.instance.decideEndpointWasHit = !errorsLoading

if (errorsLoading) {
logger.error('Failed to fetch feature flags from PostHog.')
return
Expand Down
2 changes: 2 additions & 0 deletions src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ export class PostHog {
compression?: Compression
__request_queue: QueuedRequestOptions[]
decideEndpointWasHit: boolean
receivedFlagValues: boolean
analyticsDefaultEndpoint: string
version = Config.LIB_VERSION
_initialPersonProfilesConfig: 'always' | 'never' | 'identified_only' | null
Expand All @@ -295,6 +296,7 @@ export class PostHog {
this.config = defaultConfig()

this.decideEndpointWasHit = false
this.receivedFlagValues = false
this.SentryIntegration = SentryIntegration
this.sentryIntegration = (options?: SentryIntegrationOptions) => sentryIntegration(this, options)
this.__request_queue = []
Expand Down
20 changes: 15 additions & 5 deletions src/posthog-featureflags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ export class PostHogFeatureFlags {
// This is because we don't want to block clients waiting for flags to load.
// It's possible they're waiting for the callback to render the UI, but it never occurs.
this.receivedFeatureFlags(response.json ?? {}, errorsLoading)
this.instance.decideEndpointWasHit = !errorsLoading

// :TRICKY: Reload - start another request if queued!
this._startReloadTimer()
Expand All @@ -229,7 +230,7 @@ export class PostHogFeatureFlags {
* @param {Object|String} options (optional) If {send_event: false}, we won't send an $feature_flag_call event to PostHog.
*/
getFeatureFlag(key: string, options: { send_event?: boolean } = {}): boolean | string | undefined {
if (!this.instance.decideEndpointWasHit && !(this.getFlags() && this.getFlags().length > 0)) {
if (!this.instance.receivedFlagValues && !(this.getFlags() && this.getFlags().length > 0)) {
logger.warn('getFeatureFlag for key "' + key + '" failed. Feature flags didn\'t load in time.')
return undefined
}
Expand All @@ -246,7 +247,16 @@ export class PostHogFeatureFlags {
}
this.instance.persistence?.register({ [FLAG_CALL_REPORTED]: flagCallReported })

this.instance.capture('$feature_flag_called', { $feature_flag: key, $feature_flag_response: flagValue })
this.instance.capture('$feature_flag_called', {
$feature_flag: key,
$feature_flag_response: flagValue,
$feature_flag_payload: this.getFeatureFlagPayload(key) || null,
$feature_flag_bootstrapped_response: this.instance.config.bootstrap?.featureFlags?.[key] || null,
$feature_flag_bootstrapped_payload:
this.instance.config.bootstrap?.featureFlagPayloads?.[key] || null,
// If we haven't yet received a response from the /decide endpoint, we must have used the bootstrapped value
$used_bootstrap_value: !this.instance.decideEndpointWasHit,
})
}
}
return flagValue
Expand All @@ -268,7 +278,7 @@ export class PostHogFeatureFlags {
* @param {Object|String} options (optional) If {send_event: false}, we won't send an $feature_flag_call event to PostHog.
*/
isFeatureEnabled(key: string, options: { send_event?: boolean } = {}): boolean | undefined {
if (!this.instance.decideEndpointWasHit && !(this.getFlags() && this.getFlags().length > 0)) {
if (!this.instance.receivedFlagValues && !(this.getFlags() && this.getFlags().length > 0)) {
logger.warn('isFeatureEnabled for key "' + key + '" failed. Feature flags didn\'t load in time.')
return undefined
}
Expand All @@ -287,7 +297,7 @@ export class PostHogFeatureFlags {
if (!this.instance.persistence) {
return
}
this.instance.decideEndpointWasHit = true
this.instance.receivedFlagValues = true
const currentFlags = this.getFlagVariants()
const currentFlagPayloads = this.getFlagPayloads()
parseFeatureFlagDecideResponse(response, this.instance.persistence, currentFlags, currentFlagPayloads)
Expand Down Expand Up @@ -341,7 +351,7 @@ export class PostHogFeatureFlags {
*/
onFeatureFlags(callback: FeatureFlagsCallback): () => void {
this.addFeatureFlagsHandler(callback)
if (this.instance.decideEndpointWasHit) {
if (this.instance.receivedFlagValues) {
const { flags, flagVariants } = this._prepareFeatureFlagsForCallbacks()
callback(flags, flagVariants)
}
Expand Down

0 comments on commit cf9f91e

Please sign in to comment.