Skip to content

Commit c6eb450

Browse files
JennyPngCopilot
andauthored
Add Centralized Logger for Checks (#42775)
* add wip logging.py * pass args to logging * use logger in mypy, move logger to logging/init * use env var * minor log fix * replace logging with logger * replace prints with logs * make log level args mutually exclusive * fix arg precedence logic * add unit test * test fix * test fix * minor fix * logic fix * formatting fix * merge * clean test * standardize log params * copilot formatting fix Co-authored-by: Copilot <[email protected]> * remove levels arg --------- Co-authored-by: Copilot <[email protected]>
1 parent 158e988 commit c6eb450

File tree

7 files changed

+104
-31
lines changed

7 files changed

+104
-31
lines changed

eng/tools/azure-sdk-tools/azpysdk/Check.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from ci_tools.functions import discover_targeted_packages, get_venv_call
1212
from ci_tools.variables import discover_repo_root
1313
from ci_tools.scenario import install_into_venv, get_venv_python
14+
from ci_tools.logging import logger
1415

1516
# right now, we are assuming you HAVE to be in the azure-sdk-tools repo
1617
# we assume this because we don't know how a dev has installed this package, and might be
@@ -84,17 +85,17 @@ def get_targeted_directories(self, args: argparse.Namespace) -> List[ParsedSetup
8485
try:
8586
targeted.append(ParsedSetup.from_path(targeted_dir))
8687
except Exception as e:
87-
print("Error: Current directory does not appear to be a Python package (no setup.py or setup.cfg found). Remove '.' argument to run on child directories.")
88-
print(f"Exception: {e}")
88+
logger.error("Error: Current directory does not appear to be a Python package (no setup.py or setup.cfg found). Remove '.' argument to run on child directories.")
89+
logger.error(f"Exception: {e}")
8990
return []
9091
else:
9192
targeted_packages = discover_targeted_packages(args.target, targeted_dir)
9293
for pkg in targeted_packages:
9394
try:
9495
targeted.append(ParsedSetup.from_path(pkg))
9596
except Exception as e:
96-
print(f"Unable to parse {pkg} as a Python package. Dumping exception detail and skipping.")
97-
print(f"Exception: {e}")
98-
print(traceback.format_exc())
97+
logger.error(f"Unable to parse {pkg} as a Python package. Dumping exception detail and skipping.")
98+
logger.error(f"Exception: {e}")
99+
logger.error(traceback.format_exc())
99100

100101
return targeted

eng/tools/azure-sdk-tools/azpysdk/import_all.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import argparse
22
import os
33
import sys
4-
import logging
54
import tempfile
65

76
from typing import Optional,List
@@ -11,6 +10,7 @@
1110
from ci_tools.parsing import ParsedSetup
1211
from ci_tools.functions import discover_targeted_packages
1312
from ci_tools.scenario.generation import create_package_and_install
13+
from ci_tools.logging import logger
1414

1515
# keyvault has dependency issue when loading private module _BearerTokenCredentialPolicyBase from azure.core.pipeline.policies
1616
# azure.core.tracing.opencensus and azure.eventhub.checkpointstoreblob.aio are skipped due to a known issue in loading azure.core.tracing.opencensus
@@ -42,7 +42,7 @@ def register(self, subparsers: "argparse._SubParsersAction", parent_parsers: Opt
4242
# todo: figure out venv abstraction mechanism via override
4343
def run(self, args: argparse.Namespace) -> int:
4444
"""Run the import_all check command."""
45-
print("Running import_all check in isolated venv...")
45+
logger.info("Running import_all check in isolated venv...")
4646

4747
targeted = self.get_targeted_directories(args)
4848

@@ -69,7 +69,7 @@ def run(self, args: argparse.Namespace) -> int:
6969

7070
if should_run_import_all(parsed.name):
7171
# import all modules from current package
72-
logging.info(
72+
logger.info(
7373
"Importing all modules from namespace [{0}] to verify dependency".format(
7474
parsed.namespace
7575
)
@@ -82,8 +82,8 @@ def run(self, args: argparse.Namespace) -> int:
8282
]
8383

8484
outcomes.append(check_call(commands))
85-
logging.info("Verified module dependency, no issues found")
85+
logger.info("Verified module dependency, no issues found")
8686
else:
87-
logging.info("Package {} is excluded from dependency check".format(parsed.name))
87+
logger.info("Package {} is excluded from dependency check".format(parsed.name))
8888

8989
return max(outcomes) if outcomes else 0

eng/tools/azure-sdk-tools/azpysdk/main.py

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
from .import_all import import_all
1717
from .mypy import mypy
1818

19+
from ci_tools.logging import configure_logging, logger
20+
1921
__all__ = ["main", "build_parser"]
2022
__version__ = "0.0.0"
2123

@@ -29,6 +31,26 @@ def build_parser() -> argparse.ArgumentParser:
2931
parser.add_argument("--isolate", action="store_true", default=False,
3032
help="If set, run in an isolated virtual environment.")
3133

34+
# mutually exclusive logging options
35+
log_group = parser.add_mutually_exclusive_group()
36+
log_group.add_argument(
37+
"--quiet",
38+
action="store_true",
39+
default=False,
40+
help="Enable quiet mode (only shows ERROR logs)"
41+
)
42+
log_group.add_argument(
43+
"--verbose",
44+
action="store_true",
45+
default=False,
46+
help="Enable verbose mode (shows DEBUG logs)"
47+
)
48+
log_group.add_argument(
49+
"--log-level",
50+
choices=["DEBUG", "INFO", "WARN", "ERROR", "FATAL"],
51+
help="Set the logging level."
52+
)
53+
3254
common = argparse.ArgumentParser(add_help=False)
3355
common.add_argument(
3456
"target",
@@ -65,6 +87,8 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
6587
parser = build_parser()
6688
args = parser.parse_args(argv)
6789

90+
configure_logging(args)
91+
6892
if not hasattr(args, "func"):
6993
parser.print_help()
7094
return 1
@@ -73,10 +97,10 @@ def main(argv: Optional[Sequence[str]] = None) -> int:
7397
result = args.func(args)
7498
return int(result or 0)
7599
except KeyboardInterrupt:
76-
print("Interrupted by user", file=sys.stderr)
100+
logger.error("Interrupted by user")
77101
return 130
78102
except Exception as exc: # pragma: no cover - simple top-level error handling
79-
print(f"Error: {exc}", file=sys.stderr)
103+
logger.error(f"Error: {exc}")
80104
return 2
81105

82106
if __name__ == "__main__":

eng/tools/azure-sdk-tools/azpysdk/mypy.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import argparse
22
import os
33
import sys
4-
import logging
54
import tempfile
65

76
from typing import Optional, List
@@ -15,8 +14,7 @@
1514
from ci_tools.environment_exclusions import (
1615
is_check_enabled, is_typing_ignored
1716
)
18-
19-
logging.getLogger().setLevel(logging.INFO)
17+
from ci_tools.logging import logger
2018

2119
PYTHON_VERSION = "3.9"
2220
MYPY_VERSION = "1.14.1"
@@ -41,7 +39,7 @@ def register(self, subparsers: "argparse._SubParsersAction", parent_parsers: Opt
4139

4240
def run(self, args: argparse.Namespace) -> int:
4341
"""Run the mypy check command."""
44-
print("Running mypy check in isolated venv...")
42+
logger.info("Running mypy check in isolated venv...")
4543

4644
set_envvar_defaults()
4745

@@ -52,8 +50,10 @@ def run(self, args: argparse.Namespace) -> int:
5250
for parsed in targeted:
5351
package_dir = parsed.folder
5452
package_name = parsed.name
53+
5554
executable, staging_directory = self.get_executable(args.isolate, args.command, sys.executable, package_dir)
56-
print(f"Processing {package_name} for mypy check")
55+
logger.info(f"Processing {package_name} for mypy check")
56+
5757
create_package_and_install(
5858
distribution_directory=staging_directory,
5959
target_setup=package_dir,
@@ -74,14 +74,14 @@ def run(self, args: argparse.Namespace) -> int:
7474
else:
7575
pip_install([f"mypy=={MYPY_VERSION}"], True, executable, package_dir)
7676
except CalledProcessError as e:
77-
print("Failed to install mypy:", e)
77+
logger.error("Failed to install mypy:", e)
7878
return e.returncode
7979

80-
logging.info(f"Running mypy against {package_name}")
80+
logger.info(f"Running mypy against {package_name}")
8181

8282
if not args.next and in_ci():
8383
if not is_check_enabled(package_dir, "mypy", True) or is_typing_ignored(package_name):
84-
logging.info(
84+
logger.info(
8585
f"Package {package_name} opts-out of mypy check. See https://aka.ms/python/typing-guide for information."
8686
)
8787
continue
@@ -101,17 +101,17 @@ def run(self, args: argparse.Namespace) -> int:
101101
src_code_error = None
102102
sample_code_error = None
103103
try:
104-
logging.info(
104+
logger.info(
105105
f"Running mypy commands on src code: {src_code}"
106106
)
107107
results.append(check_call(src_code))
108-
logging.info("Verified mypy, no issues found")
108+
logger.info("Verified mypy, no issues found")
109109
except CalledProcessError as src_error:
110110
src_code_error = src_error
111111
results.append(src_error.returncode)
112112

113113
if not args.next and in_ci() and not is_check_enabled(package_dir, "type_check_samples", True):
114-
logging.info(
114+
logger.info(
115115
f"Package {package_name} opts-out of mypy check on samples."
116116
)
117117
continue
@@ -120,7 +120,7 @@ def run(self, args: argparse.Namespace) -> int:
120120
samples = os.path.exists(os.path.join(package_dir, "samples"))
121121
generated_samples = os.path.exists(os.path.join(package_dir, "generated_samples"))
122122
if not samples and not generated_samples:
123-
logging.info(
123+
logger.info(
124124
f"Package {package_name} does not have a samples directory."
125125
)
126126
else:
@@ -131,7 +131,7 @@ def run(self, args: argparse.Namespace) -> int:
131131
os.path.join(package_dir, "samples" if samples else "generated_samples"),
132132
]
133133
try:
134-
logging.info(
134+
logger.info(
135135
f"Running mypy commands on sample code: {sample_code}"
136136
)
137137
results.append(check_call(sample_code))

eng/tools/azure-sdk-tools/azpysdk/whl.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import argparse
2-
import logging
32
import tempfile
43
import os
54
from typing import Optional, List, Any
@@ -12,7 +11,7 @@
1211
from ci_tools.variables import set_envvar_defaults
1312
from ci_tools.parsing import ParsedSetup
1413
from ci_tools.scenario.generation import create_package_and_install
15-
14+
from ci_tools.logging import logger
1615

1716
class whl(Check):
1817
def __init__(self) -> None:
@@ -29,7 +28,7 @@ def register(self, subparsers: "argparse._SubParsersAction", parent_parsers: Opt
2928

3029
def run(self, args: argparse.Namespace) -> int:
3130
"""Run the whl check command."""
32-
print("Running whl check...")
31+
logger.info("Running whl check...")
3332

3433
set_envvar_defaults()
3534

@@ -64,7 +63,7 @@ def run(self, args: argparse.Namespace) -> int:
6463

6564
# TODO: split sys.argv[1:] on -- and pass in everything after the -- as additional arguments
6665
# TODO: handle mark_args
67-
logging.info(f"Invoke pytest for {pkg}")
66+
logger.info(f"Invoke pytest for {pkg}")
6867
exit_code = run(
6968
[executable, "-m", "pytest", "."] + [
7069
"-rsfE",
@@ -83,9 +82,9 @@ def run(self, args: argparse.Namespace) -> int:
8382

8483
if exit_code != 0:
8584
if exit_code == 5 and is_error_code_5_allowed(parsed.folder, parsed.name):
86-
logging.info("Exit code 5 is allowed, continuing execution.")
85+
logger.info("Exit code 5 is allowed, continuing execution.")
8786
else:
88-
logging.info(f"pytest failed with exit code {exit_code}.")
87+
logger.info(f"pytest failed with exit code {exit_code}.")
8988
results.append(exit_code)
9089

9190
# final result is the worst case of all the results

eng/tools/azure-sdk-tools/ci_tools/logging/__init__.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,37 @@
44
import os
55
import datetime
66
from subprocess import run
7+
import argparse
78

89
LOGLEVEL = getattr(logging, os.environ.get("LOGLEVEL", "INFO").upper())
910

11+
logger = logging.getLogger("azure-sdk-tools")
12+
13+
def configure_logging(
14+
args: argparse.Namespace,
15+
fmt: str = "%(asctime)s [%(levelname)s] %(name)s: %(message)s"
16+
) -> None:
17+
"""
18+
Configures the shared logger. Should be called **once** at startup.
19+
"""
20+
# use cli arg > log level arg > env var
21+
22+
if args.quiet:
23+
numeric_level = logging.ERROR
24+
elif args.verbose:
25+
numeric_level = logging.DEBUG
26+
elif not args.log_level:
27+
numeric_level = getattr(logging, os.environ.get("LOGLEVEL", "INFO").upper())
28+
else:
29+
numeric_level = getattr(logging, args.log_level.upper(), None)
30+
31+
if not isinstance(numeric_level, int):
32+
raise ValueError(f"Invalid log level: {numeric_level}")
33+
logger.setLevel(numeric_level)
34+
35+
# Propagate logger config globally if needed
36+
logging.basicConfig(level=numeric_level, format=fmt)
37+
1038

1139
def now() -> str:
1240
return datetime.datetime.now().strftime("%Y-%m-%dT%H.%M.%S")
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
2+
import logging
3+
from ci_tools.logging import configure_logging, logger
4+
from unittest.mock import patch
5+
import pytest
6+
import argparse
7+
import os
8+
9+
@pytest.mark.parametrize("cli_args,level_env,expected_level", [
10+
(argparse.Namespace(quiet=True, verbose=False, log_level=None), "INFO", logging.ERROR),
11+
(argparse.Namespace(quiet=False, verbose=True, log_level=None), "INFO", logging.DEBUG),
12+
(argparse.Namespace(quiet=False, verbose=False, log_level="ERROR"), "INFO", logging.ERROR),
13+
(argparse.Namespace(quiet=False, verbose=False, log_level=None), "WARN", logging.WARNING),
14+
])
15+
@patch("logging.basicConfig")
16+
def test_configure_logging_various_levels(mock_basic_config, cli_args, level_env, expected_level, monkeypatch):
17+
monkeypatch.setenv("LOGLEVEL", level_env)
18+
assert os.environ["LOGLEVEL"] == level_env
19+
configure_logging(cli_args)
20+
assert logger.level == expected_level
21+
mock_basic_config.assert_called_with(level=expected_level, format="%(asctime)s [%(levelname)s] %(name)s: %(message)s")

0 commit comments

Comments
 (0)