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/

static/app/actionCreators/organization.tsx

@@ -15,7 +15,10 @@ import TeamStore from 'sentry/stores/teamStore';
 import type {Organization, Team} from 'sentry/types/organization';
 import type {Project} from 'sentry/types/project';
 import FeatureFlagOverrides from 'sentry/utils/featureFlagOverrides';
-import FeatureObserver from 'sentry/utils/featureObserver';
+import {
+  addOrganizationFeaturesHandler,
+  buildSentryFeaturesHandler,
+} from 'sentry/utils/featureFlags';
 import {getPreloadedDataPromise} from 'sentry/utils/getPreloadedData';
 import parseLinkHeader from 'sentry/utils/parseLinkHeader';
 
@@ -42,8 +45,9 @@ async function fetchOrg(
   }
 
   FeatureFlagOverrides.singleton().loadOrg(org);
-  FeatureObserver.singleton({}).observeOrganizationFlags({
+  addOrganizationFeaturesHandler({
     organization: org,
+    handler: buildSentryFeaturesHandler('feature.organizations:'),
   });
 
   OrganizationStore.onUpdate(org, {replace: true});

static/app/bootstrap/initializeSdk.tsx

@@ -14,7 +14,6 @@ import {
   useNavigationType,
 } from 'react-router-dom';
 import {useEffect} from 'react';
-import FeatureObserver from 'sentry/utils/featureObserver';
 
 const SPA_MODE_ALLOW_URLS = [
   'localhost',
@@ -72,6 +71,7 @@ function getSentryIntegrations() {
       filterKeys: ['sentry-spa'],
       behaviour: 'apply-tag-if-contains-third-party-frames',
     }),
+    Sentry.featureFlagsIntegration(),
   ];
 
   return integrations;
@@ -179,15 +179,8 @@ export function initializeSdk(config: Config) {
 
       handlePossibleUndefinedResponseBodyErrors(event);
       addEndpointTagToRequestError(event);
-
       lastEventId = event.event_id || hint.event_id;
 
-      // attach feature flags to the event context
-      if (event.contexts) {
-        const flags = FeatureObserver.singleton({}).getFeatureFlags();
-        event.contexts.flags = flags;
-      }
-
       return event;
     },
   });

static/app/utils/featureFlags.spec.ts

@@ -0,0 +1,69 @@
+import {OrganizationFixture} from 'sentry-fixture/organization';
+import {ProjectFixture} from 'sentry-fixture/project';
+
+import {
+  addOrganizationFeaturesHandler,
+  addProjectFeaturesHandler,
+} from 'sentry/utils/featureFlags';
+
+describe('addOrganizationFeaturesHandler', () => {
+  let organization;
+
+  beforeEach(() => {
+    organization = OrganizationFixture({
+      features: ['enable-issues', 'enable-replay'],
+    });
+  });
+
+  it('should pass the flag name and result to the handler on each evaluation', () => {
+    const mockHandler = jest.fn();
+    addOrganizationFeaturesHandler({organization, handler: mockHandler});
+
+    organization.features.includes('enable-replay');
+    organization.features.includes('replay-mobile-ui');
+    organization.features.includes('enable-issues');
+
+    expect(mockHandler).toHaveBeenNthCalledWith(1, 'enable-replay', true);
+    expect(mockHandler).toHaveBeenNthCalledWith(2, 'replay-mobile-ui', false);
+    expect(mockHandler).toHaveBeenNthCalledWith(3, 'enable-issues', true);
+  });
+
+  it('should not change the functionality of `includes`', () => {
+    const mockHandler = jest.fn();
+    addOrganizationFeaturesHandler({organization, handler: mockHandler});
+    expect(organization.features.includes('enable-issues')).toBe(true);
+    expect(organization.features.includes('enable-replay')).toBe(true);
+    expect(organization.features.includes('replay-mobile-ui')).toBe(false);
+  });
+});
+
+describe('addProjectFeaturesHandler', () => {
+  let project;
+
+  beforeEach(() => {
+    project = ProjectFixture({
+      features: ['enable-issues', 'enable-replay'],
+    });
+  });
+
+  it('should pass the flag name and result to the handler on each evaluation', () => {
+    const mockHandler = jest.fn();
+    addProjectFeaturesHandler({project, handler: mockHandler});
+
+    project.features.includes('enable-replay');
+    project.features.includes('replay-mobile-ui');
+    project.features.includes('enable-issues');
+
+    expect(mockHandler).toHaveBeenNthCalledWith(1, 'enable-replay', true);
+    expect(mockHandler).toHaveBeenNthCalledWith(2, 'replay-mobile-ui', false);
+    expect(mockHandler).toHaveBeenNthCalledWith(3, 'enable-issues', true);
+  });
+
+  it('should not change the functionality of `includes`', () => {
+    const mockHandler = jest.fn();
+    addProjectFeaturesHandler({project, handler: mockHandler});
+    expect(project.features.includes('enable-issues')).toBe(true);
+    expect(project.features.includes('enable-replay')).toBe(true);
+    expect(project.features.includes('replay-mobile-ui')).toBe(false);
+  });
+});

