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

Added apply_logger_env_levels #2649

Merged
merged 1 commit into from
Feb 25, 2023

Conversation

afshinpir
Copy link
Contributor

@afshinpir afshinpir commented Feb 23, 2023

This method applies levels which is set by environment variable SPDLOG_LEVEL to the a single logger. Useful for loading configuration into manually created loggers.

@tt4g
Copy link
Contributor

tt4g commented Feb 23, 2023

Fix #2648

include/spdlog/spdlog.h Outdated Show resolved Hide resolved
This method applies levels which is set by environment variable
`SPDLOG_LEVEL` to the a single controller. Usefull for loading
configuration into manually created loggers.
@afshinpir afshinpir force-pushed the add-logger-set-env-levels branch from 1e8b408 to 1f6e866 Compare February 25, 2023 00:29
@gabime gabime merged commit 51bcff8 into gabime:v1.x Feb 25, 2023
@gabime
Copy link
Owner

gabime commented Feb 25, 2023

Thanks @afshinpir

@tt4g
Copy link
Contributor

tt4g commented Feb 25, 2023

@afshinpir Sorry about this after merged. I forgot to submit my review.
Could you please open PR to remove std::lock_guard<std::mutex> lock(logger_map_mutex_); in registry::apply_logger_env_levels()?
This is because logger_map_mutex_ is std::mutex to protect loggers_ variables.

SPDLOG_INLINE void registry::apply_logger_env_levels(std::shared_ptr<logger> new_logger)
{
std::lock_guard<std::mutex> lock(logger_map_mutex_);
auto it = log_levels_.find(new_logger->name());
auto new_level = it != log_levels_.end() ? it->second : global_log_level_;
new_logger->set_level(new_level);
}

@afshinpir afshinpir deleted the add-logger-set-env-levels branch February 25, 2023 11:37
@afshinpir
Copy link
Contributor Author

afshinpir commented Feb 25, 2023

@tt4g I can open a new PR, but since log_levels_ is a map, so we may need to have excusive access to it for thread safety. Since registry::set_levels and registry::initialize_logger both use this mutex, I used the same one. So this mutex currently protects access to both log_levels_ and loggers_. I think it is better to have it like this rather than adding new critical section. or I can just remove it and assume that set_levels and apply_logger_env_levels will never be called at same time. Please let me know what to do.

@tt4g
Copy link
Contributor

tt4g commented Feb 25, 2023

@afshinpir I agree with your thoughts.
Because of the variable names I was unaware that logger_map_mutex_ protected loggers_ and log_levels_.
There would be no need to open a PR.

seker pushed a commit to seker/spdlog that referenced this pull request Mar 3, 2023
This method applies levels which is set by environment variable
`SPDLOG_LEVEL` to the a single controller. Usefull for loading
configuration into manually created loggers.
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.

3 participants