From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id F3E5B45460; Fri, 14 Jun 2024 22:26:18 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8846340674; Fri, 14 Jun 2024 22:26:18 +0200 (CEST) Received: from mail-lf1-f41.google.com (mail-lf1-f41.google.com [209.85.167.41]) by mails.dpdk.org (Postfix) with ESMTP id 74D544060B for ; Fri, 14 Jun 2024 22:26:17 +0200 (CEST) Received: by mail-lf1-f41.google.com with SMTP id 2adb3069b0e04-52a559e4429so336578e87.0 for ; Fri, 14 Jun 2024 13:26:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1718396777; x=1719001577; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=1vMnhMWzgKSowXKQtiigV6aLUcWNnb3vIseitHtCszU=; b=Lp0KmVfKmMl0/KR5f9W6jrKw+ARYRTXsyq96UzOHFDp5aYO3SdU9AK9O5Klq2etiv/ MnbYP4/3sC/3qqw+OyJqqvcx1a7JH8AYLarQK2myhfL+NuzV76ip9WigUhvLc91tXt7/ sx4k7s9qs0T+OPKjEYHEuqDTEqXNC+I8N0g/U= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718396777; x=1719001577; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=1vMnhMWzgKSowXKQtiigV6aLUcWNnb3vIseitHtCszU=; b=wy7nyU46lAcu1nEYIJ9H6Pixeel/qDXtnTi2os9saA/9+fd1kZgp7cUwvvmWrQ3/kA yByv4h0wJV6jk/ju/JSC6N/H42CDpL61JEMRacirLuRiBrzR03Wud6Fk46RJHsJAz1B3 1xgkv056kK2LkNawfinHKgbpSAln1Oe+TVyFPp9Qj4iE9nCXwl9c7si27RIEU+E/Vp7m jP0wv3+wyARz9+pxUG4WRpoLG5hvvYXc/1/aeKFqZo4cblc1MdwFXJaTD4sl+R0kE7zX spzX8U8pqFf6DUXP7n7EW2Y5AjtyunOvWcMDPR0ky41ylKekvOq14h6CAXuHS9Fopnt2 TGhA== X-Forwarded-Encrypted: i=1; AJvYcCX4681TsDJvwmXnqZQ78SS+LUsNCcc3eQhhAEXSp5saTI38wI4iUU3UNrsrA37iYt3qA+nXKxR3Sg+1mik= X-Gm-Message-State: AOJu0Yygbk6cqNRe7QsbIlWWr+CT8K4o6sXXMWu0Nl1iTomPufqAye3R KuK5LV9qvRVx/oX+wdLV8CD8v7skX1RX/vohrriABXcHt58ZFhQ1uH33LMoXSwUa6EVgOKRkNYX sxYK2DcwUe1pDpcx5NzQSM2Rc66h6r8gMSLFDcQ== X-Google-Smtp-Source: AGHT+IFTDi64Ei67nuoLEK3czkYPu5TYlQouI9bCX1zOwuzuHjoolL17MBSTjesONPJGYh0u6rU5hRmFBMpg6y6yNnk= X-Received: by 2002:a05:651c:154a:b0:2ec:143e:2893 with SMTP id 38308e7fff4ca-2ec143e2926mr20955561fa.2.1718396776752; Fri, 14 Jun 2024 13:26:16 -0700 (PDT) MIME-Version: 1.0 References: <20240501161623.26672-1-jspewock@iol.unh.edu> <20240529194910.26803-1-jspewock@iol.unh.edu> <20240529194910.26803-4-jspewock@iol.unh.edu> In-Reply-To: <20240529194910.26803-4-jspewock@iol.unh.edu> From: Nicholas Pratte Date: Fri, 14 Jun 2024 16:26:05 -0400 Message-ID: Subject: Re: [PATCH v3 3/3] dts: Improve logging for interactive shells To: jspewock@iol.unh.edu Cc: paul.szczepanek@arm.com, wathsala.vithanage@arm.com, probb@iol.unh.edu, Luca.Vizzarro@arm.com, thomas@monjalon.net, juraj.linkes@pantheon.tech, Honnappa.Nagarahalli@arm.com, yoan.picchi@foss.arm.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Ran a couple test suites to observe the output. As expected, everything works fine. Tested-by: Nicholas Pratte Reviewed-by: Nicholas Pratte On Wed, May 29, 2024 at 3:49=E2=80=AFPM wrote: > > From: Jeremy Spewock > > The messages being logged by interactive shells currently are using the > same logger as the node they were created from. Because of this, when > sending interactive commands, the logs make no distinction between when > you are sending a command directly to the host and when you are using an > interactive shell on the host. This change adds names to interactive > shells so that they are able to use their own loggers with distinct > names. > > Signed-off-by: Jeremy Spewock > --- > .../remote_session/interactive_remote_session.py | 5 +++-- > dts/framework/remote_session/interactive_shell.py | 9 +++++---- > dts/framework/testbed_model/node.py | 7 +++++++ > dts/framework/testbed_model/os_session.py | 6 ++++-- > dts/framework/testbed_model/sut_node.py | 7 ++++++- > dts/framework/testbed_model/traffic_generator/scapy.py | 2 +- > 6 files changed, 26 insertions(+), 10 deletions(-) > > diff --git a/dts/framework/remote_session/interactive_remote_session.py b= /dts/framework/remote_session/interactive_remote_session.py > index c50790db79..4ddad4f428 100644 > --- a/dts/framework/remote_session/interactive_remote_session.py > +++ b/dts/framework/remote_session/interactive_remote_session.py > @@ -39,6 +39,7 @@ class InteractiveRemoteSession: > password: Password of the user connecting to the host. This will= default to an > empty string if a password is not provided. > session: The underlying paramiko connection. > + node_config: The configuration of the node that this session is = connected to. > > Raises: > SSHConnectionError: There is an error creating the SSH connectio= n. > @@ -50,8 +51,8 @@ class InteractiveRemoteSession: > username: str > password: str > session: SSHClient > + node_config: NodeConfiguration > _logger: DTSLogger > - _node_config: NodeConfiguration > _transport: Transport | None > > def __init__(self, node_config: NodeConfiguration, logger: DTSLogger= ) -> None: > @@ -61,7 +62,7 @@ def __init__(self, node_config: NodeConfiguration, logg= er: DTSLogger) -> None: > node_config: The test run configuration of the node to conne= ct to. > logger: The logger instance this session will use. > """ > - self._node_config =3D node_config > + self.node_config =3D node_config > self._logger =3D logger > self.hostname =3D node_config.hostname > self.username =3D node_config.user > diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/fram= ework/remote_session/interactive_shell.py > index 148907f645..edc8f11443 100644 > --- a/dts/framework/remote_session/interactive_shell.py > +++ b/dts/framework/remote_session/interactive_shell.py > @@ -24,7 +24,7 @@ > InteractiveSSHSessionDeadError, > InteractiveSSHTimeoutError, > ) > -from framework.logger import DTSLogger > +from framework.logger import DTSLogger, get_dts_logger > from framework.settings import SETTINGS > > from .interactive_remote_session import InteractiveRemoteSession > @@ -69,8 +69,8 @@ class InteractiveShell(ABC): > > def __init__( > self, > + name: str, > interactive_session: InteractiveRemoteSession, > - logger: DTSLogger, > get_privileged_command: Callable[[str], str] | None, > app_args: str =3D "", > timeout: float =3D SETTINGS.timeout, > @@ -78,8 +78,9 @@ def __init__( > """Create an SSH channel during initialization. > > Args: > + name: Name for the interactive shell to use for logging. Thi= s name will be appended to > + the name of the underlying node which it is running on. > interactive_session: The SSH session dedicated to interactiv= e shells. > - logger: The logger instance this session will use. > get_privileged_command: A method for modifying a command to = allow it to use > elevated privileges. If :data:`None`, the application wi= ll not be started > with elevated privileges. > @@ -94,7 +95,7 @@ def __init__( > self._stdout =3D self._ssh_channel.makefile("r") > self._ssh_channel.settimeout(timeout) > self._ssh_channel.set_combine_stderr(True) # combines stdout an= d stderr streams > - self._logger =3D logger > + self._logger =3D get_dts_logger(f"{interactive_session.node_conf= ig.name}.{name}") > self._timeout =3D timeout > self._app_args =3D app_args > self._start_application(get_privileged_command) > diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_= model/node.py > index 74061f6262..a5beeae7e7 100644 > --- a/dts/framework/testbed_model/node.py > +++ b/dts/framework/testbed_model/node.py > @@ -199,6 +199,7 @@ def create_interactive_shell( > shell_cls: Type[InteractiveShellType], > timeout: float =3D SETTINGS.timeout, > privileged: bool =3D False, > + name: str =3D "", > app_args: str =3D "", > ) -> InteractiveShellType: > """Factory for interactive session handlers. > @@ -210,6 +211,8 @@ def create_interactive_shell( > timeout: Timeout for reading output from the SSH channel. If= you are reading from > the buffer and don't receive any data within the timeout= it will throw an error. > privileged: Whether to run the shell with administrative pri= vileges. > + name: The name to use for the interactive application in the= logs. If not specified, > + this will default to the name of `shell_cls`. > app_args: The arguments to be passed to the application. > > Returns: > @@ -218,10 +221,14 @@ def create_interactive_shell( > if not shell_cls.dpdk_app: > shell_cls.path =3D self.main_session.join_remote_path(shell_= cls.path) > > + if name =3D=3D "": > + name =3D shell_cls.__name__ > + > return self.main_session.create_interactive_shell( > shell_cls, > timeout, > privileged, > + name, > app_args, > ) > > diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/te= stbed_model/os_session.py > index d5bf7e0401..1f6d8257ed 100644 > --- a/dts/framework/testbed_model/os_session.py > +++ b/dts/framework/testbed_model/os_session.py > @@ -134,6 +134,7 @@ def create_interactive_shell( > shell_cls: Type[InteractiveShellType], > timeout: float, > privileged: bool, > + name: str, > app_args: str, > ) -> InteractiveShellType: > """Factory for interactive session handlers. > @@ -146,14 +147,15 @@ def create_interactive_shell( > reading from the buffer and don't receive any data withi= n the timeout > it will throw an error. > privileged: Whether to run the shell with administrative pri= vileges. > + name: Name for the shell to use in the logs. > app_args: The arguments to be passed to the application. > > Returns: > An instance of the desired interactive application shell. > """ > return shell_cls( > - self.interactive_session.session, > - self._logger, > + name, > + self.interactive_session, > self._get_privileged_command if privileged else None, > app_args, > timeout, > diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/test= bed_model/sut_node.py > index 97aa26d419..0bddef6f44 100644 > --- a/dts/framework/testbed_model/sut_node.py > +++ b/dts/framework/testbed_model/sut_node.py > @@ -442,6 +442,7 @@ def create_interactive_shell( > shell_cls: Type[InteractiveShellType], > timeout: float =3D SETTINGS.timeout, > privileged: bool =3D False, > + name: str =3D "", > app_parameters: str =3D "", > eal_parameters: EalParameters | None =3D None, > ) -> InteractiveShellType: > @@ -459,6 +460,8 @@ def create_interactive_shell( > reading from the buffer and don't receive any data withi= n the timeout > it will throw an error. > privileged: Whether to run the shell with administrative pri= vileges. > + name: The name to use for the interactive application in the= logs. If not specified, > + this will default to the name `shell_cls`. > eal_parameters: List of EAL parameters to use to launch the = app. If this > isn't provided or an empty string is passed, it will def= ault to calling > :meth:`create_eal_parameters`. > @@ -478,7 +481,9 @@ def create_interactive_shell( > self.remote_dpdk_build_dir, shell_cls.path > ) > > - return super().create_interactive_shell(shell_cls, timeout, priv= ileged, app_parameters) > + return super().create_interactive_shell( > + shell_cls, timeout, privileged, name, app_parameters > + ) > > def bind_ports_to_driver(self, for_dpdk: bool =3D True) -> None: > """Bind all ports on the SUT to a driver. > diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts= /framework/testbed_model/traffic_generator/scapy.py > index d0e0a7c64e..5676235119 100644 > --- a/dts/framework/testbed_model/traffic_generator/scapy.py > +++ b/dts/framework/testbed_model/traffic_generator/scapy.py > @@ -262,7 +262,7 @@ def __init__(self, tg_node: Node, config: ScapyTraffi= cGeneratorConfig): > ), "Linux is the only supported OS for scapy traffic generation" > > self.session =3D self._tg_node.create_interactive_shell( > - PythonShell, timeout=3D5, privileged=3DTrue > + PythonShell, timeout=3D5, privileged=3DTrue, name=3D"ScapyXM= LRPCServer" > ) > > # import libs in remote python console > -- > 2.45.1 >