Skip to content

Draft: AI Feature flag removal

Dylan Griffith requested to merge 450442-automatic-feature-flag-removal into master

What does this MR do and why?

TODO

  1. Deal with javascript feature flag usage (camelCased) . Will require updating the regex to find JS files and also we need a custom prompt to explain to the AI how to remove the JS references.
  2. Fix or removed the aborted logic
    1. This was added to deal with the problem of running the keep with multiple changes at once. Without the abort option housekeeper will always try to create an MR for every change we yield. We can choose to skip the yield(change) when we know that the AI did not succeed but this brings a different problem if we've made some edits but not edited everything. Because we also rely on yield(change) to have housekeeper undo all the changes that we've made. So I was noticing problems here and I introduced abort but in my testing it didn't seem to be working and I was regularly left with extra files in my diff after running the keep when it aborted changes.
  3. We should also abort a change if ::Gitlab::Housekeeper::Shell.rubocop_autocorrect(file) fails. Right now it raises and stops running housekeeper. But it would be ideal if we could run housekeeper with -m 10 and it would just skip any changes that failed for some reason and move on to the next change. This probably requires abort to work correctly or some other way to catch these errors and undo all our changes and continue on.
  4. Add an additional check after all changes are made to grep the codebase one more time. We should abort the change if we find any remaining references to the feature flag
  5. Extract the abort and filter_identifiers improvements into a separate MR to be merged first as they are general improvements . They probably also need tests
  6. Run keep for 5 MRs and figure out what problems it's having and add more TODOs for those problems
  7. Keep iterating until this is more reliable. It doesn't always have to succesfully remove every feature flag but we want to minimize noise and not create too many MRs that are wrong and someone has to take the time to review them.
  8. Once this keep is reliable we can merge it and start running it in CI
  9. Update the keep to work out which feature flags are enabled on GitLab.com instead of just relying on the YAML default_enabled: true. This is necessary because most of the time people aren't even updating the YAML file once they enable it via ChatOps. This will also require us to figure out how to get this data somehow from GitLab. We can do this using the Postgres AI approach we used for ::Keeps::OverdueFinalizeBackgroundMigration as the feature flag state is in the database. But it would be nice if it was easier for people to run this without production DB access. We could maybe also get this data from prometheus which is slightly lower risk and easier to add to our CI environment. Alternatively we could extract the data from comments on the rollout issues as these are also commenting every time the feature flag is changed.
  10. Add it to CI similar to what is done in gitlab-org/quality/engineering-productivity/team!238 (diffs)

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

Related to #450442

Edited by Siddharth Dungarwal

Merge request reports

Loading