From cf9f91e403743affca6b1c8045539e4d0f64d071 Mon Sep 17 00:00:00 2001 From: Haven Date: Thu, 12 Dec 2024 15:44:38 -0800 Subject: [PATCH] chore(flags): Add new debugging property `$used_bootstrap_value`, `$feature_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 --- cypress/e2e/capture.cy.ts | 45 ++++++++++++++++++++++++++++++++++- src/__tests__/featureflags.ts | 19 ++++++++------- src/decide.ts | 4 +++- src/posthog-core.ts | 2 ++ src/posthog-featureflags.ts | 20 ++++++++++++---- 5 files changed, 74 insertions(+), 16 deletions(-) diff --git a/cypress/e2e/capture.cy.ts b/cypress/e2e/capture.cy.ts index 3516e35e7..bc11604d1 100644 --- a/cypress/e2e/capture.cy.ts +++ b/cypress/e2e/capture.cy.ts @@ -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', () => { diff --git a/src/__tests__/featureflags.ts b/src/__tests__/featureflags.ts index 8dd585b94..fd2b5792c 100644 --- a/src/__tests__/featureflags.ts +++ b/src/__tests__/featureflags.ts @@ -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, @@ -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', @@ -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') @@ -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', @@ -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: { @@ -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' }, }) @@ -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() @@ -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) @@ -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) => { diff --git a/src/decide.ts b/src/decide.ts index 35a1747ef..9483e4d20 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -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 { @@ -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 diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 65469a7ca..f7225bb48 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -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 @@ -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 = [] diff --git a/src/posthog-featureflags.ts b/src/posthog-featureflags.ts index 55debed5e..5adc3315c 100644 --- a/src/posthog-featureflags.ts +++ b/src/posthog-featureflags.ts @@ -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() @@ -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 } @@ -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 @@ -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 } @@ -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) @@ -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) }