From 439c65e108d5c9da157df424188bcf4376770571 Mon Sep 17 00:00:00 2001 From: Pierre Tessier Date: Mon, 6 Nov 2023 11:49:46 -0600 Subject: [PATCH 1/8] use float to check for flag probability Signed-off-by: Pierre Tessier --- .../feature_flags/feature_flag.ex | 3 +- .../templates/feature_flag/form.html.heex | 6 +- .../20220524172636_create_featureflags.exs | 10 +-- src/featureflagservice/src/ffs_service.erl | 76 ++++++++++--------- 4 files changed, 51 insertions(+), 44 deletions(-) diff --git a/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex b/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex index 42f79755cf..e1472c7744 100644 --- a/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex +++ b/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex @@ -8,7 +8,7 @@ defmodule Featureflagservice.FeatureFlags.FeatureFlag do schema "featureflags" do field :description, :string - field :enabled, :boolean, default: false + field :enabled, :float, default: 0.0 field :name, :string timestamps() @@ -19,6 +19,7 @@ defmodule Featureflagservice.FeatureFlags.FeatureFlag do feature_flag |> cast(attrs, [:name, :description, :enabled]) |> validate_required([:name, :description, :enabled]) + |> validate_number(:enabled, greater_than_or_equal_to: 0.0, less_than_or_equal_to: 1.0) |> unique_constraint(:name) end end diff --git a/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex b/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex index c730c993b3..0c8030069c 100644 --- a/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex +++ b/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex @@ -29,10 +29,8 @@ <%= text_input f, :description %> <%= error_tag f, :description %> -
- <%= checkbox f, :enabled %> - <%= label f, :enabled, class: "label-inline" %> -
+ <%= label f, :enabled %> + <%= text_input f, :enabled %> <%= error_tag f, :enabled %> <%= submit "Save" %> diff --git a/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs b/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs index a33df54743..47aeb2c391 100644 --- a/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs +++ b/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs @@ -7,7 +7,7 @@ defmodule Featureflagservice.Repo.Migrations.CreateFeatureflags do create table(:featureflags) do add :name, :string add :description, :string - add :enabled, :boolean, default: false, null: false + add :enabled, :float, default: 0, null: false timestamps() end @@ -21,22 +21,22 @@ defmodule Featureflagservice.Repo.Migrations.CreateFeatureflags do repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "productCatalogFailure", description: "Fail product catalog service on a specific product", - enabled: false}) + enabled: 0.0}) repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "recommendationCache", description: "Cache recommendations", - enabled: false}) + enabled: 0.0}) repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "adServiceFailure", description: "Fail ad service requests sporadically", - enabled: false}) + enabled: 0.0}) repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "cartServiceFailure", description: "Fail cart service requests sporadically", - enabled: false}) + enabled: 0.0}) end defp execute_down do diff --git a/src/featureflagservice/src/ffs_service.erl b/src/featureflagservice/src/ffs_service.erl index 6cf534c601..bab2e38d2b 100644 --- a/src/featureflagservice/src/ffs_service.erl +++ b/src/featureflagservice/src/ffs_service.erl @@ -17,56 +17,64 @@ -behaviour(ffs_service_bhvr). -export([get_flag/2, - create_flag/2, - update_flag/2, - list_flags/2, - delete_flag/2]). + create_flag/2, + update_flag/2, + list_flags/2, + delete_flag/2]). -include_lib("grpcbox/include/grpcbox.hrl"). -include_lib("opentelemetry_api/include/otel_tracer.hrl"). -spec get_flag(ctx:t(), ffs_demo_pb:get_flag_request()) -> - {ok, ffs_demo_pb:get_flag_response(), ctx:t()} | grpcbox_stream:grpc_error_response(). + {ok, ffs_demo_pb:get_flag_response(), ctx:t()} | grpcbox_stream:grpc_error_response(). get_flag(Ctx, #{name := Name}) -> - case 'Elixir.Featureflagservice.FeatureFlags':get_feature_flag_by_name(Name) of - nil -> - {grpc_error, {?GRPC_STATUS_NOT_FOUND, <<"the requested feature flag does not exist">>}}; - #{'__struct__' := 'Elixir.Featureflagservice.FeatureFlags.FeatureFlag', - description := Description, - enabled := Enabled, - inserted_at := CreatedAt, - updated_at := UpdatedAt - } -> - ?set_attribute('app.featureflag.name', Name), - ?set_attribute('app.featureflag.enabled', Enabled), - {ok, Epoch} = 'Elixir.NaiveDateTime':from_erl({{1970, 1, 1}, {0, 0, 0}}), - CreatedAtSeconds = 'Elixir.NaiveDateTime':diff(CreatedAt, Epoch), - UpdatedAtSeconds = 'Elixir.NaiveDateTime':diff(UpdatedAt, Epoch), - Flag = #{name => Name, - description => Description, - enabled => Enabled, - created_at => #{seconds => CreatedAtSeconds, nanos => 0}, - updated_at => #{seconds => UpdatedAtSeconds, nanos => 0}}, - {ok, #{flag => Flag}, Ctx} - end. + case 'Elixir.Featureflagservice.FeatureFlags':get_feature_flag_by_name(Name) of + nil -> + {grpc_error, {?GRPC_STATUS_NOT_FOUND, <<"the requested feature flag does not exist">>}}; + #{'__struct__' := 'Elixir.Featureflagservice.FeatureFlags.FeatureFlag', + description := Description, + enabled := Enabled, + inserted_at := CreatedAt, + updated_at := UpdatedAt + } -> + RandomNumber = rand:uniform(100), % Generate a random number between 0 and 100 + Probability = trunc(Enabled * 100), % Convert the Enabled value to a percentage + FlagEnabledValue = RandomNumber =< Probability, % Determine if the random number falls within the probability range + + ?set_attribute('app.featureflag.name', Name), + ?set_attribute('app.featureflag.raw_value', Enabled), + ?set_attribute('app.featureflag.enabled', FlagEnabledValue), + + {ok, Epoch} = 'Elixir.NaiveDateTime':from_erl({{1970, 1, 1}, {0, 0, 0}}), + CreatedAtSeconds = 'Elixir.NaiveDateTime':diff(CreatedAt, Epoch), + UpdatedAtSeconds = 'Elixir.NaiveDateTime':diff(UpdatedAt, Epoch), + + Flag = #{name => Name, + description => Description, + enabled => FlagEnabledValue, + created_at => #{seconds => CreatedAtSeconds, nanos => 0}, + updated_at => #{seconds => UpdatedAtSeconds, nanos => 0}}, + + {ok, #{flag => Flag}, Ctx} + end. -spec create_flag(ctx:t(), ffs_demo_pb:create_flag_request()) -> - {ok, ffs_demo_pb:create_flag_response(), ctx:t()} | grpcbox_stream:grpc_error_response(). + {ok, ffs_demo_pb:create_flag_response(), ctx:t()} | grpcbox_stream:grpc_error_response(). create_flag(_Ctx, _) -> - {grpc_error, {?GRPC_STATUS_UNIMPLEMENTED, <<"use the web interface to create flags.">>}}. + {grpc_error, {?GRPC_STATUS_UNIMPLEMENTED, <<"use the web interface to create flags.">>}}. -spec update_flag(ctx:t(), ffs_demo_pb:update_flag_request()) -> - {ok, ffs_demo_pb:update_flag_response(), ctx:t()} | grpcbox_stream:grpc_error_response(). + {ok, ffs_demo_pb:update_flag_response(), ctx:t()} | grpcbox_stream:grpc_error_response(). update_flag(_Ctx, _) -> - {grpc_error, {?GRPC_STATUS_UNIMPLEMENTED, <<"use the web interface to update flags.">>}}. + {grpc_error, {?GRPC_STATUS_UNIMPLEMENTED, <<"use the web interface to update flags.">>}}. -spec list_flags(ctx:t(), ffs_demo_pb:list_flags_request()) -> - {ok, ffs_demo_pb:list_flags_response(), ctx:t()} | grpcbox_stream:grpc_error_response(). + {ok, ffs_demo_pb:list_flags_response(), ctx:t()} | grpcbox_stream:grpc_error_response(). list_flags(_Ctx, _) -> - {grpc_error, {?GRPC_STATUS_UNIMPLEMENTED, <<"use the web interface to view all flags.">>}}. + {grpc_error, {?GRPC_STATUS_UNIMPLEMENTED, <<"use the web interface to view all flags.">>}}. -spec delete_flag(ctx:t(), ffs_demo_pb:delete_flag_request()) -> - {ok, ffs_demo_pb:delete_flag_response(), ctx:t()} | grpcbox_stream:grpc_error_response(). + {ok, ffs_demo_pb:delete_flag_response(), ctx:t()} | grpcbox_stream:grpc_error_response(). delete_flag(_Ctx, _) -> - {grpc_error, {?GRPC_STATUS_UNIMPLEMENTED, <<"use the web interface to delete flags.">>}}. + {grpc_error, {?GRPC_STATUS_UNIMPLEMENTED, <<"use the web interface to delete flags.">>}}. From fbef8162ce290d08d6fd34fb20ea5c4084e4be5a Mon Sep 17 00:00:00 2001 From: Pierre Tessier Date: Mon, 6 Nov 2023 11:55:27 -0600 Subject: [PATCH 2/8] use float to check for flag probability Signed-off-by: Pierre Tessier --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 808e9656bb..c30590bde3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ release. ## Unreleased +* Add ability to do probabilistic A/B testing with feature flags + ([#1237](https://github.com/open-telemetry/opentelemetry-demo/pull/1237)) + ## 1.6.0 * update PHP quoteservice to use RC1 From 8ff0b270ca7c4ae4780c217f328dca462f7a0430 Mon Sep 17 00:00:00 2001 From: Austin Parker Date: Mon, 6 Nov 2023 16:42:57 -0600 Subject: [PATCH 3/8] Update CHANGELOG.md --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12b97437fe..4225b4e479 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,6 @@ release. * update PHP quoteservice to use 1.0.0 ([#1236](https://github.com/open-telemetry/opentelemetry-demo/pull/1236)) - ## 1.6.0 * update PHP quoteservice to use RC1 From 0f057dfc3fe5686aa3aede68296e5713435959c5 Mon Sep 17 00:00:00 2001 From: Josh Lee Date: Thu, 16 Nov 2023 17:18:48 -0500 Subject: [PATCH 4/8] Add help text for float feature flags; Make upgrades possible for postgres --- .../templates/feature_flag/form.html.heex | 3 ++- .../20220524172636_create_featureflags.exs | 16 +++++++++------- ...31104172636_update_featureflags_use_float.exs | 7 +++++++ 3 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 src/featureflagservice/priv/repo/migrations/20231104172636_update_featureflags_use_float.exs diff --git a/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex b/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex index 0c8030069c..c01310f22c 100644 --- a/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex +++ b/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex @@ -30,7 +30,8 @@ <%= error_tag f, :description %> <%= label f, :enabled %> - <%= text_input f, :enabled %> + <%= number_input f, :enabled, min: 0, max: 1, step: 0.1, "aria-describedby": "enabled_help_text" %> +

A value between 0.0 and 1.0. 0.0 is always disabled, 1.0 is always enabled, and all values between set a percentage chance on each request.

<%= error_tag f, :enabled %> <%= submit "Save" %> diff --git a/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs b/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs index 47aeb2c391..9462edebb6 100644 --- a/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs +++ b/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs @@ -1,5 +1,3 @@ - - defmodule Featureflagservice.Repo.Migrations.CreateFeatureflags do use Ecto.Migration @@ -7,7 +5,7 @@ defmodule Featureflagservice.Repo.Migrations.CreateFeatureflags do create table(:featureflags) do add :name, :string add :description, :string - add :enabled, :float, default: 0, null: false + add :enabled, :boolean, default: false, null: false timestamps() end @@ -21,22 +19,26 @@ defmodule Featureflagservice.Repo.Migrations.CreateFeatureflags do repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "productCatalogFailure", description: "Fail product catalog service on a specific product", - enabled: 0.0}) + enabled: false + }) repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "recommendationCache", description: "Cache recommendations", - enabled: 0.0}) + enabled: false + }) repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "adServiceFailure", description: "Fail ad service requests sporadically", - enabled: 0.0}) + enabled: false + }) repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "cartServiceFailure", description: "Fail cart service requests sporadically", - enabled: 0.0}) + enabled: false + }) end defp execute_down do diff --git a/src/featureflagservice/priv/repo/migrations/20231104172636_update_featureflags_use_float.exs b/src/featureflagservice/priv/repo/migrations/20231104172636_update_featureflags_use_float.exs new file mode 100644 index 0000000000..117cff788b --- /dev/null +++ b/src/featureflagservice/priv/repo/migrations/20231104172636_update_featureflags_use_float.exs @@ -0,0 +1,7 @@ +defmodule Featureflagservice.Repo.Migrations.UpdateFeatureFlagsUseFloat do + use Ecto.Migration + + def change do + "alter table featureflags alter enabled DROP DEFAULT,alter table featureflags alter enabled type numeric(2,1) using (case when enabled then 1.0 else 0.0 end), alter enabled set default '0.0';" + end +end From 149a3de4cc13419921649fc42ca7495de44cd84a Mon Sep 17 00:00:00 2001 From: Pierre Tessier Date: Tue, 21 Nov 2023 00:20:40 -0500 Subject: [PATCH 5/8] add description for feature flag value Signed-off-by: Pierre Tessier --- .../templates/feature_flag/form.html.heex | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex b/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex index 0c8030069c..536af6d1f7 100644 --- a/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex +++ b/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex @@ -30,6 +30,9 @@ <%= error_tag f, :description %> <%= label f, :enabled %> +
+ 0 = false. 1 = true. Values between 0 and 1 are treated as a random probability. +
<%= text_input f, :enabled %> <%= error_tag f, :enabled %> From 24b5922af8abe8050034d710dcc651dd53870cb2 Mon Sep 17 00:00:00 2001 From: Pierre Tessier Date: Sun, 3 Dec 2023 18:24:01 -0500 Subject: [PATCH 6/8] clean up help text Signed-off-by: Pierre Tessier --- .../templates/feature_flag/form.html.heex | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex b/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex index 6df2e210fb..442fa4942b 100644 --- a/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex +++ b/src/featureflagservice/lib/featureflagservice_web/templates/feature_flag/form.html.heex @@ -30,8 +30,14 @@ <%= error_tag f, :description %> <%= label f, :enabled %> - <%= number_input f, :enabled, min: 0, max: 1, step: 0.1, "aria-describedby": "enabled_help_text" %> -

