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

Introduce LOGGING_OVERRIDE config var #10808

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions localstack/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,10 @@ def in_docker():
LS_LOG = eval_log_type("LS_LOG")
DEBUG = is_env_true("DEBUG") or LS_LOG in TRACE_LOG_LEVELS

# EXPERIMENTAL
# allow setting custom log levels for individual loggers
simonrw marked this conversation as resolved.
Show resolved Hide resolved
LOGGING_OVERRIDE = os.environ.get("LOGGING_OVERRIDE", "")

# whether to enable debugpy
DEVELOP = is_env_true("DEVELOP")

Expand Down
19 changes: 19 additions & 0 deletions localstack/logging/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from localstack import config, constants

from ..utils.collections import parse_key_value_pairs
from .format import AddFormattedAttributes, DefaultFormatter

# The log levels for modules are evaluated incrementally for logging granularity,
Expand Down Expand Up @@ -78,6 +79,24 @@ def setup_logging_from_config():
for name, level in trace_internal_log_levels.items():
logging.getLogger(name).setLevel(level)

raw_logging_override = config.LOGGING_OVERRIDE
if raw_logging_override:
try:
logging_overrides = parse_key_value_pairs(raw_logging_override)
for logger, level_name in logging_overrides.items():
level = getattr(logging, level_name, None)
if not level:
raise RuntimeError(
f"Failed to configure logging overrides ({raw_logging_override}): '{level_name}' is not a valid log level"
)
Comment on lines +89 to +91
Copy link
Member

@thrau thrau May 15, 2024

Choose a reason for hiding this comment

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

nit: is there a specific reason to raise RuntimeError and not ValueError?

logging.getLogger(logger).setLevel(level)
except RuntimeError:
raise
except Exception as e:
raise RuntimeError(
Copy link
Member

Choose a reason for hiding this comment

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

q: Can this exception really only be raised in case the value is malformed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure, so since we "raise ... from e" I am happy to remove the "malformed input" part of the error message as it only printed e anyway.

f"Failed to configure logging overrides ({raw_logging_override})"
) from e


def create_default_handler(log_level: int):
log_handler = logging.StreamHandler(stream=sys.stderr)
Expand Down
21 changes: 21 additions & 0 deletions localstack/utils/collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -533,3 +533,24 @@ def is_comma_delimited_list(string: str, item_regex: Optional[str] = None) -> bo
if pattern.match(string) is None:
return False
return True


def parse_key_value_pairs(raw_text: str) -> dict:
Copy link
Member

Choose a reason for hiding this comment

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

nit: could add a docstring here with for example the values from one of the test cases below to clearly demonstrate its purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion!

"""
Parse a series of key-value pairs, in an environment variable format into a dictionary

>>> input = "a=b,c=d"
>>> assert parse_key_value_pairs(input) == {"a": "b", "c": "d"}
"""
result = {}
for pair in raw_text.split(","):
items = pair.split("=")
if len(items) != 2:
raise ValueError(f"invalid key/value pair: '{pair}'")
Comment on lines +548 to +549
Copy link
Member

Choose a reason for hiding this comment

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

q: Since maxsplit=2 is set, this means that this check really only checks if the pair does not contain = right?
Maybe it would be better to remove the maxsplit to also catch cases like a=b=c?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, thanks!

raw_key, raw_value = items[0].strip(), items[1].strip()
if not raw_key:
raise ValueError(f"missing key: '{pair}'")
if not raw_value:
raise ValueError(f"missing value: '{pair}'")
result[raw_key] = raw_value
return result
26 changes: 26 additions & 0 deletions tests/unit/utils/test_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
ImmutableList,
convert_to_typed_dict,
is_comma_delimited_list,
parse_key_value_pairs,
select_from_typed_dict,
)

Expand Down Expand Up @@ -193,3 +194,28 @@ def test_is_comma_limited_list():
assert not is_comma_delimited_list("foo, bar baz")
assert not is_comma_delimited_list("foo,")
assert not is_comma_delimited_list("")


@pytest.mark.parametrize(
"input_text,expected",
[
("a=b", {"a": "b"}),
("a=b,c=d", {"a": "b", "c": "d"}),
],
)
def test_parse_key_value_pairs(input_text, expected):
assert parse_key_value_pairs(input_text) == expected


@pytest.mark.parametrize(
"input_text,message",
[
("a=b,", "invalid key/value pair: ''"),
("a=b,c=", "missing value: 'c='"),
],
)
def test_parse_key_value_pairs_error_messages(input_text, message):
with pytest.raises(ValueError) as exc_info:
parse_key_value_pairs(input_text)

assert str(exc_info.value) == message
Loading