Skip to content

Commit 977fe77

Browse files
authored
ref(flags): instrument sdk featureFlagsIntegration to track FE flag evals (#81954)
Re-opening https://github.com/getsentry/sentry/pull/81159/files after releasing a custom flag tracking integration in JS SDK 8.43. WIth this integration, we don't need to manage our own buffer in featureObserver anymore. Sentry employees shouldn't notice any changes, this is a proof-of-concept for the new SDK. The SDK has its own tests already, so I'm only unit testing the add(__)FeaturesHook fxs. Manually tested with dev-ui sending to a test org: https://andrew-onboarding.sentry.io/share/issue/664703eb3a98441b8a203dc44e803cee/
1 parent cd2520e commit 977fe77

File tree

7 files changed

+160
-374
lines changed

7 files changed

+160
-374
lines changed

static/app/actionCreators/organization.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ import TeamStore from 'sentry/stores/teamStore';
1515
import type {Organization, Team} from 'sentry/types/organization';
1616
import type {Project} from 'sentry/types/project';
1717
import FeatureFlagOverrides from 'sentry/utils/featureFlagOverrides';
18-
import FeatureObserver from 'sentry/utils/featureObserver';
18+
import {
19+
addOrganizationFeaturesHandler,
20+
buildSentryFeaturesHandler,
21+
} from 'sentry/utils/featureFlags';
1922
import {getPreloadedDataPromise} from 'sentry/utils/getPreloadedData';
2023
import parseLinkHeader from 'sentry/utils/parseLinkHeader';
2124

@@ -42,8 +45,9 @@ async function fetchOrg(
4245
}
4346

4447
FeatureFlagOverrides.singleton().loadOrg(org);
45-
FeatureObserver.singleton({}).observeOrganizationFlags({
48+
addOrganizationFeaturesHandler({
4649
organization: org,
50+
handler: buildSentryFeaturesHandler('feature.organizations:'),
4751
});
4852

4953
OrganizationStore.onUpdate(org, {replace: true});

static/app/bootstrap/initializeSdk.tsx

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
useNavigationType,
1515
} from 'react-router-dom';
1616
import {useEffect} from 'react';
17-
import FeatureObserver from 'sentry/utils/featureObserver';
1817

1918
const SPA_MODE_ALLOW_URLS = [
2019
'localhost',
@@ -72,6 +71,7 @@ function getSentryIntegrations() {
7271
filterKeys: ['sentry-spa'],
7372
behaviour: 'apply-tag-if-contains-third-party-frames',
7473
}),
74+
Sentry.featureFlagsIntegration(),
7575
];
7676

7777
return integrations;
@@ -179,15 +179,8 @@ export function initializeSdk(config: Config) {
179179

180180
handlePossibleUndefinedResponseBodyErrors(event);
181181
addEndpointTagToRequestError(event);
182-
183182
lastEventId = event.event_id || hint.event_id;
184183

185-
// attach feature flags to the event context
186-
if (event.contexts) {
187-
const flags = FeatureObserver.singleton({}).getFeatureFlags();
188-
event.contexts.flags = flags;
189-
}
190-
191184
return event;
192185
},
193186
});
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import {OrganizationFixture} from 'sentry-fixture/organization';
2+
import {ProjectFixture} from 'sentry-fixture/project';
3+
4+
import {
5+
addOrganizationFeaturesHandler,
6+
addProjectFeaturesHandler,
7+
} from 'sentry/utils/featureFlags';
8+
9+
describe('addOrganizationFeaturesHandler', () => {
10+
let organization;
11+
12+
beforeEach(() => {
13+
organization = OrganizationFixture({
14+
features: ['enable-issues', 'enable-replay'],
15+
});
16+
});
17+
18+
it('should pass the flag name and result to the handler on each evaluation', () => {
19+
const mockHandler = jest.fn();
20+
addOrganizationFeaturesHandler({organization, handler: mockHandler});
21+
22+
organization.features.includes('enable-replay');
23+
organization.features.includes('replay-mobile-ui');
24+
organization.features.includes('enable-issues');
25+
26+
expect(mockHandler).toHaveBeenNthCalledWith(1, 'enable-replay', true);
27+
expect(mockHandler).toHaveBeenNthCalledWith(2, 'replay-mobile-ui', false);
28+
expect(mockHandler).toHaveBeenNthCalledWith(3, 'enable-issues', true);
29+
});
30+
31+
it('should not change the functionality of `includes`', () => {
32+
const mockHandler = jest.fn();
33+
addOrganizationFeaturesHandler({organization, handler: mockHandler});
34+
expect(organization.features.includes('enable-issues')).toBe(true);
35+
expect(organization.features.includes('enable-replay')).toBe(true);
36+
expect(organization.features.includes('replay-mobile-ui')).toBe(false);
37+
});
38+
});
39+
40+
describe('addProjectFeaturesHandler', () => {
41+
let project;
42+
43+
beforeEach(() => {
44+
project = ProjectFixture({
45+
features: ['enable-issues', 'enable-replay'],
46+
});
47+
});
48+
49+
it('should pass the flag name and result to the handler on each evaluation', () => {
50+
const mockHandler = jest.fn();
51+
addProjectFeaturesHandler({project, handler: mockHandler});
52+
53+
project.features.includes('enable-replay');
54+
project.features.includes('replay-mobile-ui');
55+
project.features.includes('enable-issues');
56+
57+
expect(mockHandler).toHaveBeenNthCalledWith(1, 'enable-replay', true);
58+
expect(mockHandler).toHaveBeenNthCalledWith(2, 'replay-mobile-ui', false);
59+
expect(mockHandler).toHaveBeenNthCalledWith(3, 'enable-issues', true);
60+
});
61+
62+
it('should not change the functionality of `includes`', () => {
63+
const mockHandler = jest.fn();
64+
addProjectFeaturesHandler({project, handler: mockHandler});
65+
expect(project.features.includes('enable-issues')).toBe(true);
66+
expect(project.features.includes('enable-replay')).toBe(true);
67+
expect(project.features.includes('replay-mobile-ui')).toBe(false);
68+
});
69+
});