static/app/utils/featureFlags.ts

@@ -0,0 +1,78 @@
+import {logger} from '@sentry/core';
+import * as Sentry from '@sentry/react';
+
+import type {Organization} from 'sentry/types/organization';
+import type {Project} from 'sentry/types/project';
+
+/**
+ * Returns a callback that can be used to track sentry flag evaluations through
+ * the Sentry SDK, in the event context. If the FeatureFlagsIntegration is not
+ * installed, the callback is a no-op.
+ *
+ * @param prefix - optionally specifies a prefix for flag names, before calling
+ *  the SDK hook
+ */
+export function buildSentryFeaturesHandler(
+  prefix?: string
+): (name: string, value: unknown) => void {
+  const featureFlagsIntegration =
+    Sentry.getClient()?.getIntegrationByName<Sentry.FeatureFlagsIntegration>(
+      'FeatureFlags'
+    );
+  if (!featureFlagsIntegration || !('addFeatureFlag' in featureFlagsIntegration)) {
+    logger.error(
+      'Unable to track flag evaluations because FeatureFlagsIntegration is not installed correctly.'
+    );
+    return (_name, _value) => {};
+  }
+  return (name: string, value: unknown) => {
+    // Append `feature.organizations:` in front to match the Sentry options automator format
+    featureFlagsIntegration?.addFeatureFlag((prefix ?? '') + name, value);
+  };
+}
+
+/**
+ * Registers a handler that processes feature names and values on each call to
+ * organization.features.includes().
+ */
+export function addOrganizationFeaturesHandler({
+  organization,
+  handler,
+}: {
+  handler: (name: string, value: unknown) => void;
+  organization: Organization;
+}) {
+  const includesHandler = {
+    apply: (includes: any, orgFeatures: string[], flagName: string[]) => {
+      // Evaluate the result of .includes() and pass it to hook before returning
+      const flagResult = includes.apply(orgFeatures, flagName);
+      handler(flagName[0], flagResult);
+      return flagResult;
+    },
+  };
+  const proxy = new Proxy(organization.features.includes, includesHandler);
+  organization.features.includes = proxy;
+}
+
+/**
+ * Registers a handler that processes feature names and values on each call to
+ * organization.features.includes().
+ */
+export function addProjectFeaturesHandler({
+  project,
+  handler,
+}: {
+  handler: (name: string, value: unknown) => void;
+  project: Project;
+}) {
+  const includesHandler = {
+    apply: (includes: any, projFeatures: string[], flagName: string[]) => {
+      // Evaluate the result of .includes() and pass it to hook before returning
+      const flagResult = includes.apply(projFeatures, flagName);
+      handler(flagName[0], flagResult);
+      return flagResult;
+    },
+  };
+  const proxy = new Proxy(project.features.includes, includesHandler);
+  project.features.includes = proxy;
+}

static/app/utils/featureObserver.spec.ts

