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

chore(flags): use the new /flags endpoint to manage flag evaluation for team 2 #1841

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
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
60 changes: 60 additions & 0 deletions functional_tests/feature-flags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,3 +333,63 @@ describe('FunctionalTests / Feature Flags', () => {
})
})
})

describe('feature flags v2', () => {
let token: string

beforeEach(() => {
token = uuidv7()
})

it('should call flags endpoint when eligible', async () => {
const posthog = await createPosthogInstance(token, {
__preview_flags_v2: true,
__preview_remote_config: true,
advanced_disable_decide: false,
})

await waitFor(() => {
expect(getRequests(token)['/flags/']).toEqual([
expect.objectContaining({
token,
distinct_id: posthog.get_distinct_id(),
}),
])
})
})

it('should call decide endpoint when not eligible', async () => {
const posthog = await createPosthogInstance(token, {
__preview_flags_v2: false,
__preview_remote_config: true,
advanced_disable_decide: false,
})

await waitFor(() => {
expect(getRequests(token)['/decide/']).toEqual([
expect.objectContaining({
token,
distinct_id: posthog.get_distinct_id(),
}),
])
})
})

// TODO: eventually I want to deprecate these behavior, but for now I want to make sure we don't break people
it('should call decide endpoint when preview flags is enabled but remote config is disabled', async () => {
const posthog = await createPosthogInstance(token, {
__preview_flags_v2: true,
__preview_remote_config: false,
advanced_disable_decide: false,
})

await waitFor(() => {
expect(getRequests(token)['/decide/']).toEqual([
expect.objectContaining({
token,
distinct_id: posthog.get_distinct_id(),
}),
])
})
})
})
10 changes: 9 additions & 1 deletion functional_tests/mock-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ import { RestRequest } from 'msw'
import { decompressSync, strFromU8 } from 'fflate'

// the request bodies in a store that we can inspect within tests.
const capturedRequests: { '/e/': any[]; '/engage/': any[]; '/decide/': any[] } = {
const capturedRequests: { '/e/': any[]; '/engage/': any[]; '/decide/': any[]; '/flags/': any[] } = {
'/e/': [],
'/engage/': [],
'/decide/': [],
'/flags/': [],
}

const handleRequest = (group: string) => (req: RestRequest, res: ResponseComposition, ctx: RestContext) => {
Expand Down Expand Up @@ -49,6 +50,9 @@ const server = setupServer(
}),
rest.post('http://localhost/decide/', (req, res, ctx) => {
return handleRequest('/decide/')(req, res, ctx)
}),
rest.post('http://localhost/flags/', (req, res, ctx) => {
return handleRequest('/flags/')(req, res, ctx)
})
)

Expand All @@ -66,6 +70,7 @@ export const getRequests = (token: string) => {
'/e/': capturedRequests['/e/'].filter((request) => request.properties.token === token),
'/engage/': capturedRequests['/engage/'].filter((request) => request.properties.token === token),
'/decide/': capturedRequests['/decide/'].filter((request) => request.token === token),
'/flags/': capturedRequests['/flags/'].filter((request) => request.token === token),
}
}

Expand All @@ -80,5 +85,8 @@ export const resetRequests = (token: string) => {
'/decide/': (capturedRequests['/decide/'] = capturedRequests['/decide/'].filter(
(request) => request.token !== token
)),
'/flags/': (capturedRequests['/flags/'] = capturedRequests['/flags/'].filter(
(request) => request.token !== token
)),
})
}
1 change: 1 addition & 0 deletions playground/nextjs/src/posthog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ if (typeof window !== 'undefined') {
opt_in_site_apps: true,
__preview_remote_config: true,
__preview_experimental_cookieless_mode: false,
__preview_flags_v2: true,
...configForConsent(),
})
// Help with debugging
Expand Down
5 changes: 5 additions & 0 deletions src/__tests__/__snapshots__/config-snapshot.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,11 @@ exports[`config snapshot for PostHogConfig 1`] = `
\\"false\\",
\\"true\\"
],
\\"__preview_flags_v2\\": [
\\"undefined\\",
\\"false\\",
\\"true\\"
],
\\"api_method\\": [
\\"undefined\\",
\\"string\\"
Expand Down
17 changes: 12 additions & 5 deletions src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1794,11 +1794,18 @@ export class PostHog {
}
if (this.config.debug) {
Config.DEBUG = true
logger.info('set_config', {
config,
oldConfig,
newConfig: { ...this.config },
})
logger.info(
'set_config',
JSON.stringify(
{
config,
oldConfig,
newConfig: { ...this.config },
},
null,
2
)
)
}

this.sessionRecording?.startIfEnabledOrStop()
Expand Down
10 changes: 9 additions & 1 deletion src/posthog-featureflags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,17 @@ export class PostHogFeatureFlags {
data.disable_flags = true
}

// NB: flags v2 requires remote config to be enabled as well, since the idea is that we will skip calling /decide altogether
// (which remote config does) and just use the /flags endpoint. May revisit this if we need to support flags v2 without remote config
// (e.g. we could call `/decide` with flags disabled for the data otherwise returned by remote config, and then still call
// `/flags/` to get the flag evaluation data).
const eligibleForFlagsV2 =
this.instance.config.__preview_flags_v2 && this.instance.config.__preview_remote_config

this._requestInFlight = true
this.instance._send_request({
method: 'POST',
url: this.instance.requestRouter.endpointFor('api', '/decide/?v=4'),
url: this.instance.requestRouter.endpointFor('api', eligibleForFlagsV2 ? '/flags/?v=2' : '/decide/?v=4'),
data,
compression: this.instance.config.disable_compression ? undefined : Compression.Base64,
timeout: this.instance.config.feature_flag_request_timeout_ms,
Expand All @@ -412,6 +419,7 @@ export class PostHogFeatureFlags {

this._requestInFlight = false

// NB: this block is only reached if this.instance.config.__preview_remote_config is false
if (!this._decideCalled) {
this._decideCalled = true
this.instance._onRemoteConfig(response.json ?? {})
Expand Down
6 changes: 6 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,12 @@ export interface PostHogConfig {
* */
__preview_experimental_cookieless_mode?: boolean

/**
* PREVIEW - MAY CHANGE WITHOUT WARNING - DO NOT USE IN PRODUCTION
* Whether to use the new /flags/ endpoint
* */
__preview_flags_v2?: boolean

// ------- RETIRED CONFIGS - NO REPLACEMENT OR USAGE -------

/** @deprecated - NOT USED ANYMORE, kept here for backwards compatibility reasons */
Expand Down
Loading