From 56746e6f0f8948c294762ce555228f5e11c38635 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 9 Aug 2017 15:30:24 +0100 Subject: [PATCH 1/2] don't capture logs directly from stdout previously in run_app_paas.sh, we captured stdout from the app and piped that into the log file. However, this came up with a bunch of problems, mainly that exceptions with stack traces often weren't formatted properly, and kibana could not parse them instead, with the updated utils library, we can use that to log json straight to the appropriate directory directly. --- app/cloudfoundry_config.py | 2 +- app/config.py | 4 ++-- requirements.txt | 2 +- scripts/run_app_paas.sh | 7 ++----- tests/app/test_cloudfoundry_config.py | 2 +- tests/app/test_config.py | 21 --------------------- 6 files changed, 7 insertions(+), 31 deletions(-) diff --git a/app/cloudfoundry_config.py b/app/cloudfoundry_config.py index 5ad48d854..d04b3eb5d 100644 --- a/app/cloudfoundry_config.py +++ b/app/cloudfoundry_config.py @@ -16,7 +16,7 @@ def extract_cloudfoundry_config(): def set_config_env_vars(vcap_services): vcap_application = json.loads(os.environ.get('VCAP_APPLICATION')) os.environ['NOTIFY_ENVIRONMENT'] = vcap_application['space_name'] - os.environ['LOGGING_STDOUT_JSON'] = '1' + os.environ['NOTIFY_LOG_PATH'] = '/home/vcap/logs/app.log' for s in vcap_services['user-provided']: if s['name'] == 'notify-config': diff --git a/app/config.py b/app/config.py index 4232e2411..1fc80ceea 100644 --- a/app/config.py +++ b/app/config.py @@ -28,7 +28,6 @@ class Config(object): # Logging DEBUG = False - LOGGING_STDOUT_JSON = os.getenv('LOGGING_STDOUT_JSON') == '1' DESKPRO_DEPT_ID = 5 DESKPRO_ASSIGNED_AGENT_TEAM_ID = 5 @@ -43,7 +42,7 @@ class Config(object): MAX_FAILED_LOGIN_COUNT = 10 NOTIFY_APP_NAME = 'admin' NOTIFY_LOG_LEVEL = 'DEBUG' - NOTIFY_LOG_PATH = '/var/log/notify/application.log' + NOTIFY_LOG_PATH = None PERMANENT_SESSION_LIFETIME = 20 * 60 * 60 # 20 hours SEND_FILE_MAX_AGE_DEFAULT = 365 * 24 * 60 * 60 # 1 year SESSION_COOKIE_HTTPONLY = True @@ -91,6 +90,7 @@ class Config(object): class Development(Config): + NOTIFY_LOG_PATH = 'application.log' DEBUG = True SESSION_COOKIE_SECURE = False SESSION_PROTECTION = None diff --git a/requirements.txt b/requirements.txt index db12cedcd..4dd5fd3dc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,4 +26,4 @@ notifications-python-client==4.3.1 awscli>=1.11,<1.12 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@17.8.0#egg=notifications-utils==17.8.0 +git+https://github.com/alphagov/notifications-utils.git@18.0.0#egg=notifications-utils==18.0.0 diff --git a/scripts/run_app_paas.sh b/scripts/run_app_paas.sh index 2ac52c388..8e0bf6ee8 100755 --- a/scripts/run_app_paas.sh +++ b/scripts/run_app_paas.sh @@ -52,11 +52,9 @@ function on_exit { kill 0 } -function start_appplication { - exec "$@" 2>&1 | while read line; do echo $line; echo $line >> /home/vcap/logs/app.log.`date +%Y-%m-%d`; done & - LOGGER_PID=$! +function start_application { + exec "$@" & APP_PID=`jobs -p` - echo "Logger process pid: ${LOGGER_PID}" echo "Application process pid: ${APP_PID}" } @@ -69,7 +67,6 @@ function start_aws_logs_agent { function run { while true; do kill -0 ${APP_PID} 2&>/dev/null || break - kill -0 ${LOGGER_PID} 2&>/dev/null || break kill -0 ${AWSLOGS_AGENT_PID} 2&>/dev/null || start_aws_logs_agent sleep 1 done diff --git a/tests/app/test_cloudfoundry_config.py b/tests/app/test_cloudfoundry_config.py index a3a8ab63a..81852d7bf 100644 --- a/tests/app/test_cloudfoundry_config.py +++ b/tests/app/test_cloudfoundry_config.py @@ -92,8 +92,8 @@ def cloudfoundry_environ(monkeypatch, cloudfoundry_config): def test_extract_cloudfoundry_config_populates_other_vars(): extract_cloudfoundry_config() - assert os.environ['LOGGING_STDOUT_JSON'] == '1' assert os.environ['NOTIFY_ENVIRONMENT'] == '🚀🌌' + assert os.environ['NOTIFY_LOG_PATH'] == '/home/vcap/logs/app.log' @pytest.mark.usefixtures('os_environ', 'cloudfoundry_environ') diff --git a/tests/app/test_config.py b/tests/app/test_config.py index eb4dd1e5f..87cc10aed 100644 --- a/tests/app/test_config.py +++ b/tests/app/test_config.py @@ -51,24 +51,3 @@ def test_load_config_if_cloudfoundry_not_available(monkeypatch, reload_config): assert os.environ['API_HOST_NAME'] == 'env' assert config.Config.API_HOST_NAME == 'env' - - -def test_logging_stdout_json_defaults_to_off(reload_config): - os.environ.pop('LOGGING_STDOUT_JSON', None) - assert config.Config.LOGGING_STDOUT_JSON is False - - -def test_logging_stdout_json_sets_to_off_if_not_recognised(reload_config): - os.environ['LOGGING_STDOUT_JSON'] = 'foo' - - importlib.reload(config) - - assert config.Config.LOGGING_STDOUT_JSON is False - - -def test_logging_stdout_json_sets_to_on_if_set_to_1(reload_config): - os.environ['LOGGING_STDOUT_JSON'] = '1' - - importlib.reload(config) - - assert config.Config.LOGGING_STDOUT_JSON is True From bf1f68fd55410566a3fbe3985126beb9371efe1b Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 9 Aug 2017 16:53:56 +0100 Subject: [PATCH 2/2] make sure log path is set --- app/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/config.py b/app/config.py index 1fc80ceea..1f9ea09cd 100644 --- a/app/config.py +++ b/app/config.py @@ -28,6 +28,7 @@ class Config(object): # Logging DEBUG = False + NOTIFY_LOG_PATH = os.getenv('NOTIFY_LOG_PATH') DESKPRO_DEPT_ID = 5 DESKPRO_ASSIGNED_AGENT_TEAM_ID = 5 @@ -42,7 +43,6 @@ class Config(object): MAX_FAILED_LOGIN_COUNT = 10 NOTIFY_APP_NAME = 'admin' NOTIFY_LOG_LEVEL = 'DEBUG' - NOTIFY_LOG_PATH = None PERMANENT_SESSION_LIFETIME = 20 * 60 * 60 # 20 hours SEND_FILE_MAX_AGE_DEFAULT = 365 * 24 * 60 * 60 # 1 year SESSION_COOKIE_HTTPONLY = True