@@ -1,251 +0,0 @@
-import {OrganizationFixture} from 'sentry-fixture/organization';
-import {ProjectFixture} from 'sentry-fixture/project';
-
-import type {Organization} from 'sentry/types/organization';
-import type {Project} from 'sentry/types/project';
-import FeatureObserver from 'sentry/utils/featureObserver';
-
-describe('FeatureObserver', () => {
-  let organization: Organization;
-  let project: Project;
-
-  beforeEach(() => {
-    organization = OrganizationFixture({
-      features: ['enable-issues', 'enable-profiling', 'enable-replay'],
-    });
-    project = ProjectFixture({
-      features: ['enable-proj-flag', 'enable-performance'],
-    });
-  });
-
-  describe('observeOrganizationFlags', () => {
-    it('should add recently evaluated org flags to the flag queue', () => {
-      const inst = new FeatureObserver({bufferSize: 3});
-      expect(organization.features).toEqual([
-        'enable-issues',
-        'enable-profiling',
-        'enable-replay',
-      ]);
-
-      inst.observeOrganizationFlags({organization});
-      expect(inst.getFeatureFlags().values).toEqual([]);
-
-      organization.features.includes('enable-issues');
-      organization.features.includes('replay-mobile-ui');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.organizations:enable-issues', result: true},
-        {flag: 'feature.organizations:replay-mobile-ui', result: false},
-      ]);
-
-      // do more evaluations to fill up and overflow the buffer
-      organization.features.includes('enable-replay');
-      organization.features.includes('autofix-ui');
-      organization.features.includes('new-issue-details');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.organizations:enable-replay', result: true},
-        {flag: 'feature.organizations:autofix-ui', result: false},
-        {flag: 'feature.organizations:new-issue-details', result: false},
-      ]);
-    });
-
-    it('should remove duplicate flags with a full queue', () => {
-      const inst = new FeatureObserver({bufferSize: 3});
-      inst.observeOrganizationFlags({organization});
-      expect(inst.getFeatureFlags().values).toEqual([]);
-
-      organization.features.includes('enable-issues');
-      organization.features.includes('replay-mobile-ui');
-      organization.features.includes('enable-discover');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.organizations:enable-issues', result: true},
-        {flag: 'feature.organizations:replay-mobile-ui', result: false},
-        {flag: 'feature.organizations:enable-discover', result: false},
-      ]);
-
-      // this is already in the queue; it should be removed and
-      // added back to the end of the queue
-      organization.features.includes('enable-issues');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.organizations:replay-mobile-ui', result: false},
-        {flag: 'feature.organizations:enable-discover', result: false},
-        {flag: 'feature.organizations:enable-issues', result: true},
-      ]);
-
-      organization.features.includes('spam-ingest');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.organizations:enable-discover', result: false},
-        {flag: 'feature.organizations:enable-issues', result: true},
-        {flag: 'feature.organizations:spam-ingest', result: false},
-      ]);
-
-      // this is already in the queue but in the back
-      // the queue should not change
-      organization.features.includes('spam-ingest');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.organizations:enable-discover', result: false},
-        {flag: 'feature.organizations:enable-issues', result: true},
-        {flag: 'feature.organizations:spam-ingest', result: false},
-      ]);
-    });
-
-    it('should remove duplicate flags with an unfilled queue', () => {
-      const inst = new FeatureObserver({bufferSize: 3});
-      inst.observeOrganizationFlags({organization});
-      expect(inst.getFeatureFlags().values).toEqual([]);
-
-      organization.features.includes('enable-issues');
-      organization.features.includes('replay-mobile-ui');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.organizations:enable-issues', result: true},
-        {flag: 'feature.organizations:replay-mobile-ui', result: false},
-      ]);
-
-      // this is already in the queue; it should be removed and
-      // added back to the end of the queue
-      organization.features.includes('enable-issues');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.organizations:replay-mobile-ui', result: false},
-        {flag: 'feature.organizations:enable-issues', result: true},
-      ]);
-
-      // this is already in the queue but in the back
-      // the queue should not change
-      organization.features.includes('enable-issues');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.organizations:replay-mobile-ui', result: false},
-        {flag: 'feature.organizations:enable-issues', result: true},
-      ]);
-    });
-
-    it('should not change the functionality of `includes`', () => {
-      const inst = new FeatureObserver({bufferSize: 3});
-      inst.observeOrganizationFlags({organization});
-      expect(inst.getFeatureFlags().values).toEqual([]);
-
-      organization.features.includes('enable-issues');
-      organization.features.includes('replay-mobile-ui');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.organizations:enable-issues', result: true},
-        {flag: 'feature.organizations:replay-mobile-ui', result: false},
-      ]);
-
-      expect(organization.features.includes('enable-issues')).toBe(true);
-      expect(organization.features.includes('replay-mobile-ui')).toBe(false);
-    });
-  });
-
-  describe('observeProjectFlags', () => {
-    it('should add recently evaluated proj flags to the flag queue', () => {
-      const inst = new FeatureObserver({bufferSize: 3});
-      expect(project.features).toEqual(['enable-proj-flag', 'enable-performance']);
-
-      inst.observeProjectFlags({project});
-      expect(inst.getFeatureFlags().values).toEqual([]);
-
-      project.features.includes('enable-proj-flag');
-      project.features.includes('replay-mobile-ui');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.projects:enable-proj-flag', result: true},
-        {flag: 'feature.projects:replay-mobile-ui', result: false},
-      ]);
-
-      // do more evaluations to fill up and overflow the buffer
-      project.features.includes('enable-performance');
-      project.features.includes('autofix-ui');
-      project.features.includes('new-issue-details');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.projects:enable-performance', result: true},
-        {flag: 'feature.projects:autofix-ui', result: false},
-        {flag: 'feature.projects:new-issue-details', result: false},
-      ]);
-    });
-
-    it('should not change the functionality of `includes`', () => {
-      const inst = new FeatureObserver({bufferSize: 3});
-      inst.observeProjectFlags({project});
-      expect(inst.getFeatureFlags().values).toEqual([]);
-
-      project.features.includes('enable-proj-flag');
-      project.features.includes('replay-mobile-ui');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.projects:enable-proj-flag', result: true},
-        {flag: 'feature.projects:replay-mobile-ui', result: false},
-      ]);
-
-      expect(project.features.includes('enable-proj-flag')).toBe(true);
-      expect(project.features.includes('replay-mobile-ui')).toBe(false);
-    });
-  });
-
-  describe('observeProjectFlags and observeOrganizationFlags', () => {
-    it('should add recently evaluated org and proj flags to the flag queue', () => {
-      const inst = new FeatureObserver({bufferSize: 3});
-      expect(project.features).toEqual(['enable-proj-flag', 'enable-performance']);
-      expect(organization.features).toEqual([
-        'enable-issues',
-        'enable-profiling',
-        'enable-replay',
-      ]);
-
-      inst.observeProjectFlags({project});
-      inst.observeOrganizationFlags({organization});
-      expect(inst.getFeatureFlags().values).toEqual([]);
-
-      project.features.includes('enable-proj-flag');
-      project.features.includes('enable-replay');
-      organization.features.includes('enable-issues');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.projects:enable-proj-flag', result: true},
-        {flag: 'feature.projects:enable-replay', result: false},
-        {flag: 'feature.organizations:enable-issues', result: true},
-      ]);
-
-      organization.features.includes('enable-replay');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.projects:enable-replay', result: false},
-        {flag: 'feature.organizations:enable-issues', result: true},
-        {flag: 'feature.organizations:enable-replay', result: true},
-      ]);
-    });
-
-    it('should keep the same queue if the project changes', () => {
-      const inst = new FeatureObserver({bufferSize: 3});
-      expect(project.features).toEqual(['enable-proj-flag', 'enable-performance']);
-      expect(organization.features).toEqual([
-        'enable-issues',
-        'enable-profiling',
-        'enable-replay',
-      ]);
-
-      inst.observeProjectFlags({project});
-      inst.observeOrganizationFlags({organization});
-      expect(inst.getFeatureFlags().values).toEqual([]);
-
-      project.features.includes('enable-proj-flag');
-      project.features.includes('enable-replay');
-      organization.features.includes('enable-issues');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.projects:enable-proj-flag', result: true},
-        {flag: 'feature.projects:enable-replay', result: false},
-        {flag: 'feature.organizations:enable-issues', result: true},
-      ]);
-
-      project = ProjectFixture({
-        features: ['enable-new-flag'],
-      });
-      inst.observeProjectFlags({project});
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.projects:enable-proj-flag', result: true},
-        {flag: 'feature.projects:enable-replay', result: false},
-        {flag: 'feature.organizations:enable-issues', result: true},
-      ]);
-
-      project.features.includes('enable-new-flag');
-      expect(inst.getFeatureFlags().values).toEqual([
-        {flag: 'feature.projects:enable-replay', result: false},
-        {flag: 'feature.organizations:enable-issues', result: true},
-        {flag: 'feature.projects:enable-new-flag', result: true},
-      ]);
-    });
-  });
-});

