Introduces basic logging back

[purbon]

Introduce basic logging, including basic access logging. This PR introduces basic access and error logging, booths serve the purpose of knowing:

• When something goes wrong, why it's going wrong (aka exception and error handling).
• Know usage of resources, including status code monitoring.

Fixes #4575

[purbon]

@ph your thoughts are more than welcome as always 😸

[jordansissel]

Feels odd doing it this way. Can we add a mixin or extension to Cabin?

[purbon]

We could, but does it makes sense to change the way cabin works because of Sinatra?

[ph]

What @jordansissel is suggesting is to create a module adding the compatibility layer required by Sinatra instead of using class_eval.

Something like

::Cabin::Channel.extend(Cabin::SinatraLoggerCompatibility)

The module can live inside logstash in the util directory, this is a bit cleaner and we don't need to push this change upstream.

[purbon]

Oh! got it, will do it that way then. I agree is a clear solution

[ph]

I have been trying to test this PR, but I can seen to get the webserver started? It should answer on the default sinatra port of 4567 correct?

[purbon]

@ph sorry to tell you there is no default sinatra port per se, this is answering now to the default rackup port (aka 9292) .... we should definitely decide to which port do we want to make this listen to? do we have an issue for this?

[ph]

You don't need to do class_eval in that case.

You can do something like this

jruby-1.7.23 :001 > class Hello;end
 => nil
jruby-1.7.23 :002 > module Ola; def write;puts "ninja";end;end
 => nil
jruby-1.7.23 :003 > Hello.send(:include, Ola)
 => Hello
jruby-1.7.23 :004 > Ola.write
jruby-1.7.23 :006 > Hello.new.write
ninja
 => nil

Or we can do this if you require the exiting file before, the new functionality will be mixed in.

# in the helper
require "cabin/namespace"
class Cabin::Channel 
 def write(data, &block)
   self.publish(data, &block)
 end
end

Not sure if we have a preferred way

[purbon]

Hi, I see this two methods as being very similar to what I did, isn't? what are the pros and cons of both for you? by using class_eval and include it make the definition very explicit, something less obscure than your option 1.

I do really see option 2 as doing the same as with the class eval, only thing is there is no module that we're actually including, just adding a new method and per se not using the same mechanism as other cabin extensions.

[ph]

OK, valid point, I think its okay

[purbon]

@ph @jsvd can you folks validate this PR so we can move forward? looks not that hard to actually review, isn't?

[ph]

@purbon once its rebased with the new branch I will test it. Thanks.

[jsvd]

same, code looks ok, will test manually after rebase

[purbon]

Closing this in benefit of #4635 that points to the new_metrics (rebuild branch with up to date master) branch.