PyPlugin logging policy

[lacraig2]

We have a --verbose option that is passed to the overall project on run.

Who is that verbose argument for? The core penguin code? All the plugins?

Because plugin arguments are the combination of the penguin arguments and the plugin arguments we really can't tell.

At the moment we just pass verbose as a bool for the overall project to the plugins and they set their own logger level individually based on that like so:

        if self.get_arg_bool("verbose"):
            self.logger.setLevel("DEBUG")

Some plugins also seem to have their own "verbose" argument.

I'd like to coordinate the log levels in a better way.

To do that I propose:

• The argument that indicates the presence of the "--verbose" flag will be "penguin_verbose"
• We can set the level of the logger with arguments to "verbose" and "loglevel" with bool, str, or int levels

We update all pyplugins to support this functionality as well

[Copilot]

Pull Request Overview

This PR implements a new logging policy for PyPlugins to better coordinate log levels between the core penguin code and plugins. The main goal is to distinguish between global penguin verbosity and plugin-specific verbosity settings.

• Replaces the ambiguous "verbose" argument with "penguin_verbose" to clearly indicate when the global penguin verbose flag is active
• Adds centralized logger initialization in the base Plugin class that supports both "verbose" and "loglevel" arguments
• Updates all existing pyplugins to use the new "penguin_verbose" argument instead of the old "verbose" argument

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

File	Description
src/penguin/plugin_manager.py	Adds centralized logger initialization with support for verbose/loglevel args and renames verbose to penguin_verbose in plugin manager
src/penguin/penguin_run.py	Updates argument passing to use penguin_verbose instead of verbose
pyplugins/*.py (21 files)	Updates all plugin files to use penguin_verbose argument for logger configuration

Comments suppressed due to low confidence (2)

src/penguin/plugin_manager.py:69

• The method name '_set_level_arg' is misleading since it doesn't set a level argument but rather sets the logger level based on an argument. Consider renaming to '_set_logger_level' or '_apply_log_level'.

    def _set_level_arg(self, level: Union[str, bool, int]) -> None:

src/penguin/plugin_manager.py:71

• The variable name 'verbose_arg' is confusing since this method is called with both 'verbose' and 'loglevel' arguments. Consider renaming to 'level_arg' or 'log_level' to better reflect its purpose.

        verbose_arg = self.args.get("verbose", False)

[Copilot]

The comment is misleading since this method handles both 'verbose' and 'loglevel' arguments, not just verbose. Update the comment to reflect that it handles any log level argument.

Suggested change

	# The verbose argument is used to set the logger level
	# This method sets the logger level based on the provided log level argument (e.g., 'verbose' or 'loglevel').

[Copilot]

Use 'isinstance(verbose_arg, (int, str))' instead of separate isinstance checks for better readability and performance.

Suggested change

	elif isinstance(verbose_arg, int) or isinstance(verbose_arg, str):
	# If verbose is an int, set the logger level to that value
	elif isinstance(verbose_arg, (int, str)):
	# If verbose is an int or str, set the logger level to that value

[Copilot]

The method parameter is named 'level' but the method implementation retrieves values from self.args instead of using the passed parameter. Either use the parameter or remove it and pass the argument value directly.

Suggested change

	def _set_level_arg(self, level: Union[str, bool, int]) -> None:
	def _set_level_arg(self) -> None:

[zestrada]

Probably missing something, but I'm a bit confused - how is verbose being set/used here?

penguin/src/penguin/plugin_manager.py

Lines 69 to 90 in a5d6e0b

	def _set_level_arg(self, level: Union[str, bool, int]) -> None:
	# The verbose argument is used to set the logger level
	verbose_arg = self.args.get("verbose", False)
	if isinstance(verbose_arg, bool):
	if verbose_arg is True:
	self.logger.setLevel("DEBUG")
	elif isinstance(verbose_arg, int) or isinstance(verbose_arg, str):
	# If verbose is an int, set the logger level to that value
	self.logger.setLevel(verbose_arg)
	else:
	raise ValueError(
	f"Unsupported verbose argument type: {type(verbose_arg)}. Expected bool, int, or str.")
	def _initialize_logger(self):
	logname = camel_to_snake(self.name)
	self.logger = getColoredLogger(f"plugins.{logname}")
	if self.args.get("verbose", None) is not None:
	# If verbose is set, set the logger level to DEBUG
	self._set_level_arg(self.args["verbose"])
	elif self.args.get("loglevel", None) is not None:
	# If loglevel is set, set the logger level to that value
	self._set_level_arg(self.args["loglevel"])

From what I can tell, now we pass penguin_verbose through args?

penguin/src/penguin/penguin_run.py

Lines 536 to 552 in a5d6e0b

	args = {
	"plugins": conf_plugins,
	"conf": conf,
	"proj_name": os.path.basename(proj_dir).replace("host_", ""),
	"proj_dir": proj_dir,
	"plugin_path": plugin_path,
	"fs": config_fs,
	"fw": config_image,
	"outdir": out_dir,
	"penguin_verbose": verbose,
	"telnet_port": telnet_port,
	}
	args.update(vpn_args)
	sys.path.append("/pyplugins")
	plugins.initialize(panda, args)

[lacraig2]

@zestrada currently "verbose" is carried through from the --verbose argument on the command line in penguin.

This PR relabels that as "penguin_verbose" in the arguments passed to plugins so that they can set their own "verbose" flag in plugin arguments.