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

Log Level #2

Closed
KarelCemus opened this issue Feb 1, 2017 · 7 comments
Closed

Log Level #2

KarelCemus opened this issue Feb 1, 2017 · 7 comments
Assignees

Comments

@KarelCemus
Copy link

Hi,

you've made nice library! Great job!

I propose to reduce log level in LeafBuilder and PageBuilder to debug or even trace. Personally, I think info level should be used for more serious messages.

@splink
Copy link
Owner

splink commented Feb 1, 2017

Hey Karel,
thanks for your praise :)

You can silence the logs via logback.conf. See Line 29 in https://github.com/splink/pagelets-seed/blob/master/conf/logback.xml If you set the level to OFF or WARN, the logs will disappear.

I think INFO is an appropriate level for this kind of information. I think it is quite helpful to see how the individual pagelets (in a production system) perform. You can also easily aggregate the values and plot enlightening graphs.

Cheers,
Max

@KarelCemus
Copy link
Author

KarelCemus commented Feb 2, 2017

Sure I can rise the level in logback, I already did. I just meant to let you know my opinion. IMO, INFO level should be somehow significant for the application to inform you about what happens in there as it is highest non-error log level. For example some business flow or possibly about connecting to/disconnecting from some services etc. Flooding logs with performance tracing messages does not fit into my view of INFO. However, I fully agree with you that easily tracing performance might be helpful even in production, but as it is tracing performance, I don't think the INFO level is appropriate. Finally, when you can suppress logging INFO, you can as well enable logging DEBUG/TRACE in production when it is necessary.

By the way, first I saw those INFO logs it came to me as another idea. I though that you are logging them as INFO in development mode but I hopped I will reduce the level for production mode. This might be also interesting feature.

Either way, If you disagree, you can freely close this issue.

Best, Karel

@splink
Copy link
Owner

splink commented Feb 2, 2017

I'll think about it. Somehow it doesn't feel semantically right to call that kind of log "debug". I also want to keep the option to add some log statements which fit into the category debug (currently there are none in pagelets).
So if I would change the performance output logs to be at level debug or even trace, other debug/trace level logs which I don't want to log in production would also be logged. But I usually want to see these performance measuring logs in production.

@KarelCemus
Copy link
Author

I see. Then what about another performance logger and log page performance as debug and leaf performance as trace? You'd have a full control and still would not produce performance tracing at info level.

@ghost
Copy link

ghost commented Feb 3, 2017

I don't really understand the problem. In case of the page builder and the leaf builder there are two loggers .. (val log = play.api.Logger("LeafBuilder").logger) - so it should be possible for you to set the logging level for "LeafBuilder" to any level you want...

@KarelCemus
Copy link
Author

KarelCemus commented Feb 3, 2017

@fbe Are you reacting at me or at @splink?

My proposal is just to reduce info level of those messages because IMO performance tracing messages should not be logged with this level. I do not object anything against these loggers at all and I am aware of custom settings of those loggers. This is just about semantics.

@splink
Copy link
Owner

splink commented Feb 5, 2017

@KarelCemus After some consideration, I have to say that I don't like the idea of two loggers and I also find that "INFO" is the semantically appropriate level for these kind of logs. I know that you don't agree, but you have fine-grained control over the logs though your logback configuration, so my views shouldn't bother you too much ;)

@splink splink self-assigned this Feb 5, 2017
@splink splink closed this as completed Feb 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants