flippercloud / flipper

🐬 Beautiful, performant feature flags for Ruby.
https://www.flippercloud.io/docs
MIT License
3.71k stars 417 forks source link

Missing RedisCache Adapter #323

Closed kidsalsa closed 6 years ago

kidsalsa commented 6 years ago

I'm not sure what I'm doing wrong, I haven't run into this sort of thing before. When I tried to use the RedisCache adapter for optimization the require 'flipper/adapters/redis_cache' failed, the find the file couldn't be found. When I inspected the contents of the gem, indeed the redis_cache.rb file was not there, in fact there were quite a few adapters missing when comparing to the code with github.

My next thought was that redis_cache.rb must be a new adapter that hasn't been released yet but when I checked the git history on that file it was last modified on Oct 31, 2017 and the version.rb file was last updated on November 6, 2017. So I'm perplexed why I'm not getting the file when I install the gem.

What am I missing?

AlexWheeler commented 6 years ago

hey @kidsalsa although RedisCache is documented it hasn't been published to rubygems yet. That's on us! I don't see any reason why it shouldn't be released I think it just got overlooked. @jnunemaker

As for why the file isn't included in the installed Flipper gem its because Flipper uses a monorepo and actually distributes many of the adapters as separate gems. Let me know if this post I wrote helps out because you're definitely not the first person to ask this!

AlexWheeler commented 6 years ago

@kidsalsa Ah looking again this am with my ☕️ I've actually noticed the real problem!

flipper.gemspec ignores any adapters files that are distributed as their own gem. The flipper-redis.gemspec is matching on /redis/ so redis_cache.rb is not being downloaded.

In the meantime you can get this working if you install flipper-redis by adding it to your gemfile:

gem flipper-redis bundle install

