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

Create a new feature-flag-controller #27254

Open
9 tasks
vthomas13 opened this issue Sep 18, 2024 · 3 comments
Open
9 tasks

Create a new feature-flag-controller #27254

vthomas13 opened this issue Sep 18, 2024 · 3 comments
Assignees
Labels

Comments

@vthomas13
Copy link
Contributor

vthomas13 commented Sep 18, 2024

Objectives

The FeatureFlagController will be a specialized extension of BaseController with state management, caching, and feature flag fetching capabilities. It will use ClientConfigApi to retrieve feature flags from LaunchDarkly, applying caching to optimize API usage and ensuring user privacy through configurable settings.

  1. Centralized Access: Expose feature flags consistently across both the Extension and Mobile clients.
  2. Caching: Reduce redundant API calls by caching flag values for a short period, enhancing performance and reliability.
  3. Fallback Logic: Ensure functionality in cases where the remote server is unreachable, feature flag values should fall back to those stored locally or, if no cache exists, to default values hardcoded in the codebase.
  4. User Management: Allow user control over remote feature flag usage to maintain privacy.

High-Level Architecture

The core controller will:

  • Be published as @metamask/feature-flag-controller, detailed configuration for package.json can refer to packages/address-book-controller/package.json
  • Request feature flag values from ClientConfigApi
curl --location 'https://client-config.api.cx.metamask.io/v1/flags?client=extension&distribution=main&environment=dev' \
--header 'Accept: application/json'
  • Use profileId to determine unique user-based flag values.
curl --location 'https://client-config.api.cx.metamask.io/v1/flags?client=extension&distribution=main&environment=dev&user=f47ac10b-58cc-4372-a567-0e02b2c3d479' \
--header 'Accept: application/json'
  • Apply caching to reduce the load on ClientConfigApi.
    Provide a fallback mechanism, either using cached values or default values in case of API unavailability, or a request is made during the updating, or no cache exists.
  • Expose methods for retrieving feature flag values to both the Extension and Mobile clients.
  • Allows manual refresh for situations that need immediate data.

Implementation Details

Controller guideline:
https://github.com/MetaMask/core/blob/main/docs/writing-controllers.md
Best practice:
https://github.com/MetaMask/core/tree/0fc0396cbcaae95ea5c8bf3f92fef18adc496283/examples/example-controllers
Service guidance:
https://github.com/MetaMask/decisions/blob/main/decisions/core/0002-external-api-integrations.md

1. Define Initial State and Metadata

Set up the initial state and metadata for the FeatureFlagController to hold feature flags and cache details. This state will persist between sessions for reliability in offline scenarios.

type FeatureFlagControllerState = {
  flags: Record<string, boolean>; // Cached feature flag values
  cacheTimestamp: number;          // Timestamp to validate cache freshness
};

2. Implement Caching Mechanism
To reduce API calls, we’ll implement a cache with a validity period. Use cacheTimestamp to track when the last fetch occurred, and check this before fetching again. Also allow the caller to specify how frequently the API should be called by adding an optional fetchInterval parameter.

// Define the default cache duration (24 hours in milliseconds)
const DEFAULT_CACHE_DURATION = 30 * 24 * 60 * 60 * 1000; // 30 days

// Method to check if cache is still valid
private isCacheValid(fetchInterval: number = DEFAULT_CACHE_DURATION): boolean {
  return (Date.now() - this.state.cacheTimestamp) < fetchInterval;
}

3. Fetch Feature Flags from API in a service
The feature-flag-service will fetch feature flags from ClientConfigApi. It should handle cases with and without a profileId for user-specific flags. Implement error handling to manage API failures and cache fallback if needed.

// Define accepted values for client, distribution, and environment
type ClientType = 'extension' | 'mobile';
type DistributionType = 'main' | 'flask';
type EnvironmentType = 'prod' | 'rc' | 'dev';

// Method to fetch flags from the API with mandatory parameters and optional profileId
private async _fetchFlagsFromApi(
  client: ClientType,
  distribution: DistributionType,
  environment: EnvironmentType,
  profileId?: string
): Promise<Record<string, boolean>> {
  const url = `https://client-config.api.cx.metamask.io/v1/flags?client=${client}&distribution=${distribution}&environment=${environment}${profileId ? `&user=${profileId}` : ''}`;
  
  try {
    const response = await handleFetch(url);
    if (!response.ok) throw new Error('Failed to fetch flags');
    return await response.json();
  } catch (error) {
    console.error('Feature flag API request failed:', error);
    return {}; // Return an empty object as fallback
  }
}

and use in controller:


// Public method to retrieve all flags with caching, passing mandatory params
public async getFeatureFlags(
  client: ClientType,
  distribution: DistributionType,
  environment: EnvironmentType,
  profileId?: string,
  fetchInterval?: number = DEFAULT_CACHE_DURATION,
): Promise<Record<string, boolean>> {
  // Use cached flags if the cache is still valid
  if (this.isCacheValid(fetchInterval)) {
    return this.state.flags;
  }

  // Attempt to fetch new flags from the API
  const flags = await this.fetchFlagsFromApi(client, distribution, environment, profileId);

  // Check if we got a valid response from the API
  if (Object.keys(flags).length > 0) {
    this.updateCache(flags); // Update cache if we received flags from the API
    return flags;
  }

  // Fallback to cached flags if API failed and we have cache available
  if (Object.keys(this.state.flags).length > 0) {
    return this.state.flags;
  }

  // Fallback to empty object if both API and cache are unavailable
  // Client can determine the value if the flags are empty
  return {};
}


