Feature flag name should be unique, have a consistent default_enabled: and be used only in single context

TL;DR

Today we have a number of implicit feature flags. This does create a few problems for inconclusive default state of them.

Usage of feature flags

Usage of feature_available? and beta_feature_available?

We use feature_available? always as part of precompiling a list of feature flags, this leads to problem with beta_feature_available? as it has different default_enabled:.

Example from current master:

    condition(:group_activity_analytics_available) do
      @subject.beta_feature_available?(:group_activity_analytics)
    end

Both use the same licensed name, but have a different default state.

Usage of a colliding implicit licensed feature vs explicit feature flag

Since feature_available? has default_enabled: true this leads to a situation where we might use the same name in two different contexes, but with different defaults:

  • development
  • licensed

Example from current master:

  EEP_FEATURES = EES_FEATURES + %i[
    ...
    ci_project_subscriptions
  ]
  EEP_FEATURES.freeze
   def project_has_subscriptions?
     return false unless ::Feature.enabled?(:ci_project_subscriptions, project)

     project.downstream_projects.any?
   end

Both use the same licensed name, but have a non-consistent default state.

Usage of promo_...

This creates yet another feature flag for each licensed feature making this explicit. Do we need that? Can we model that differently?

    def promo_feature_available?(feature)
      ::Feature.enabled?("promo_#{feature}", default_enabled: false)
    end

    def feature_available_in_plan?(feature)
      return true if ::License.promo_feature_available?(feature)

      available_features = strong_memoize(:features_available_in_plan) do
        Hash.new do |h, f|
          h[f] = (plans.map(&:name) & self.class.plans_with_feature(f)).any?
        end
      end

      available_features[feature]
    end

This introduces a number of additional feature flags required to be created for each licensed feature, with a prefix of promo_.

Proposal

  • Remove default_enabled: from Feature.enabled/disabled? and read it from YAML definition
  • Extend Feature.enabled/disabled? with an expect type of feature flag, defaulting to :development or whatever name is
  • Make feature_available? to use Feature.enabled?(..., type: :licensed, ...)
  • Remove beta_feature_available? and control beta features with default_state:
  • Remove promo_feature_available? and control promo features with default_state:

Links

Edited by Kamil Trzciński