Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Feature support set level of logger#777

Open
zhuliquan wants to merge 3 commits intoArroyoSystems:masterfrom
zhuliquan:feature-min_log_level
Open

Feature support set level of logger#777
zhuliquan wants to merge 3 commits intoArroyoSystems:masterfrom
zhuliquan:feature-min_log_level

Conversation

Copy link
Contributor

zhuliquan commented Nov 1, 2024

Hello, Every time I want to debug problem and view the logs, I always see a large number of INFO logs, sometimes I only need to look at the warn and error level logs, meanwhile printing log in terminal frequently will affect the performance. If we can control the log level through the config file will be better, so that we can debug problem in the development stage, and ensure the performance in the production stage.

zhuliquan force-pushed the feature-min_log_level branch from 5c0911f to ce14784 Compare November 4, 2024 03:56
zhuliquan force-pushed the feature-min_log_level branch from ce14784 to 7ab913a Compare November 5, 2024 03:06
mwylde requested changes Dec 4, 2024
}

fn parse_level_filter() -> LevelFilter {
let level_filter = config()
Copy link
Member

mwylde Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want the RUST_LOG env var to have the highest precedence, so that they can override what's the in the config file as needed.

With this change, it's not possible to use RUST_LOG at all because min_level is set in defaults.toml, and always will override the env var.

Additionally, the env var supports much more than just static levels (like "info" or "warn")--it's able to completely configure the logger on a per-crate basis. For example:

RUST_LOG=info,arroyo_worker=debug,arroyo_controller=debug,arroyo_state=debug

I think an approach would be to:

  • First check if the env var is set, and if so use that with from_env_lossy; if not we can use the min_level config.

Copy link
Contributor Author

zhuliquan Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank for @mwylde review, Sorry for later reply. I agree with Env var high priority, but I don't know how to implement min log level for each sub-crate.

Copy link
Contributor Author

zhuliquan Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want the RUST_LOG env var to have the highest precedence, so that they can override what's the in the config file as needed.

With this change, it's not possible to use RUST_LOG at all because min_level is set in defaults.toml, and always will override the env var.

Additionally, the env var supports much more than just static levels (like "info" or "warn")--it's able to completely configure the logger on a per-crate basis. For example:

RUST_LOG=info,arroyo_worker=debug,arroyo_controller=debug,arroyo_state=debug

I think an approach would be to:

  • First check if the env var is set, and if so use that with from_env_lossy; if not we can use the min_level config.

parse level like this? first fetch ENV var then fetch min_level of logging config

let level_filter = std::env::var(EnvFilter::DEFAULT_ENV).unwrap_or(
config()
.logging
.min_level
.clone()
.unwrap_or("info".to_string()),
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mwylde mwylde requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants