aserafin / grape_logging

Request logging for Grape!
MIT License
147 stars 76 forks source link

Pluggable loggers #9

Closed 987poiuytrewq closed 8 years ago

987poiuytrewq commented 9 years ago

Hi, great work on creating this middleware, its a solid implementation of a generic logger for Grape.

After wanting some extra functionality in what you can log, I ended up refactoring the code to allow you to pick and choose what things you want to log. I also ended up moving the filtering work by @diguliu into an opt-in logging module while I was at it.

Please do let me know if you think something is done strangely, I'd like to get this into master and I'm happy to make any changes.

dmitry commented 9 years ago

Really great! :+1:

fidelisrafael commented 9 years ago

Very nice! :+1:

dmitry commented 9 years ago

@aserafin can it be merged?

aserafin commented 9 years ago

thanks @987poiuytrewq for the awesome pull request. I've found few issues though:

987poiuytrewq commented 9 years ago

Good catch - I've decided to with Loggers for the module name as it more consistent with Formatters.

I'm not sure why the body proxy isn't being turned into a string for you, as it seems to work for me. Looking at the source, https://github.com/rack/rack/blob/master/lib/rack/body_proxy.rb#L41 it seems to delegate all methods to the actual body so I've put in a to_s that should work.

guizmaii commented 8 years ago

Great work :+1:

dmitry commented 8 years ago

@aserafin hopefully this one could be merged soon.

987poiuytrewq commented 8 years ago

Hi @aserafin is there anything I can do to get this merged in?

987poiuytrewq commented 8 years ago

@aserafin rebased onto master

rngtng commented 8 years ago

can you pls resolve the conflicts so this can be merged?