Commit 0902a6f8 authored by Alex Kalderimis's avatar Alex Kalderimis

Merge branch 'modify-experiments-api' into 'master'

Make experiments API a filter of features API

See merge request gitlab-org/gitlab!66488
parents fec40483 b7cfcaf8
...@@ -28,13 +28,51 @@ Example response: ...@@ -28,13 +28,51 @@ Example response:
```json ```json
[ [
{ {
"key": "experiment_1", "key": "code_quality_walkthrough",
"enabled": true "definition": {
"name": "code_quality_walkthrough",
"introduced_by_url": "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/58900",
"rollout_issue_url": "https://gitlab.com/gitlab-org/gitlab/-/issues/327229",
"milestone": "13.12",
"type": "experiment",
"group": "group::activation",
"default_enabled": false
}, },
{ "current_status": {
"key": "experiment_2", "state": "conditional",
"enabled": false "gates": [
{
"key": "boolean",
"value": false
},
{
"key": "percentage_of_actors",
"value": 25
}
]
} }
},
{
"key": "ci_runner_templates",
"definition": {
"name": "ci_runner_templates",
"introduced_by_url": "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/58357",
"rollout_issue_url": "https://gitlab.com/gitlab-org/gitlab/-/issues/326725",
"milestone": "14.0",
"type": "experiment",
"group": "group::activation",
"default_enabled": false
},
"current_status": {
"state": "off",
"gates": [
{
"key": "boolean",
"value": false
}
]
}
}
] ]
``` ```
...@@ -15,7 +15,9 @@ module API ...@@ -15,7 +15,9 @@ module API
experiment(:null_hypothesis, canary: true, user: current_user) do |e| experiment(:null_hypothesis, canary: true, user: current_user) do |e|
e.use { bad_request! 'experimentation may not be working right now' } e.use { bad_request! 'experimentation may not be working right now' }
e.try { experiments = Feature::Definition.definitions.values.select { |d| d.attributes[:type] == 'experiment' } } e.try do
experiments = Feature::Definition.definitions.values.select { |d| d.attributes[:type] == 'experiment' }
end
end end
present experiments, with: EE::API::Entities::Experiment, current_user: current_user present experiments, with: EE::API::Entities::Experiment, current_user: current_user
......
...@@ -3,17 +3,33 @@ ...@@ -3,17 +3,33 @@
module EE module EE
module API module API
module Entities module Entities
class Experiment < ::API::Entities::Feature::Definition class Experiment < Grape::Entity
expose :key do |definition| expose :key do |definition|
definition.attributes[:name].gsub(/_experiment_percentage$/, '') definition.attributes[:name].gsub(/_experiment_percentage$/, '')
end end
expose :enabled do |definition| expose :definition, using: ::API::Entities::Feature::Definition do |feature|
feature(definition).state != :off ::Feature::Definition.definitions[feature.name.to_sym]
end end
expose :state do |definition| class CurrentStatus < Grape::Entity
feature(definition).state expose :state
expose :gates, using: ::API::Entities::FeatureGate do |model|
model.gates.map do |gate|
value = model.gate_values[gate.key]
# By default all gate values are populated. Only show relevant ones.
if (value.is_a?(Integer) && value == 0) || (value.is_a?(Set) && value.empty?)
next
end
{ key: gate.key, value: value }
end.compact
end
end
expose :current_status, using: CurrentStatus do |definition|
feature(definition)
end end
private private
......
...@@ -11,38 +11,65 @@ RSpec.describe EE::API::Entities::Experiment do ...@@ -11,38 +11,65 @@ RSpec.describe EE::API::Entities::Experiment do
it do it do
is_expected.to match( is_expected.to match(
key: "null_hypothesis", key: "null_hypothesis",
state: :off, definition: {
enabled: false, name: 'null_hypothesis',
name: "null_hypothesis", introduced_by_url: 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45840',
introduced_by_url: "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45840", rollout_issue_url: nil,
rollout_issue_url: nil, milestone: '13.7',
milestone: "13.7", type: 'experiment',
type: "experiment", group: 'group::adoption',
group: "group::adoption", default_enabled: false
default_enabled: false },
current_status: {
state: :off,
gates: [
{
key: :boolean,
value: false
}
]
}
) )
end end
it "understands conditional state and what that means" do it "understands conditional state and what that means" do
Feature.enable_percentage_of_time(:null_hypothesis, 1) Feature.enable_percentage_of_time(:null_hypothesis, 1)
expect(subject).to include( expect(subject[:current_status]).to match({
state: :conditional, state: :conditional,
enabled: true gates: [
) {
key: :boolean,
value: false
},
{
key: :percentage_of_time,
value: 1
}
]
})
end end
it "understands state and what that means for if its enabled or not" do it "understands state and what that means for if its enabled or not" do
Feature.enable_percentage_of_time(:null_hypothesis, 100) Feature.enable_percentage_of_time(:null_hypothesis, 100)
expect(subject).to include( expect(subject[:current_status]).to match({
state: :on, state: :on,
enabled: true gates: [
) {
key: :boolean,
value: false
},
{
key: :percentage_of_time,
value: 100
}
]
})
end end
it "truncates the name since some experiments include extra data in their feature flag name" do it "truncates the name since some experiments include extra data in their feature flag name" do
experiment.attributes[:name] = 'foo_experiment_percentage' allow(experiment).to receive(:attributes).and_return({ name: 'foo_experiment_percentage' })
expect(subject).to include( expect(subject).to include(
key: 'foo' key: 'foo'
......
...@@ -50,34 +50,54 @@ RSpec.describe API::Experiments do ...@@ -50,34 +50,54 @@ RSpec.describe API::Experiments do
group.add_developer(user) group.add_developer(user)
end end
it 'returns the feature flag details' do it 'returns the feature flag details', :aggregate_failures do
get api('/experiments', user) get api('/experiments', user)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to include({ expect(json_response).to include({
key: 'null_hypothesis', key: 'null_hypothesis',
state: :off, definition: {
enabled: false, name: 'null_hypothesis',
name: 'null_hypothesis', introduced_by_url: 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45840',
introduced_by_url: 'https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45840', rollout_issue_url: nil,
rollout_issue_url: nil, milestone: '13.7',
milestone: '13.7', type: 'experiment',
type: 'experiment', group: 'group::adoption',
group: 'group::adoption', default_enabled: false
default_enabled: false },
current_status: {
state: :off,
gates: [
{
key: :boolean,
value: false
}
]
}
}.as_json) }.as_json)
end end
it 'understands the state of the feature flag and what that means for an experiment' do it 'understands the state of the feature flag and what that means for an experiment', :aggregate_failures do
Feature.enable_percentage_of_actors(:null_hypothesis, 1) Feature.enable_percentage_of_actors(:null_hypothesis, 1)
get api('/experiments', user) get api('/experiments', user)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to include(hash_including({ expect(json_response).to include(hash_including({
state: :conditional, key: 'null_hypothesis',
enabled: true, current_status: {
name: 'null_hypothesis' state: :conditional,
gates: [
{
key: :boolean,
value: false
},
{
key: :percentage_of_actors,
value: 1
}
]
}
}.as_json)) }.as_json))
end end
...@@ -127,7 +147,7 @@ RSpec.describe API::Experiments do ...@@ -127,7 +147,7 @@ RSpec.describe API::Experiments do
}) })
end end
it 'returns a 400 if experimentation seems broken' do it 'returns a 400 if experimentation seems broken', :aggregate_failures do
# we assume that rendering control would only be done in error. # we assume that rendering control would only be done in error.
stub_experiments(null_hypothesis: :control) stub_experiments(null_hypothesis: :control)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment