Skip to content
This repository was archived by the owner on Apr 4, 2024. It is now read-only.

Set Logger level using BORS_LOG_LEVEL if provided#1602

Merged
bors[bot] merged 4 commits into
bors-ng:masterfrom
NoRedInk:set-log-level-via-env-var
Jan 13, 2023
Merged

Set Logger level using BORS_LOG_LEVEL if provided#1602
bors[bot] merged 4 commits into
bors-ng:masterfrom
NoRedInk:set-log-level-via-env-var

Conversation

@jali-clarke

Copy link
Copy Markdown
Contributor

Sometimes when the Github API is on the fritz it can be useful to have debug-level logging. Setting the log level in config.exs and rebuilding the container image is not ideal - having something more dynamic would be very nice to have.

This PR introduces the BORS_LOG_LEVEL environment variable which is converted to a Logger level atom using String.to_existing_atom when provided. If BORS_LOG_LEVEL is unset, the Logger level will remain unchanged and is inherited from config.exs (i.e. :info for prod release).

@notriddle

Copy link
Copy Markdown
Member

Have you tested this?

I was under the impression that the Elixir logging package used macros to purge logging calls from the code entirely when the log level is lower than the one given.

@jali-clarke

jali-clarke commented Jan 13, 2023

Copy link
Copy Markdown
Contributor Author

@notriddle We have tested this and it does work - we can even get a console into the running process using bors rpc and set the log level there using Logger.configure(level: :debug). We are consequently able to see debug log messages being emitted.

That purging only happens if :compile_time_purge_matching is used, and it's not being used. I'll add a comment to that effect in the code.

@notriddle

Copy link
Copy Markdown
Member

Okay, makes sense.

bors r+

bors Bot added a commit that referenced this pull request Jan 13, 2023
1602: Set `Logger` level using `BORS_LOG_LEVEL` if provided r=notriddle a=jali-clarke

Sometimes when the Github API is on the fritz it can be useful to have debug-level logging.  Setting the log level in `config.exs` and rebuilding the container image is not ideal - having something more dynamic would be very nice to have.

This PR introduces the `BORS_LOG_LEVEL` environment variable which is converted to a `Logger` level atom using `String.to_existing_atom` when provided.  If `BORS_LOG_LEVEL` is unset, the `Logger` level will remain unchanged and is inherited from `config.exs` (i.e. `:info` for prod release).

Co-authored-by: Jinnah Ali-Clarke <jinnah@noredink.com>
@bors

bors Bot commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

Build failed:

@jali-clarke

Copy link
Copy Markdown
Contributor Author

Fixing formatting issues...

@jali-clarke
jali-clarke force-pushed the set-log-level-via-env-var branch from 8a516f2 to 1ded233 Compare January 13, 2023 20:48
@jali-clarke

Copy link
Copy Markdown
Contributor Author

@notriddle Amended and force-pushed.

@notriddle

Copy link
Copy Markdown
Member

bors r+

@bors

bors Bot commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

Build succeeded:

@bors
bors Bot merged commit ce24030 into bors-ng:master Jan 13, 2023
@jali-clarke
jali-clarke deleted the set-log-level-via-env-var branch January 13, 2023 21:05
notriddle added a commit to bors-ng/bors-ng.github.io that referenced this pull request Feb 1, 2023
Add @jali-clarke as new contributor
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants