Some questions about Logger and format_message in Dancer2
Hi, I'm using Dancer2 in a RESTful JSON API. Works like a charm. Made everything so much easier! :) Now I needed to implement some logging, and I noticed that the documentation describes some placeholders that haven't been implemented yet, like %h (host emitting the request) and %{header}h (value of a request header). I really needed those two so I cloned the repo and implemented these myself, which was quite easy. I'll push the changes and create a pull request. But I have some questions: - According to the docs, the (not yet implemented) placeholder %i should give the "request id". I'm not sure what that should be? Some kind of session id or something? - lib/Dancer2/Core/Role/Logger.pm:74 : $level = sprintf( '%5s', $level ); Here $level is padded with spaces if it's shorter than 5 chars. So "info" gets an extra space. I'm not sure about the rationale, as "warning" has 7 chars so this line seems a bit pointless. (And the extra space before "info" makes my OCD play up ;)) - lib/Dancer2/Core/Role/Logger.pm:83 : return "[" . strftime( $block, localtime(time) ) . "]"; Why is the custom formatted time/date enclosed in brackets? Wouldn't it be better to do that in the log_format config directive if you wanted to? Obviously I'm hesitant to change the behaviour of these last two things as that may break stuff (like log parsers)... Thanks, Lennart
On Thu, Nov 6, 2014 at 2:44 PM, Lennart Hengstmengel <lennart@tevreden.nl> wrote:
Hi,
Hey.
I'm using Dancer2 in a RESTful JSON API. Works like a charm. Made everything so much easier! :)
Happy to hear! :)
Now I needed to implement some logging, and I noticed that the documentation describes some placeholders that haven't been implemented yet, like %h (host emitting the request) and %{header}h (value of a request header).
I really needed those two so I cloned the repo and implemented these myself, which was quite easy. I'll push the changes and create a pull request.
First let me thank you for putting the time on this. Well done!
But I have some questions:
- According to the docs, the (not yet implemented) placeholder %i should give the "request id". I'm not sure what that should be? Some kind of session id or something?
The Request ID (will be available as "id" method in the next release) is documented as: The ID of the request. This allows you to trace a specific request in loggers, per the string created using "to_string". The ID of the request is essentially the number of requests run in the current class.
- lib/Dancer2/Core/Role/Logger.pm:74 : $level = sprintf( '%5s', $level ); Here $level is padded with spaces if it's shorter than 5 chars. So "info" gets an extra space. I'm not sure about the rationale, as "warning" has 7 chars so this line seems a bit pointless. (And the extra space before "info" makes my OCD play up ;))
OCD++ Go for it!
- lib/Dancer2/Core/Role/Logger.pm:83 : return "[" . strftime( $block, localtime(time) ) . "]"; Why is the custom formatted time/date enclosed in brackets? Wouldn't it be better to do that in the log_format config directive if you wanted to?
Makes sense, yup.
Obviously I'm hesitant to change the behaviour of these last two things as that may break stuff (like log parsers)...
I'm not aware of any log parsers for the Dancer2 logs. It's also not an official format (unlike the common log format in Apache), so I wouldn't worry. Submit the PR and let's check it out. All the best, Sawyer.
Hi Sawyer, On 08-11-14 12:15, Sawyer X wrote:
Submit the PR and let's check it out.
Thanks for the useful feedback! I submitted a PR at https://github.com/PerlDancer/Dancer2/pull/760 There's one thing I encountered and I'm not sure whether I did this right. In the code there was: setting('charset'), which triggered an error (undefined function). So I changed it to $self->config->{'charset'} however $self->config appears to be an empty hashref. I expected it to contain the app config (with at least a key 'charset' as that is in the config.yml). Is this assumption correct? If not, what is the best way to get access to the app config / charset setting? Best regards, Lennart
On Mon, Nov 10, 2014 at 12:46 PM, Lennart Hengstmengel <lennart@tevreden.nl> wrote:
Hi Sawyer,
Hey. :)
On 08-11-14 12:15, Sawyer X wrote:
Submit the PR and let's check it out.
Thanks for the useful feedback! I submitted a PR at https://github.com/PerlDancer/Dancer2/pull/760
Wonderful! I've seen it but haven't had time to review it. I hope to do so today.
There's one thing I encountered and I'm not sure whether I did this right. In the code there was: setting('charset'), which triggered an error (undefined function). So I changed it to $self->config->{'charset'} however $self->config appears to be an empty hashref. I expected it to contain the app config (with at least a key 'charset' as that is in the config.yml). Is this assumption correct? If not, what is the best way to get access to the app config / charset setting?
Since the internals are... well, internals, they should not be calling the DSL. DSL is for end-users. I'll take a look at why it's missing. Perhaps it's not available yet. I'll reply on the issue! Thanks again for the pull request! Sawyer.
participants (2)
-
Lennart Hengstmengel -
Sawyer X