Browse Source

feat(flags): add proj feature observer (#79455)

followup to 
- https://github.com/getsentry/sentry/pull/77532
- https://github.com/getsentry/sentry/pull/79445


add a feature observer for calls to `project.features.includes(...)`
similar to the one we have for `organization.features.includes(...)`.

this one behaves the same way as the old one. i refactored out the
buffer updating code so we can reuse it for both cases. the flag
evaluations for both org + project are pushed into the same flag queue,
which is already being used in the `beforeSend` in `initializeSdk`.
Michelle Zhang 4 months ago
parent
commit
866bd2151e

+ 3 - 1
static/app/actionCreators/organization.tsx

@@ -42,7 +42,9 @@ async function fetchOrg(
   }
 
   FeatureFlagOverrides.singleton().loadOrg(org);
-  FeatureObserver.singleton().observeFlags({organization: org, bufferSize: 100});
+  FeatureObserver.singleton({}).observeOrganizationFlags({
+    organization: org,
+  });
 
   OrganizationStore.onUpdate(org, {replace: true});
   setActiveOrganization(org);

+ 1 - 1
static/app/bootstrap/initializeSdk.tsx

@@ -185,7 +185,7 @@ export function initializeSdk(config: Config) {
 
       // attach feature flags to the event context
       if (event.contexts) {
-        const flags = FeatureObserver.singleton().getFeatureFlags();
+        const flags = FeatureObserver.singleton({}).getFeatureFlags();
         event.contexts.flags = flags;
       }
 

+ 134 - 11
static/app/utils/featureObserver.spec.ts

@@ -1,25 +1,33 @@
 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;
+  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('observeFlags', () => {
-    it('should add recently evaluated flags to the flag queue', () => {
-      const inst = new FeatureObserver();
+  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.observeFlags({organization, bufferSize: 3});
+      inst.observeOrganizationFlags({organization});
       expect(inst.getFeatureFlags().values).toEqual([]);
 
       organization.features.includes('enable-issues');
@@ -41,8 +49,8 @@ describe('FeatureObserver', () => {
     });
 
     it('should remove duplicate flags with a full queue', () => {
-      const inst = new FeatureObserver();
-      inst.observeFlags({organization, bufferSize: 3});
+      const inst = new FeatureObserver({bufferSize: 3});
+      inst.observeOrganizationFlags({organization});
       expect(inst.getFeatureFlags().values).toEqual([]);
 
       organization.features.includes('enable-issues');
@@ -81,8 +89,8 @@ describe('FeatureObserver', () => {
     });
 
     it('should remove duplicate flags with an unfilled queue', () => {
-      const inst = new FeatureObserver();
-      inst.observeFlags({organization, bufferSize: 3});
+      const inst = new FeatureObserver({bufferSize: 3});
+      inst.observeOrganizationFlags({organization});
       expect(inst.getFeatureFlags().values).toEqual([]);
 
       organization.features.includes('enable-issues');
@@ -110,8 +118,8 @@ describe('FeatureObserver', () => {
     });
 
     it('should not change the functionality of `includes`', () => {
-      const inst = new FeatureObserver();
-      inst.observeFlags({organization, bufferSize: 3});
+      const inst = new FeatureObserver({bufferSize: 3});
+      inst.observeOrganizationFlags({organization});
       expect(inst.getFeatureFlags().values).toEqual([]);
 
       organization.features.includes('enable-issues');
@@ -125,4 +133,119 @@ describe('FeatureObserver', () => {
       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},
+      ]);
+    });
+  });
 });

+ 65 - 26
static/app/utils/featureObserver.ts

@@ -1,6 +1,8 @@
 import type {Flags} from 'sentry/types/event';
 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 {
@@ -8,15 +10,24 @@ 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() {
+  public static singleton({
+    bufferSize = FEATURE_FLAG_BUFFER_SIZE,
+  }: {
+    bufferSize?: number;
+  }) {
     if (!__SINGLETON) {
-      __SINGLETON = new FeatureObserver();
+      __SINGLETON = new FeatureObserver({bufferSize});
     }
     return __SINGLETON;
   }
 
+  private _bufferSize = 0;
   private FEATURE_FLAGS: Flags = {values: []};
 
+  constructor({bufferSize}: {bufferSize: number}) {
+    this._bufferSize = bufferSize;
+  }
+
   /**
    * Return list of recently accessed feature flags.
    */
@@ -24,48 +35,76 @@ export default class FeatureObserver {
     return this.FEATURE_FLAGS;
   }
 
-  public observeFlags({
-    organization,
-    bufferSize,
+  public updateFlagBuffer({
+    flagName,
+    flagResult,
   }: {
-    bufferSize: number;
-    organization: Organization;
+    flagName: string;
+    flagResult: boolean;
   }) {
-    const FLAGS = this.FEATURE_FLAGS;
+    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: function (target, orgFeatures, flagName) {
+      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];
 
-        // Check if the flag is already in the buffer
-        const index = FLAGS.values.findIndex(f => f.flag === name);
+        this.updateFlagBuffer({
+          flagName: name,
+          flagResult,
+        });
 
-        // The flag is already in the buffer
-        if (index !== -1) {
-          FLAGS.values.splice(index, 1);
-        }
+        return flagResult;
+      },
+    };
+    const proxy = new Proxy(organization.features.includes, handler);
+    organization.features.includes = proxy;
+  }
 
-        // If at capacity, we need to remove the earliest flag
-        // This will only happen if not a duplicate flag
-        if (FLAGS.values.length === bufferSize) {
-          FLAGS.values.shift();
-        }
+  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];
 
-        // Store the flag and its result in the buffer
-        FLAGS.values.push({
-          flag: name,
-          result: flagResult,
+        this.updateFlagBuffer({
+          flagName: name,
+          flagResult,
         });
 
         return flagResult;
       },
     };
-    const proxy = new Proxy(organization.features.includes, handler);
-    organization.features.includes = proxy;
+    const proxy = new Proxy(project.features.includes, handler);
+    project.features.includes = proxy;
   }
 }

+ 4 - 0
static/app/views/projects/projectContext.tsx

@@ -17,6 +17,7 @@ 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 withApi from 'sentry/utils/withApi';
 import withOrganization from 'sentry/utils/withOrganization';
 import withProjects from 'sentry/utils/withProjects';
@@ -180,6 +181,9 @@ class ProjectContextProvider extends Component<Props, State> {
 
         // assuming here that this means the project is considered the active project
         setActiveProject(project);
+        FeatureObserver.singleton({}).observeProjectFlags({
+          project,
+        });
       } catch (error) {
         this.setState({
           loading: false,