static/app/utils/featureFlags.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import {logger} from '@sentry/core';
2+
import * as Sentry from '@sentry/react';
3+
4+
import type {Organization} from 'sentry/types/organization';
5+
import type {Project} from 'sentry/types/project';
6+
7+
/**
8+
* Returns a callback that can be used to track sentry flag evaluations through
9+
* the Sentry SDK, in the event context. If the FeatureFlagsIntegration is not
10+
* installed, the callback is a no-op.
11+
*
12+
* @param prefix - optionally specifies a prefix for flag names, before calling
13+
* the SDK hook
14+
*/
15+
export function buildSentryFeaturesHandler(
16+
prefix?: string
17+
): (name: string, value: unknown) => void {
18+
const featureFlagsIntegration =
19+
Sentry.getClient()?.getIntegrationByName<Sentry.FeatureFlagsIntegration>(
20+
'FeatureFlags'
21+
);
22+
if (!featureFlagsIntegration || !('addFeatureFlag' in featureFlagsIntegration)) {
23+
logger.error(
24+
'Unable to track flag evaluations because FeatureFlagsIntegration is not installed correctly.'
25+
);
26+
return (_name, _value) => {};
27+
}
28+
return (name: string, value: unknown) => {
29+
// Append `feature.organizations:` in front to match the Sentry options automator format
30+
featureFlagsIntegration?.addFeatureFlag((prefix ?? '') + name, value);
31+
};
32+
}
33+
34+
/**
35+
* Registers a handler that processes feature names and values on each call to
36+
* organization.features.includes().
37+
*/
38+
export function addOrganizationFeaturesHandler({
39+
organization,
40+
handler,
41+
}: {
42+
handler: (name: string, value: unknown) => void;
43+
organization: Organization;
44+
}) {
45+
const includesHandler = {
46+
apply: (includes: any, orgFeatures: string[], flagName: string[]) => {
47+
// Evaluate the result of .includes() and pass it to hook before returning
48+
const flagResult = includes.apply(orgFeatures, flagName);
49+
handler(flagName[0], flagResult);
50+
return flagResult;
51+
},
52+
};
53+
const proxy = new Proxy(organization.features.includes, includesHandler);
54+
organization.features.includes = proxy;
55+
}
56+
57+
/**
58+
* Registers a handler that processes feature names and values on each call to
59+
* organization.features.includes().
60+
*/
61+
export function addProjectFeaturesHandler({
62+
project,
63+
handler,
64+
}: {
65+
handler: (name: string, value: unknown) => void;
66+
project: Project;
67+
}) {
68+
const includesHandler = {
69+
apply: (includes: any, projFeatures: string[], flagName: string[]) => {
70+
// Evaluate the result of .includes() and pass it to hook before returning
71+
const flagResult = includes.apply(projFeatures, flagName);
72+
handler(flagName[0], flagResult);
73+
return flagResult;
74+
},
75+
};
76+
const proxy = new Proxy(project.features.includes, includesHandler);
77+
project.features.includes = proxy;
78+
}

0 commit comments

Comments
 (0)