Then you'll be able to require 'flipper/adapters/redis_cacheand can confirm that the file is there by looking inbundle show flipper-redis`

I will look into a longer-term solution so that users don't have to install both gems. Thanks for opening a great issue

kidsalsa commented 6 years ago

Hey thanks for the help @AlexWheeler and thank you for your and @jnunemaker's awesome work to create the flipper ecosystem.

I tried installing the flipper-redis gem but it didn't work. When I inspected the contents of the gem it did not have the redis_cache.rb adapter, only redis.rb. This is weird because when I run the code to generate the files for the flipper-redis gem it produces the list of files that you would expect:

["docs/redis/README.md", "examples/redis/basic.rb", "examples/redis/internals.rb", "examples/redis/namespaced.rb", "flipper-redis.gemspec", "lib/flipper-redis.rb", "lib/flipper/adapters/redis.rb", "lib/flipper/adapters/redis_cache.rb", "spec/flipper/adapters/redis_cache_spec.rb", "spec/flipper/adapters/redis_spec.rb", "test/adapters/redis_cache_test.rb", "test/adapters/redis_test.rb", "lib/flipper/version.rb"]

But for some reason the gem itself does not include all those files...

AlexWheeler commented 6 years ago

@kidsalsa RedisCache was introduced in 0.11.0, it sounds like you might be on a previous version?

kidsalsa commented 6 years ago

That fixed it. Thanks, I meant to check that then forgot before I replied. My flipper-redis gem that got installed was 0.3.0. I had to downgrade to my redis gem to 3.3.5 from 4.0.1 because flipper-redis requires the version to be less than 4.0.0. I guess there are breaking changes in 4.0.0 for flipper-redis?

One other thing, in order for the redis_cache.rb adapter to work correctly I had to modify the following method:

      def fetch(key)
        cached = @cache.get(key)
        if cached
          Marshal.load(cached)
        else
          to_cache = yield
          set_with_ttl(key_for(key), to_cache)
          to_cache
        end
      end

I modified set_with_ttl(key_for(key), to_cache) to set_with_ttl(key, to_cache) because key_for(key) was producing "flipper/v1/feature/flipper/v1/feature/customer_portal_payments1" because the key was already set to flipper/v1/feature/customer_portal_payments1.

Without that change I was getting a cache miss with every request and it was calling the http adapter and hence making a network call to the feature toggle service with every request.

This seems like a bug. The only other thing that I could think might be a problem is that I have the client and the server both using the same redis instance (On my local box, I'm standing up a flipper service (using flipper-api) and then I'm having a different app call the service using the flipper gem with the RedisCache adapter and the http adapter). But it seems like having them both using the same redis isn't the problem because I don't seem to be having any other problems with turning features on and off and the cache expiring correctly for the client.

jnunemaker commented 6 years ago

@kidsalsa you da best. That is totally a bug. Thanks for reporting. I'm fixing right now.

kidsalsa commented 6 years ago

Awesome thanks @jnunemaker!

jnunemaker commented 6 years ago

This should be all fixed up and I release 0.12. Additionally, https://github.com/jnunemaker/flipper/pull/317 was in 0.12 which relaxes the redis dependency version.

On my local box, I'm standing up a flipper service (using flipper-api) and then I'm having a different app call the service using the flipper gem with the RedisCache adapter and the http adapter

@kidsalsa neat! This type of usage is exactly what I'm building flippercloud.io for (docs). Let me know if you want to kick the tires and avoid setting up and managing a service on your own.

kidsalsa commented 6 years ago

Everything is working great @jnunemaker thanks a ton for releasing those fixes! I didn't know you guys were going to be doing a cloud based offering for this. We looked a couple of other outsourced solutions, and the cost seemed prohibitive. I think we are going to go with self-hosted for now, but we'll keep your upcoming service on the radar.

jnunemaker commented 6 years ago

Everything is working great @jnunemaker thanks a ton for releasing those fixes!

Thank you for reporting them! Super helpful.

I didn't know you guys were going to be doing a cloud based offering for this.

I didn't either! Haha. It just hit me like a year ago that it might be a good idea. You can read more here if you are curious: https://johnnunemaker.com/featureflipper-com-why/.

We looked a couple of other outsourced solutions, and the cost seemed prohibitive.

I'd love to know more about what you looked at, what cost range you would have been in, and what cost range would have been "ok" to you. If you aren't comfortable posting that here, I'd love an email to john@fewerandfaster.com.

I think we are going to go with self-hosted for now, but we'll keep your upcoming service on the radar.

Sounds good. Let me know if you have any problems with self-hosting. Happy to work through stuff with you.

kidsalsa commented 6 years ago

I read your why, cool story. The other solution I looked at was LaunchDarkly. Their solution gets really expensive really fast. The only solution that they offer that would work for us is the $700 solution. That just seems way too much for feature flipping. Maybe we are being cheap. But this isn't the only cost and they all add up, the biggest being the huge AWS bill every month. But in the end, maybe that is cheaper than having a developer maintaining an in house solution...This got me thinking and I talked to our VP of engineering about it. He thinks it is expensive if it is over say $200 a month. If it were a mature service and it was $200 a month, that seems like he would consider it. But he had concerns about ensuring the access (security), the reliability, and the support. We have about 30 services in our stack right now just to give you a feel for our size. I'm not sure it is actually going to be cheaper to maintain our own feature flipping service in the end, but it sounds like, for the time being, that is what we are doing. I know you didn't ask for all those details, but you made me start thinking about this and I know how valuable feedback like this is when you are starting up a new venture.

jnunemaker commented 6 years ago

@kidsalsa you rock! Hard for me to say what is cheap or expensive since I don't know much about your setup (active users, etc.), but it was great hearing all of that. Definitely drop your email on https://flippercloud.io/ or shoot me an email if you want me to check in sometime in the future. Or just track me on twitter, I'm sure I'll keep blabbing there about it. 😉

kidsalsa commented 6 years ago

Great, glad the information helped! (I subscribed to to the featureflipper.com list ;)