-
Notifications
You must be signed in to change notification settings - Fork 4
Delegate save to saviour proc #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Conflicts: lib/stackprof/middleware.rb test/test_middleware.rb
Signed-off-by: Simon Eskildsen <sirup@sirupsen.com>
middleware: allow raw dumps
ensure raw is present in joined output
bump flamegraph script
Bump to 0.2.9
| filename ||= "stackprof-#{results[:mode]}-#{Process.pid}-#{Time.now.to_i}.dump" | ||
| File.open(File.join(Middleware.path, filename), 'wb') do |f| | ||
| f.write Marshal.dump(results) | ||
| if saviour.respond_to?(:call) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should just assume saviour is callable if non-nil, so if saviour here.
|
👍 for the name saviour, for starters. |
test/test_middleware.rb
Outdated
| proc_called = false | ||
| env_set = nil | ||
| results_received = nil | ||
| middleware = StackProf::Middleware.new(proc {|env| proc_called = true}, enabled: Proc.new{ [true, 'foo'] }, saviour: Proc.new{ |env, results| env_set = env['FOO'] ; results_received = results[:mode] }, save_every: 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OMG - can we split this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When can we call proc {|arg| ... } vs. Proc.new {|arg| ...}?
| proc_called = false | ||
| env_set = nil | ||
| results_received = nil | ||
| enable_proc = Proc.new{ [true, 'foo'] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enable_proc = proc{ [true, 'foo'] }?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize I want to just replace all Proc.new calls with proc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or better: use lambda
This adds a
Middleware.saviourhook to allow overriding the save mechanism. In our case, we want to push theStackProf.resultsto Kafka.r: @shivnagarajan