static/app/utils/featureObserver.ts

@@ -1,111 +0,0 @@
-import type {FeatureFlagContext} from '@sentry/core/build/types/types-hoist/context';
-
-import type {Organization} from 'sentry/types/organization';
-import type {Project} from 'sentry/types/project';
-
-const FEATURE_FLAG_BUFFER_SIZE = 100;
-let __SINGLETON: FeatureObserver | null = null;
-
-export default class FeatureObserver {
-  /**
-   * Return the same instance of FeatureObserver in each part of the app.
-   * Multiple instances of FeatureObserver are needed by tests only.
-   */
-  public static singleton({
-    bufferSize = FEATURE_FLAG_BUFFER_SIZE,
-  }: {
-    bufferSize?: number;
-  }) {
-    if (!__SINGLETON) {
-      __SINGLETON = new FeatureObserver({bufferSize});
-    }
-    return __SINGLETON;
-  }
-
-  private _bufferSize = 0;
-  private FEATURE_FLAGS: FeatureFlagContext = {values: []};
-
-  constructor({bufferSize}: {bufferSize: number}) {
-    this._bufferSize = bufferSize;
-  }
-
-  /**
-   * Return list of recently accessed feature flags.
-   */
-  public getFeatureFlags() {
-    return this.FEATURE_FLAGS;
-  }
-
-  public updateFlagBuffer({
-    flagName,
-    flagResult,
-  }: {
-    flagName: string;
-    flagResult: boolean;
-  }) {
-    const flagBuffer = this.FEATURE_FLAGS;
-    // Check if the flag is already in the buffer
-    const index = flagBuffer.values.findIndex(f => f.flag === flagName);
-
-    // The flag is already in the buffer
-    if (index !== -1) {
-      flagBuffer.values.splice(index, 1);
-    }
-
-    // If at capacity, we need to remove the earliest flag
-    // This will only happen if not a duplicate flag
-    if (flagBuffer.values.length === this._bufferSize) {
-      flagBuffer.values.shift();
-    }
-
-    // Store the flag and its result in the buffer
-    flagBuffer.values.push({
-      flag: flagName,
-      result: flagResult,
-    });
-  }
-
-  public observeOrganizationFlags({organization}: {organization: Organization}) {
-    // Track names of features that are passed into the .includes() function.
-    const handler = {
-      apply: (target: any, orgFeatures: string[], flagName: string[]) => {
-        // Evaluate the result of .includes()
-        const flagResult = target.apply(orgFeatures, flagName);
-
-        // Append `feature.organizations:` in front to match the Sentry options automator format
-        const name = 'feature.organizations:' + flagName[0];
-
-        this.updateFlagBuffer({
-          flagName: name,
-          flagResult,
-        });
-
-        return flagResult;
-      },
-    };
-    const proxy = new Proxy(organization.features.includes, handler);
-    organization.features.includes = proxy;
-  }
-
-  public observeProjectFlags({project}: {project: Project}) {
-    // Track names of features that are passed into the .includes() function.
-    const handler = {
-      apply: (target: any, projFeatures: string[], flagName: string[]) => {
-        // Evaluate the result of .includes()
-        const flagResult = target.apply(projFeatures, flagName);
-
-        // Append `feature.projects:` in front to match the Sentry options automator format
-        const name = 'feature.projects:' + flagName[0];
-
-        this.updateFlagBuffer({
-          flagName: name,
-          flagResult,
-        });
-
-        return flagResult;
-      },
-    };
-    const proxy = new Proxy(project.features.includes, handler);
-    project.features.includes = proxy;
-  }
-}

static/app/views/projects/projectContext.tsx

@@ -17,7 +17,10 @@ import {space} from 'sentry/styles/space';
 import type {Organization} from 'sentry/types/organization';
 import type {Project} from 'sentry/types/project';
 import type {User} from 'sentry/types/user';
-import FeatureObserver from 'sentry/utils/featureObserver';
+import {
+  addProjectFeaturesHandler,
+  buildSentryFeaturesHandler,
+} from 'sentry/utils/featureFlags';
 import withApi from 'sentry/utils/withApi';
 import withOrganization from 'sentry/utils/withOrganization';
 import withProjects from 'sentry/utils/withProjects';
@@ -181,8 +184,9 @@ class ProjectContextProvider extends Component<Props, State> {
 
         // assuming here that this means the project is considered the active project
         setActiveProject(project);
-        FeatureObserver.singleton({}).observeProjectFlags({
+        addProjectFeaturesHandler({
           project,
+          handler: buildSentryFeaturesHandler('feature.projects:'),
         });
       } catch (error) {
         this.setState({