Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add caller_depth arg #461

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jwittkoski
Copy link

Adds an optional argument caller_depth to the psgix.logger anonymous sub so that callers can add additional layers of caller_depth to Log4perl output that is using Plack::Middleware::Log4perl PatternLayout config values which include the file or line number (like %F, %l and %L).

Without the ability to have the caller additionally adjust the caller depth, a psgix.logger using Log4perl always shows the immediate caller and not (necessarily) the "original" caller.

For example, when using Dancer::Logger::PSGI, the Dancer logging keywords have three additional calling layers so the %F, %l and %L placeholders point to Dancer/Logger/PSGI.pm instead of the "original" caller.

Originally I addressed this problem by using local and adjusting $Log::Log4perl::caller_depth in Dancer::Logger::PSGI which works. However, because psgix.logger is an anonymous sub, the caller to psgix.logger (Dancer::Logger::PSGI in this case) doesn't doesn't know whether the currently configured Plack logger is Log4perl or something else (like Log::Dispatch) and therefore whether $Log::Log4perl::caller_depth is even available.

Of course, to get this all to work Dancer::Logger::PSGI would also have to be modified to set the caller_depth argument when calling psgix.logger.

Perhaps there is a cleaner way to address this problem? Dancer::Logger::PSGI could just check whether Logger::Log4perl is loaded and set $Log::Log4perl::caller_depth only in that case.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 6d5b781 on jwittkoski:caller_depth_adjust into cf3e3db on plack:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling da59a75 on jwittkoski:caller_depth_adjust into 2dcdf47 on plack:master.

@jwittkoski
Copy link
Author

I also looked at using Log4perl's wrapper_register but similar to using Log::Log4perl::caller_depth it requires the "downstream" caller to determine whether Log::Log4perl is in use which might be problematic.

Adding this optional caller_depth argument to Plack::Middleware::Log4perl allows "downstream" callers to specify the additional caller depth without knowing or caring which middleware logging module is currently in use. If the current middleware doesn't use or need it it will be silently ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants