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

fix(logger-utils): ensure external loggers doesn't propagate logs on config copy #1075

Merged
Merged
9 changes: 5 additions & 4 deletions aws_lambda_powertools/logging/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def copy_config_to_registered_loggers(
exclude : Optional[Set[str]], optional
List of logger names to exclude, by default None
"""

package_logger = logging.getLogger("aws_lambda_powertools")
level = log_level or source_logger.level

# Assumptions: Only take parent loggers not children (dot notation rule)
Expand All @@ -34,11 +34,11 @@ def copy_config_to_registered_loggers(
# 3. Include and exclude set? Add Logger if it’s in include and not in exclude
# 4. Only exclude set? Ignore Logger in the excluding list

# Exclude source logger by default
# Exclude source and powertools package logger by default
if exclude:
exclude.add(source_logger.name)
exclude.update(source_logger.name, package_logger.name)
else:
exclude = {source_logger.name}
exclude = {source_logger.name, package_logger.name}

# Prepare loggers set
if include:
Expand Down Expand Up @@ -75,6 +75,7 @@ def _find_registered_loggers(
def _configure_logger(source_logger: Logger, logger: logging.Logger, level: Union[int, str]) -> None:
logger.handlers = []
logger.setLevel(level)
logger.propagate = False
heitorlessa marked this conversation as resolved.
Show resolved Hide resolved
source_logger.debug(f"Logger {logger} reconfigured to use logging level {level}")
for source_handler in source_logger.handlers:
logger.addHandler(source_handler)
Expand Down
28 changes: 27 additions & 1 deletion tests/functional/test_logger_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def test_copy_config_to_ext_loggers_custom_log_level(stdout, logger, log_level):
assert log["level"] == log_level.WARNING.name


def test_copy_config_to_ext_loggers_should_not_break_append_keys(stdout, logger, log_level):
def test_copy_config_to_ext_loggers_should_not_break_append_keys(stdout, log_level):
# GIVEN powertools logger initialized
powertools_logger = Logger(service=service_name(), level=log_level.INFO.value, stream=stdout)

Expand All @@ -193,3 +193,29 @@ def test_copy_config_to_ext_loggers_should_not_break_append_keys(stdout, logger,

# THEN append_keys should not raise an exception
powertools_logger.append_keys(key="value")


def test_copy_config_to_ext_loggers_no_duplicate_logs(stdout, logger, log_level):
# GIVEN an root logger, external logger and powertools logger initialized

root_logger = logging.getLogger()
handler = logging.StreamHandler(stdout)
formatter = logging.Formatter('{"message": "%(message)s"}')
handler.setFormatter(formatter)
root_logger.handlers = [handler]

logger = logger()

powertools_logger = Logger(service=service_name(), level=log_level.CRITICAL.value, stream=stdout)
level = log_level.WARNING.name

# WHEN configuration copied from powertools logger
# AND external logger used with custom log_level
utils.copy_config_to_registered_loggers(source_logger=powertools_logger, include={logger.name}, log_level=level)
msg = "test message4"
logger.warning(msg)

# THEN no root logger logs AND log is not duplicated
logs = capture_multiple_logging_statements_output(stdout)
assert not {"message": msg} in logs
assert sum(msg in log.values() for log in logs) == 1