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

Allow user to override log level #2489

Closed
wants to merge 1 commit into from

Conversation

FreedomBen
Copy link
Contributor

This PR adds an environment variable LOG_LEVEL that can be used to control the application log level. For example, this would allow you to view dev logs while in production, should you so choose.

If the env var is not set, behavior should not change from the current behavior.

Env var can be set with the log level string (case insensitive). For example: LOG_LEVEL=debug

This PR adds an environment variable `LOG_LEVEL` that can be used to
control the application log level.  For example, this would allow you to
view dev logs while in production, should you so choose.

If the env var is not set, behavior should not change from the current
behavior.

Env var can be set with the log level string (case insensitive).  For
example:  `LOG_LEVEL=debug`
@FreedomBen
Copy link
Contributor Author

If this is accepted, I'm happy to add it to the documentation

@advplyr
Copy link
Owner

advplyr commented Jan 3, 2024

This was added a few months ago but never put in the docs #2326

Since dev is the only log level used that isn't accessible in the dropdown of the log page

@FreedomBen
Copy link
Contributor Author

Thanks, looking into this deeper I've come to the following proposal. I'm willing to do the work if this direction is agreeable, but I don't want to do it unnecesarily

  1. The "dev" log seems to bypass the standard logging framework. There are quite a few log lines using the structure, and a small handful using "dev"
  2. Most of the dev logs look like they would appropriately be debug lines, although some would be better for trace
  3. Having logging that goes outside the logging framework feels overly complex and unexpected
  4. The original intent looked to be that these would show up in trace mode (see https://github.com/advplyr/audiobookshelf/blob/master/server/Logger.js#L7C34-L7C34 )

My proposal is to remove the "dev" logging code to simplify the logging framework, and update the current invocations of dev log to use debug or trace instead.

What are your thoughts?

@advplyr
Copy link
Owner

advplyr commented Jan 4, 2024

That makes sense to me. I originally added the dev logs just because I was doing a lot of testing after adding sqlite and never intended for them to be permanent. They are still useful for me in testing. I did a quick skim through and it looks like they could all be replaced with Logger.debug and the entire dev thing gets dropped.

@FreedomBen
Copy link
Contributor Author

Cool, which close this and send a new PR

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