Add JSON formatter + Fix response.body format to follow the request.params format + Add an option to use Rails instrumentation instead of a logger + AS:N thread safety + Update to Grape > v0.14.0 (after callback is always called)

aserafin / grape_logging

Request logging for Grape!

MIT License

147 stars 76 forks source link

Add JSON formatter + Fix response.body format to follow the request.params format + Add an option to use Rails instrumentation instead of a logger + AS:N thread safety + Update to Grape > v0.14.0 (after callback is always called) #16

Closed guizmaii closed 8 years ago

guizmaii commented 8 years ago

To merge after the #9 PR (or close #9 and merge only mine) :

Add a JSON formatter
Fix response.body format to follow the request.params format => it was a string but the params are logged as a serialized Hash (~JSON)
Add an option to use Rails instrumentation instead of a logger => Mostly because the default Rails 3 formatter doesn't have a formatter= method and also because I use the Logging gem (https://github.com/TwP/logging) as logger. This gem is already in charge of formatting data and it does not have a formatter= method neither.
Add the missing patch of @marshall-lee given in the PR #7
Add the thread-safety given by @johnthethird in #18
Update to Grape > v0.14.0 (after callback is always called, so the response could be nil)

guizmaii commented 8 years ago

The instrumentation idea come from here : https://gist.github.com/teamon/e8ae16ffb0cb447e5b49

guizmaii commented 8 years ago

Just added the #18 code

rngtng commented 8 years ago

nice, could you pls squash you commits pls? :+1:

guizmaii commented 8 years ago

Hum why ?

rngtng commented 8 years ago

Well I find it a good practice to enable code review, cherry picking and reverting when a commit encapsulates a feature/bug/refactor in full. Currently its in 41 changes, no test and contains even others changes, which is maintainers hell... If we want to @aserafin keep on doing he good job, we should make his live a simple as possible. So let's keeps changes small & simple without any side effects (well and we need specs, but that's a different story ;)

guizmaii commented 8 years ago

I'm not sure to understand your why.

My PR is mostly the merge of some others PR like #7 (which is now merged), #9, #14, #15 and #18 with some more work to add the optionnal use of Rails instrumentation and add the compatibility with the next Grape version. Nothing more. Nothing complicated. Furthermore, the diff is pretty small and simple. I personnaly don't care of the git history, it's only esthetic. If you think that I should change/improve something in my code I'm listening to you but I don't understand why a more beautiful git history will help.

And yes we need specs but for now I does not have the time to write them.

;)

rngtng commented 8 years ago

ok ;)