A value between 0.0 and 1.0. 0.0 is always disabled, 1.0 is always enabled, and all values between set a percentage chance on each request.

+ <%= number_input f, :enabled, min: 0, max: 1, step: 0.01, "aria-describedby": "enabled_help_text" %> +

+ A decimal value between 0 and 1 (inclusive)
+ 0.0 is always disabled
+ 1.0 is always enabled
+ All values between set a percentage chance on each request
+ example: 0.55 is enabled 55% of the time
+

<%= error_tag f, :enabled %> <%= submit "Save" %> From 125ad35257b5fe600ff05818624a585a81687e70 Mon Sep 17 00:00:00 2001 From: Josh Lee Date: Sat, 9 Dec 2023 10:21:45 -0500 Subject: [PATCH 7/8] return to single migration with decimal type --- .../featureflagservice/feature_flags/feature_flag.ex | 3 +-- .../migrations/20220524172636_create_featureflags.exs | 10 +++++----- .../20231104172636_update_featureflags_use_float.exs | 7 ------- 3 files changed, 6 insertions(+), 14 deletions(-) delete mode 100644 src/featureflagservice/priv/repo/migrations/20231104172636_update_featureflags_use_float.exs diff --git a/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex b/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex index e1472c7744..4a540758d9 100644 --- a/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex +++ b/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex @@ -1,14 +1,13 @@ # Copyright The OpenTelemetry Authors # SPDX-License-Identifier: Apache-2.0 - defmodule Featureflagservice.FeatureFlags.FeatureFlag do use Ecto.Schema import Ecto.Changeset schema "featureflags" do field :description, :string - field :enabled, :float, default: 0.0 + field :enabled, :decimal, default: 0.0 field :name, :string timestamps() diff --git a/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs b/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs index 9462edebb6..e322d20785 100644 --- a/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs +++ b/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs @@ -5,7 +5,7 @@ defmodule Featureflagservice.Repo.Migrations.CreateFeatureflags do create table(:featureflags) do add :name, :string add :description, :string - add :enabled, :boolean, default: false, null: false + add :enabled, :decimal, default: 0.0, null: false timestamps() end @@ -19,25 +19,25 @@ defmodule Featureflagservice.Repo.Migrations.CreateFeatureflags do repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "productCatalogFailure", description: "Fail product catalog service on a specific product", - enabled: false + enabled: 0.0 }) repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "recommendationCache", description: "Cache recommendations", - enabled: false + enabled: 0.0 }) repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "adServiceFailure", description: "Fail ad service requests sporadically", - enabled: false + enabled: 0.0 }) repo().insert(%Featureflagservice.FeatureFlags.FeatureFlag{ name: "cartServiceFailure", description: "Fail cart service requests sporadically", - enabled: false + enabled: 0.0 }) end diff --git a/src/featureflagservice/priv/repo/migrations/20231104172636_update_featureflags_use_float.exs b/src/featureflagservice/priv/repo/migrations/20231104172636_update_featureflags_use_float.exs deleted file mode 100644 index 117cff788b..0000000000 --- a/src/featureflagservice/priv/repo/migrations/20231104172636_update_featureflags_use_float.exs +++ /dev/null @@ -1,7 +0,0 @@ -defmodule Featureflagservice.Repo.Migrations.UpdateFeatureFlagsUseFloat do - use Ecto.Migration - - def change do - "alter table featureflags alter enabled DROP DEFAULT,alter table featureflags alter enabled type numeric(2,1) using (case when enabled then 1.0 else 0.0 end), alter enabled set default '0.0';" - end -end From 71388f0d811cf473b404be5b49df450852526584 Mon Sep 17 00:00:00 2001 From: Pierre Tessier Date: Wed, 13 Dec 2023 15:24:01 -0500 Subject: [PATCH 8/8] use float instead of decimal Signed-off-by: Pierre Tessier --- .../lib/featureflagservice/feature_flags/feature_flag.ex | 2 +- .../priv/repo/migrations/20220524172636_create_featureflags.exs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex b/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex index 4a540758d9..ed68a5841a 100644 --- a/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex +++ b/src/featureflagservice/lib/featureflagservice/feature_flags/feature_flag.ex @@ -7,7 +7,7 @@ defmodule Featureflagservice.FeatureFlags.FeatureFlag do schema "featureflags" do field :description, :string - field :enabled, :decimal, default: 0.0 + field :enabled, :float, default: 0.0 field :name, :string timestamps() diff --git a/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs b/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs index e322d20785..1fd816fbfe 100644 --- a/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs +++ b/src/featureflagservice/priv/repo/migrations/20220524172636_create_featureflags.exs @@ -5,7 +5,7 @@ defmodule Featureflagservice.Repo.Migrations.CreateFeatureflags do create table(:featureflags) do add :name, :string add :description, :string - add :enabled, :decimal, default: 0.0, null: false + add :enabled, :float, default: 0.0, null: false timestamps() end