// Update cache with new flags and timestamp
private updateCache(flags: Record<string, boolean>) {
  this.update((state) => {
    state.flags = flags;
    state.cacheTimestamp = Date.now();
  });
}

4. Apply Fallback Logic
In case of an API failure or offline status, fallback to either cached values (if available) or default hardcoded values.

**5. Handle Simultaneous Requests with In-Progress Tracking **
The feature-flag-controller will use in-progress flags (e.g., refer to #inProgressHotlistUpdate in phishing-controller) to ensure only one update of each type occurs at any time, waiting for completion if an update is already ongoing.
This prevents redundant requests and allows the controller to be more resilient to concurrent updates, particularly helpful in a multithreaded or asynchronous environment.

public async getFeatureFlags(clientType, distributionType, environmentType, profileId): Promise<Record<string, boolean>> {
  if (this.isCacheValid()) {
    return this.state.flags;
  }

  if (this.#inProgressFlagUpdate) {
    await this.#inProgressFlagUpdate;
  }

  try {
    this.#inProgressFlagUpdate = this.fetchFlagsFromApi(clientType, distributionType, environmentType, profileId);
    const flags = await this.#inProgressFlagUpdate;

    if (Object.keys(flags).length > 0) {
      this.updateCache(flags);
      return flags;
    }
  } finally {
    this.#inProgressFlagUpdate = undefined;
  }

  return this.state.flags;
}

6. Allow Manual Refresh on Demand
Add a refreshFeatureFlags method to allow immediate refreshing of flags. This is useful for specific actions that require the latest data immediately, bypassing the cache.

public async refreshFeatureFlags(clientType, distributionType, environmentType, profileId): Promise<Record<string, boolean>> {
  const flags = await this.fetchFlagsFromApi(clientType, distributionType, environmentType, profileId);
  if (Object.keys(flags).length > 0) {
    this.updateCache(flags);
  }
  return flags;
}

7. Extend with Messaging and Registering Handlers
Set up message handlers to manage interactions and handle actions (e.g., requesting flag updates).

#registerMessageHandlers(): void {
  this.messagingSystem.registerActionHandler(
    `${controllerName}:maybeUpdateState` as const,
    this.maybeUpdateState.bind(this),
  );
}

Scenario

No response

Design

No response

Technical Details

No response

Threat Modeling Framework

No response

Acceptance Criteria

  1. A new controller feature-controller is added to core and published to be used, following the recommended pattern: https://github.com/MetaMask/core/tree/0fc0396cbcaae95ea5c8bf3f92fef18adc496283/examples/example-controllers
  2. when a consumer requests ALL feature flags from controller, controller will make request to clientConfigAPI based on client, distribution, environment and profileId (optional), and fetchInterval (optional, default 1 day)
  3. when clientConfigAPI is down, controller will return the cached value from previous recent fetch
  4. when there is no cached value stored, controller will return {} and client will decide which value as default value
  5. Allow manual polling when a certain action is triggered, eg: create a new flag
  6. unit tests will be included in this ticket

Stakeholder review needed before the work gets merged

  • Engineering (needed in most cases)
  • Design
  • Product
  • QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
  • Security
  • Legal
  • Marketing
  • Management (please specify)
  • Other (please specify)

References

No response

@DDDDDanica DDDDDanica changed the title Create a controller with unit testing for feature flags Create a new feature-flag-controller Nov 6, 2024
@metamaskbot metamaskbot added external-contributor INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. labels Nov 6, 2024
@joaoloureirop
Copy link

It should be packages/feature-flag-controller/package.json for the detailed configuration path, correct?

I have two questions / suggestions:

  1. For when the clientConfigAPI is down, fetch will rely on a retry logic? Or the default fetch (UI load for extension, cold start for Mobile) is enough?
  2. The controller state should be compatible with all flag types (boolean, number, string, json)

@DDDDDanica
Copy link
Contributor

DDDDDanica commented Nov 11, 2024

hey @joaoloureirop thanks for reviewing,

It should be packages/feature-flag-controller/package.json for the detailed configuration path, correct?

Yes indeed, the one is the expected path

For when the clientConfigAPI is down, fetch will rely on a retry logic? Or the default fetch (UI load for extension, cold start for Mobile) is enough?

If API is down, the controller actually returns a cache value from previous fetch; further more, if there's no cache available in the storage, an empty object is returned, and we can have customization in extension of mobile to decide which is the default value we want to return.

The controller state should be compatible with all flag types (boolean, number, string, json)

definitely ! The code snippet above will act as an example for some critical logics, when implementing, we have some best practice to follow, include strict types

@joaoloureirop
Copy link

Thanks for clarifying @DDDDDanica !

If API is down, the controller actually returns a cache value from previous fetch; further more, if there's no cache available in the storage, an empty object is returned, and we can have customization in extension of mobile to decide which is the default value we want to return.

Ah yes, it is up to the client (mobile, extension) to handle the empty cache return.
But how will the controller behave with non-2xx HTTP responses?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants