Fixes bug which prevented users from setting log level to DEBUG on Windows. matlab-proxy now logs to matlab_logs.txt or to the file pointed to by MWI_LOG_FILE when log level is set to DEBUG.

Author: diningPhilosopher64

matlab_proxy/app_state.py

@@ -11,8 +11,15 @@
 from typing import Final
 
 from matlab_proxy import util
-from matlab_proxy.settings import get_process_startup_timeout
-from matlab_proxy.constants import CONNECTOR_SECUREPORT_FILENAME, VERSION_INFO_FILE_NAME
+from matlab_proxy.settings import (
+    get_process_startup_timeout,
+    get_default_mwi_log_file_path,
+)
+from matlab_proxy.constants import (
+    CONNECTOR_SECUREPORT_FILENAME,
+    MATLAB_LOGS_FILE_NAME,
+    VERSION_INFO_FILE_NAME,
+)
 from matlab_proxy.util import mw, mwi, system, windows
 from matlab_proxy.util.mwi import environment_variables as mwi_env
 from matlab_proxy.util.mwi import token_auth
@@ -663,7 +670,23 @@ async def __setup_env_for_matlab(self) -> dict:
         # Env setup related to logging
         # Very verbose logging in debug mode
         if logger.isEnabledFor(logging.getLevelName("DEBUG")):
-            matlab_env["MW_DIAGNOSTIC_DEST"] = "stdout"
+            mwi_log_file = self.settings.get("mwi_log_file", None)
+            # If a log file is supplied to write matlab-proxy server logs,
+            # use it to write MATLAB logs too.
+            if mwi_log_file:
+                # Append MATLAB logs to matlab-proxy logs
+                matlab_env["MW_DIAGNOSTIC_DEST"] = f"file,append={mwi_log_file}"
+
+            elif system.is_posix():
+                matlab_env["MW_DIAGNOSTIC_DEST"] = "stdout"
+
+            else:
+                # On windows stdout is not supported yet.
+                # So, use the default log file for MATLAB logs
+                default_matlab_logs_file = get_default_mwi_log_file_path()
+                # Write MATLAB logs
+                matlab_env["MW_DIAGNOSTIC_DEST"] = f"file={default_matlab_logs_file}"
+
             matlab_env[
                 "MW_DIAGNOSTIC_SPEC"
             ] = "connector::http::server=all;connector::lifecycle=all"

matlab_proxy/constants.py

@@ -6,6 +6,7 @@
 CONNECTOR_SECUREPORT_FILENAME: Final[str] = "connector.securePort"
 VERSION_INFO_FILE_NAME: Final[str] = "VersionInfo.xml"
 MAX_HTTP_REQUEST_SIZE: Final[int] = 500_000_000  # 500MB
+MATLAB_LOGS_FILE_NAME: Final[str] = "matlab_logs.txt"
 
 # Max startup duration in seconds for processes launched by matlab-proxy
 # This constant is meant for internal use within matlab-proxy

matlab_proxy/settings.py

@@ -150,6 +150,10 @@ def get_mwi_config_folder(dev=False):
         return config_folder_path
 
 
+def get_default_mwi_log_file_path():
+    return get_mwi_config_folder() / constants.MATLAB_LOGS_FILE_NAME
+
+
 def get_mwi_logs_root_dir(dev=False):
     return get_mwi_config_folder(dev) / "ports"
 
@@ -283,6 +287,10 @@ def get_server_settings(config_name):
         os.getenv(mwi_env.get_env_name_ssl_key_file(), None),
         os.getenv(mwi_env.get_env_name_ssl_cert_file(), None),
     )
+
+    # log file validation check is already done in logger.py
+    mwi_log_file = os.getenv(mwi_env.get_env_name_log_file(), None)
+
     return {
         "create_xvfb_cmd": create_xvfb_cmd,
         "base_url": mwi.validators.validate_base_url(
@@ -302,6 +310,7 @@ def get_server_settings(config_name):
         # This directory will be used to store connector.securePort(matlab_ready_file) and its corresponding files. This will be
         # a central place to store logs of all the running instances of MATLAB launched by matlab-proxy
         "mwi_logs_root_dir": get_mwi_logs_root_dir(),
+        "mwi_log_file": mwi_log_file,
         "mw_context_tags": get_mw_context_tags(config_name),
         # The url where the matlab-proxy server is accessible at
         "mwi_server_url": None,

matlab_proxy/util/mwi/logger.py

@@ -4,9 +4,12 @@
 
 import logging
 import os
+import sys
+from pathlib import Path
 
 from . import environment_variables as mwi_env
 
+
 logging.getLogger("aiohttp_session").setLevel(logging.ERROR)
 
 
@@ -47,19 +50,35 @@ def __set_logging_configuration():
         Logger: Logger object with the set configuration.
     """
     # query for user specified environment variables
-    log_level, log_file = __query_environment()
+    log_level = os.getenv(
+        mwi_env.get_env_name_logging_level(), __get_default_log_level()
+    )
+    log_file = os.getenv(mwi_env.get_env_name_log_file(), None)
 
     ## Set logging object
     logger = __get_mw_logger()
-    if log_file is not None:
-        logger.info(f"Initializing logger with log_file:{log_file}")
-        file_handler = logging.FileHandler(filename=log_file)
-        formatter = logging.Formatter(
-            fmt="%(asctime)s - %(name)s - %(levelname)s - %(message)s"
-        )
-        file_handler.setFormatter(formatter)
-        file_handler.setLevel(log_level)
-        logger.addHandler(file_handler)
+    try:
+        if log_file:
+            log_file = Path(log_file)
+            # Need to create the file if it doesn't exist or else logging.FileHandler
+            # would open it in 'write' mode instead of 'append' mode.
+            log_file.touch(exist_ok=True)
+            logger.info(f"Initializing logger with log file:{log_file}")
+            file_handler = logging.FileHandler(filename=log_file, mode="a")
+            formatter = logging.Formatter(
+                fmt="%(asctime)s - %(name)s - %(levelname)s - %(message)s"
+            )
+            file_handler.setFormatter(formatter)
+            file_handler.setLevel(log_level)
+            logger.addHandler(file_handler)
+
+    except PermissionError:
+        print(f"PermissionError: Permission denied to create log file at: {log_file}")
+        sys.exit(1)
+
+    except Exception as err:
+        print(f"Failed to use log file: {log_file} with error: {err}")
+        sys.exit(1)
 
     # log_level is either set by environment or is the default value.
     logger.info(f"Initializing logger with log_level: {log_level}")
@@ -91,16 +110,3 @@ def __get_default_log_level():
         String: The default logging level
     """
     return "INFO"
-
-
-def __query_environment():
-    """Private function to query environment variables
-        to control logging
-
-    Returns:
-        tuple: Log level & Log file as found in the environment
-    """
-    env_var_log_level, env_var_log_file = get_environment_variable_names()
-    log_level = os.environ.get(env_var_log_level, __get_default_log_level())
-    log_file = os.environ.get(env_var_log_file)
-    return log_level, log_file

tests/unit/test_app_state.py

@@ -395,3 +395,35 @@ def test_env_variables_filtration_for_xvfb_process(
 
     # Assert
     assert filtered_env_vars.get(env_var) == is_filtered
+
+
+@pytest.mark.parametrize("platform", [("linux"), ("windows"), ("mac")])
+async def test_setup_env_for_matlab(
+    mocker_os_patching_fixture, platform, app_state_fixture, tmp_path
+):
+    """Test to check MW_DIAGNOSTIC_DEST is set appropriately for posix and non-posix systems
+
+    Args:
+        mocker_os_patching_fixture (mocker): Custom pytest fixture for mocking
+        platform (str): string describing a platform
+        app_state_fixture (AppState): Object of AppState class with defaults set
+        tmp_path (Path): Built-in pytest fixture for temporary paths
+    """
+
+    # Arrange
+    expected_log_file_path = tmp_path / "matlab_logs.txt"
+    app_state_fixture.licensing = {"type": "existing_license"}
+    app_state_fixture.settings = {"mwapikey": None, "matlab_display": ":1"}
+    app_state_fixture.mwi_logs_dir = tmp_path
+    mocker_os_patching_fixture.patch(
+        "matlab_proxy.app_state.logger.isEnabledFor", return_value=True
+    )
+
+    # Act
+    matlab_env = await app_state_fixture._AppState__setup_env_for_matlab()
+
+    # Assert
+    if "linux" or "mac" in platform:
+        assert matlab_env["MW_DIAGNOSTIC_DEST"] == "stdout"
+    else:
+        assert matlab_env["MW_DIAGNOSTIC_DEST"] == expected_log_file_path