presto icon indicating copy to clipboard operation
presto copied to clipboard

Add Feature Toggles

Open branimir-vujicic opened this issue 2 years ago • 8 comments

Feature Toggles should allow teams to modify system behavior without changing code. Feature Toggles are configured using google guice. Basic definition of toggles are crated using FeatureToggleBinder. FeatureToggleBinder creates FeatureToggle and additional configuration can be done using feature configuration. In current stage Feature Toggles supports:

  • if / else based feature toggles
  • Dependency Injection based Hot reloading implementation without restart require code refactoring to add an interface when injecting the new implementation/class
  • using various toggle strategies along with simple on / off toggles

configuration:

to allow feature toggle configuration four lines are needed in config.properties file

features.config-source-type=file
features.refresh-period=30s

configuration-source-type is source type for Feature Toggles configuration features.config-source is a source (file) of the configuration features.config-type format in which configuration is stored (json or properties) features.refresh-period configuration refresh period

Defining Feature Toggles

Feature toggle definition is done in google guice module using FeatureToggleBinder

simple feature toggle definition

    featureToggleBinder(binder)
                        .featureId("featureXX")
                        .bind()

This example creates bindings for @Inject

    @Inject
    public Runner(@FeatureToggle("featureXX") Supplier<Boolean> isFeatureXXEnabled)
    {
        this.isFeatureXXEnabled = isFeatureXXEnabled;
    }

isFeatureXXEnabled can be used to test if feature is enabled or disabled:

    boolean testFeatureXXEnabled()
    {
        return isFeatureXXEnabled.get();
    }

hot reloadable feature toggle definition

featureToggleBinder(binder, Feature01.class)
                        .featureId("feature01")
                        .baseClass(Feature01.class)
                        .defaultClass(Feature01Impl01.class)
                        .allOf(Feature01Impl01.class, Feature01Impl02.class)
                        .bind()

adding Feature Toggle switching strategy

featureToggleBinder(binder)
                        .featureId("feature04")
                        .toggleStrategy("AllowAll")
                        .toggleStrategyConfig(ImmutableMap.of("key", "value", "key2", "value2"))

feature-config.properties file example

# feature query-logger
feature.query-logger.enabled=true
feature.query-logger.strategy=OsToggle
feature.query-logger.strategy.os_name=.*Linux.*

#feature.query-rate-limiter
feature.query-rate-limiter.currentInstance=com.facebook.presto.server.protocol.QueryBlockingRateLimiter

# feature.query-cancel
feature.query-cancel.strategy=AllowList
feature.query-cancel.strategy.allow-list-source=.*IDEA.*
feature.query-cancel.strategy.allow-list-user=.*prestodb

in this example for first feature query-logger changing value of feature.query-logger.enabled to false will 'disable' this feature. Changes will be effective within refresh period.

Test plan - Unit Tests

== RELEASE NOTES ==

General Changes
 * Add support for Feature Toggles.

branimir-vujicic avatar Nov 17 '22 14:11 branimir-vujicic

@bot kick off tests

v-jizhang avatar Nov 17 '22 19:11 v-jizhang

@tdcmeehan

Standard airlift configuration doesn't allow refreshing configurations. Feature Toggles are meant to be dynamic and to allow changing configuration on runtime.

branimir-vujicic avatar Jun 22 '23 12:06 branimir-vujicic

HI @tdcmeehan, I updated the configuration loading to use the plugin mechanism.

branimir-vujicic avatar Sep 01 '23 12:09 branimir-vujicic

Thank you for refactoring this into a plugin. Can you please make sure that all the file-based loading code is in one module, and all the core code to wire in feature toggles is in a separate module? One may want to swap out the file reading code with something e.g. db backed, without depending on the file loading code.

What happens if we add a feature toggle in the code, but the operator hasn't added a configuration source plugin?

tdcmeehan avatar Sep 01 '23 14:09 tdcmeehan

HI @tdcmeehan, I separated the feature toggle module and the feature toggle plugin. Now, the file configuration is in presto-feature-toggle-plugin. If no configuration source is configured feature toggles work with a dummy configuration source.

branimir-vujicic avatar Sep 04 '23 15:09 branimir-vujicic

@branimir-vujicic This requires a release note since this is introducing a new feature. Can you please add that in.

ajaygeorge avatar Sep 06 '23 17:09 ajaygeorge

@branimir-vujicic Are the Velox changes needed as a part of this PR. ? Seems unrelated. Can you please remove them.

ajaygeorge avatar Sep 06 '23 17:09 ajaygeorge

@ajaygeorge Velox updates are included by mistake. updated release notes.

branimir-vujicic avatar Sep 06 '23 20:09 branimir-vujicic

Closing due to lack of activity and age. If this is still relevant, please re-open this PR, rebase, and resolve any file conflicts.

steveburnett avatar Jul 15 '24 17:07 steveburnett