Issue/62 custom access and error log handlers (against 0.9-stable) #71
Conversation
This is my first attempt at custom access and error log handlers.
|
If anybody was around to give some quick feedback that would be great. :) I'm halfway through having a branch ready (based on this one) that could be merged into master. Therefore if would be good to get a nod of approval on the general approach just to make sure I'm not spinning my wheels. |
|
And I have just done the work required to get this merged against master too: #72 |
robertmassaioli
changed the title
|
@gregorycollins As we talked about, I think that this PR is done (against 0.9-stable) the (1.0 release is still up for discussion). Therefore could we merge this in and then I'll do the other steps that you have asked me to do? |
As requested.
|
|
||
| ------------------------------------------------------------------------------ | ||
| debugE :: (MonadIO m) => ByteString -> m () | ||
| debugE s = debug $ "Server: " ++ (map w2c $ S.unpack s) | ||
|
|
||
| ------------------------------------------------------------------------------ | ||
| defaultAccessLogHandler :: AccessLogHandler |
There was a problem hiding this comment.
Export this one too, in case users want to mix and match?
There was a problem hiding this comment.
I don't understand, if you simply don't set anything in the config instance then won't this be the default. Do we need to export it if it is the result of the customer doing nothing? I thought that was the beauty of Config being an instance of Monoid.
That said I just wanted to be really sure that was what you wanted me to do and add it to the public api. I'm more than happy to do it if you say yes again. 😄
There was a problem hiding this comment.
Yes I meant that it should be exported from the module.
|
Sorry I meant 0.10! Meant to type .10 but fat fingered it. |
|
Oh, sweet. 😃 That makes much more sense! I'll get it done. |
|
Okay, version bumped @gregorycollins and I think that this can be merged whenever you are ready. I'm going to update the snap-website code now. |
|
Linking this here: snapframework/snap-website#40 Okay:
I think that we are good to merge @gregorycollins so just let me know if there is anything more that I can do. Otherwise I'm excited to see this happen! |
|
Hi @gregorycollins, we never merged in this pull request at the time and it seemed to be a bad time to try and get this work in. However, I am starting to see a need for this functionality again and I would love to be able to merge it back in. Do you remember what stopped us from merging this in all of those months ago? Either way I would like to resolve those issues, build it properly, and get it released. Please let me know what I can do to help get my PR into that state. Cheers! |
|
Hi all! I recently needed log customization myself, and did so by just modifying my own copy of the code. This PR seems a pretty good general solution, well-designed and without introducing any compatibility issues. Is there a reason it's been languishing and can't just be merged? |
Note: This is the first of two pull requests. This is the first one and aims to backport my changes to 0.9-stable. The second one will port these changes to master.
I am trying to solve #62 in this pull request. It is my aim to be able to push out the access and error logs in a custom format: in my case, JSON. You can see an example of me testing this branch and doing that here:
You can see an example of me using this branch in the My Reminders code here: https://bitbucket.org/atlassianlabs/my-reminders/branch/issue/MR-7-json-logs-for-my-reminders#diff
I hope that you like these changes and are willing to accept them. I have spoken to @gregorycollins and he suggested that such a feature would be desirable. I am also open to suggestions on how to improve this PR but I can tell you that it works (via my manual testing) and lightly extends existing behaviour in its current form.
P.S. If you want to know why I want to output custom log line formats into JSON then the answer is simple: it is because that is a really nice way to pass the data into Logstash / Kibana so that I can easily monitor the services that I have written using the Snap framework. There are likely many many reasons too that various snap framework users would like to customise the log lines coming from their apps.