Add specs to catch recursive feature-flag requests
What does this MR do and why?
This MR adds a spec to run Feature.enabled?
through Rails.cache
by adding use_clean_rails_redis_caching
to the example. This catches usage of feature flags in any Redis instrumentation middleware or Gitlab::Redis::Cache/ActiveSupport::Cache::RedisStoreCache
patches.
See #385237 (closed) for more background
Two complications when testing out the efficacy of this spec:
- The line of code preventing it from being captured in
gdk rails c
has been fixed in MR. - The code causing the recursion has been reverted
This new spec was tested on a commit between its introduction and removal: https://gitlab.com/gitlab-org/gitlab/-/commits/sc1-investigate. The diff is fairly small, containing 1 line of code change and 5 lines of spec changes (same as this MR).
To test it, checkout to git checkout sc1-investigate
and run the spec.
➜ gitlab git:(sc1-investigate) ✗ CI=true bundle exec rspec spec/lib/feature_spec.rb:165 -r spec_helper
/Users/sylvesterchin/work/gitlab-development-kit/gitlab/rspec/flaky/suite-report.json doesn't exist
Run options: include {:locations=>{"./spec/lib/feature_spec.rb"=>[165]}}
==> Starting GitLab Elasticsearch Indexer set up...
==> Starting Gitaly set up...
Test environment set up in 1.476678 seconds
1st Try error in ./spec/lib/feature_spec.rb:166:
(Feature).enabled?(:validate_allowed_cross_slot_commands, {:type=>:development})
expected: 1 time with any arguments
received: 2 times with arguments: (:validate_allowed_cross_slot_commands, {:type=>:development})
RSpec::Retry: 2nd try ./spec/lib/feature_spec.rb:166
F
Failures:
1) Feature.enabled? when using redis cache does not make recursive feature-flag calls
Failure/Error: Feature.enabled?(:validate_allowed_cross_slot_commands, type: :development)
(Feature).enabled?(:validate_allowed_cross_slot_commands, {:type=>:development})
expected: 1 time with any arguments
received: 2 times with arguments: (:validate_allowed_cross_slot_commands, {:type=>:development})
# ./lib/gitlab/instrumentation/redis_base.rb:82:in `block in validate_allowed_commands?'
# ./lib/gitlab/null_request_store.rb:34:in `fetch'
# ./lib/gitlab/safe_request_store.rb:12:in `fetch'
# ./lib/gitlab/instrumentation/redis_base.rb:81:in `validate_allowed_commands?'
....
Finished in 3.06 seconds (files took 1 minute 0.4 seconds to load)
1 example, 1 failure
Failed examples:
rspec ./spec/lib/feature_spec.rb:166 # Feature.enabled? when using redis cache does not make recursive feature-flag calls
[TEST PROF INFO] Time spent in factories: 00:00.010 (0.03% of total time)
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.