Do not use benchmark gem in production#324
Conversation
1991367 to
1098458
Compare
|
When I saw this I thought "surely this should be a development dependency, for our benchmarks, which almost no one will encounter", but we're actually using Benchmark in our Collector Middleware to measure response time... I do wonder whether we should switch to the much more common @Sinjo @Juanmcuello any thoughts? |
|
Yeah, I saw that too. I think dropping Benchmark would be better. There are some specs that depend on the lib, but I think everything could be replaced with something like this: # lib/prometheus/middleware/collector.rb
def realtime
start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)
yield
Process.clock_gettime(Process::CLOCK_MONOTONIC) - start_time
end
# spec/prometheus/middleware/collector_spec.rb
expect(app).to receive(:realtime).and_yield.and_return(0.2)I can try a PR if you think that's the way to go. |
|
But I see that even with these changes, we would still depend on Benchmark for testing, so we would have to keep it as a dev dependency anyway. |
I'd love @Sinjo 's thoughts, in case i'm missing something, but this seems better to me.
Yes, but that I don't mind. Dev dependencies are not inflicted on everyone installing your gem (like real dependencies are), so I'm not concerned about that one. |
|
Whoa, I had no idea we were using this in prod code! I'm +1 on making it a dev dependency and using |
The benchmark gem is no longer a default gem in Ruby 3.5, so we've removed its use by replacing the Benchmark.realtime method with our own implementation. We still rely on the gem in our tests, though. Signed-off-by: Juan Manuel Cuello <jcuello@fu.do>
It's no longer a default gem in Ruby 3.5, so we explicitly add it to the gemspec. Signed-off-by: Juan Manuel Cuello <jcuello@fu.do>
1098458 to
ddfae02
Compare
|
@Sinjo, great. I've updated the MR with the changes you mentioned, as well as the title. Let me know what you think. |
|
Thank you @Juanmcuello! I'll get a release out this weekend. |
The benchmark gem is no longer a default gem in Ruby 3.5.
Add benchmark to the gemspec, for forward compatibility with Ruby 3.5 and to resolve the `benchmark was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.' warning that Ruby 3.4 emits on loading client_ruby.
https://docs.ruby-lang.org/en/master/NEWS_md.html#label-Stdlib+updates