Skip to content

Commit 1aeb222

Browse files
Simplified the logic
Made all logs go to the container Used logger statements instead of stderr
1 parent 74d3198 commit 1aeb222

7 files changed

Lines changed: 14 additions & 57 deletions

File tree

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,9 @@
1-
import logging
2-
3-
from celery import Celery, signals
1+
from celery import Celery
42

53
from job_orchestration.executor.compress import celeryconfig
6-
from job_orchestration.executor.utils import add_container_log_handler
74

85
app = Celery("compress")
96
app.config_from_object(celeryconfig)
107

11-
12-
@signals.after_setup_logger.connect
13-
def _on_after_setup_logger(logger: logging.Logger, **_kwargs: object) -> None:
14-
add_container_log_handler(logger)
15-
16-
178
if "__main__" == __name__:
189
app.start()

components/job-orchestration/job_orchestration/executor/compress/compression_task.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import pathlib
55
import shutil
66
import subprocess
7-
import sys
87
from contextlib import closing
98
from typing import Any
109

@@ -560,12 +559,11 @@ def cleanup_temporary_files():
560559
finally:
561560
cleanup_temporary_files()
562561
stderr_log_file.close()
563-
# Dump the stderr log to sys.stderr so it's visible in container logs
562+
# Dump the stderr log so it's visible in container logs
564563
if stderr_log_path.stat().st_size > 0:
565-
sys.stderr.write(
566-
f"--- Contents of {stderr_log_path.name} ---\n"
564+
logger.error(
565+
f"Contents of {stderr_log_path.name}:\n"
567566
f"{stderr_log_path.read_text()}"
568-
f"--- End of {stderr_log_path.name} ---\n"
569567
)
570568

571569
worker_output = {
Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,9 @@
1-
import logging
2-
3-
from celery import Celery, signals
1+
from celery import Celery
42

53
from job_orchestration.executor.query import celeryconfig
6-
from job_orchestration.executor.utils import add_container_log_handler
74

85
app = Celery("query")
96
app.config_from_object(celeryconfig)
107

11-
12-
@signals.after_setup_logger.connect
13-
def _on_after_setup_logger(logger: logging.Logger, **_kwargs: object) -> None:
14-
add_container_log_handler(logger)
15-
16-
178
if "__main__" == __name__:
189
app.start()

components/job-orchestration/job_orchestration/executor/query/utils.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,11 @@ def sigterm_handler(_signo, _stack_frame):
8989
stdout_data, _ = task_proc.communicate()
9090
return_code = task_proc.returncode
9191

92-
# Dump the stderr log to sys.stderr so it's visible in container logs
92+
# Dump the stderr log so it's visible in container logs
9393
if clo_log_path.stat().st_size > 0:
94-
sys.stderr.write(
95-
f"--- Contents of {clo_log_path.name} ---\n"
94+
logger.error(
95+
f"Contents of {clo_log_path.name}:\n"
9696
f"{clo_log_path.read_text()}"
97-
f"--- End of {clo_log_path.name} ---\n"
9897
)
9998

10099
if 0 != return_code:

components/job-orchestration/job_orchestration/executor/utils.py

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,13 @@
11
"""Utilities shared by job-orchestration executors."""
22

3-
import logging
43
import os
54
from logging import Logger
65
from pathlib import Path
76

8-
from celery.app.defaults import DEFAULT_PROCESS_LOG_FMT
97
from clp_py_utils.clp_config import ClpConfig, WorkerConfig
108
from clp_py_utils.core import read_yaml_config_file
119

1210

13-
def add_container_log_handler(logger: logging.Logger) -> None:
14-
"""
15-
Add a StreamHandler that writes to the container's real stderr (fd 2).
16-
17-
In Celery workers, ``-f`` redirects ``sys.stdout`` and ``sys.stderr`` to
18-
the log file, so regular ``StreamHandler(sys.stderr)`` would write to the
19-
file instead of the container's stderr. This handler writes directly to
20-
file descriptor 2, ensuring output reaches ``docker compose logs`` /
21-
``kubectl logs``.
22-
"""
23-
# dup(2) so the handler owns its own fd and doesn't close the original
24-
stderr_stream = os.fdopen(os.dup(2), "w")
25-
handler = logging.StreamHandler(stderr_stream)
26-
handler.setFormatter(logging.Formatter(DEFAULT_PROCESS_LOG_FMT))
27-
logger.addHandler(handler)
28-
29-
3011
def load_clp_config_from_config_path_env_var() -> ClpConfig:
3112
"""
3213
Loads the CLP config from `CLP_CONFIG_PATH`.

components/job-orchestration/job_orchestration/reducer/reducer.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,12 @@ def main(argv: list[str]) -> int:
9595
logger.info(f"reducer-{i} exited with returncode={reducer.returncode}")
9696
for i, reducer_log_file in enumerate(reducer_log_files):
9797
reducer_log_file.close()
98-
# Dump the reducer log to sys.stderr so it's visible in container logs
98+
# Dump the reducer log so it's visible in container logs
9999
log_file_path = logs_dir / f"reducer-{i}.log"
100100
if log_file_path.stat().st_size > 0:
101-
sys.stderr.write(
102-
f"--- Contents of {log_file_path.name} ---\n"
101+
logger.info(
102+
f"Contents of {log_file_path.name}:\n"
103103
f"{log_file_path.read_text()}"
104-
f"--- End of {log_file_path.name} ---\n"
105104
)
106105

107106
logger.error("All reducers terminated")

tools/deployment/package/docker-compose-all.yaml

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ services:
311311
CLP_HOME: "/opt/clp"
312312
CLP_LOGGING_LEVEL: "${CLP_COMPRESSION_WORKER_LOGGING_LEVEL:-INFO}"
313313
CLP_LOGS_DIR: "/var/log/compression_worker"
314-
CLP_WORKER_LOG_PATH: "/var/log/compression_worker/worker.log"
314+
CLP_WORKER_LOG_PATH: "/dev/stdout"
315315
PYTHONPATH: "/opt/clp/lib/python3/site-packages"
316316
RESULT_BACKEND: "redis://default:${CLP_REDIS_PASS:?Please set a value.}\
317317
@redis:${CLP_REDIS_CONNECT_PORT:-6379}\
@@ -332,7 +332,6 @@ services:
332332
"worker",
333333
"--concurrency", "${CLP_COMPRESSION_WORKER_CONCURRENCY:-1}",
334334
"--loglevel", "WARNING",
335-
"-f", "/var/log/compression_worker/worker.log",
336335
"-Q", "compression",
337336
"-n", "compression-worker@%h"
338337
]
@@ -345,7 +344,7 @@ services:
345344
CLP_HOME: "/opt/clp"
346345
CLP_LOGGING_LEVEL: "${CLP_COMPRESSION_WORKER_LOGGING_LEVEL:-INFO}"
347346
CLP_LOGS_DIR: "/var/log/compression_worker"
348-
CLP_WORKER_LOG_PATH: "/var/log/compression_worker/worker.log"
347+
CLP_WORKER_LOG_PATH: "/dev/stdout"
349348
PYTHONPATH: "/opt/clp/lib/python3/site-packages"
350349
SPIDER_LOG_DIR: "/var/log/compression_worker"
351350
volumes:
@@ -499,7 +498,7 @@ services:
499498
CLP_HOME: "/opt/clp"
500499
CLP_LOGGING_LEVEL: "${CLP_QUERY_WORKER_LOGGING_LEVEL:-INFO}"
501500
CLP_LOGS_DIR: "/var/log/query_worker"
502-
CLP_WORKER_LOG_PATH: "/var/log/query_worker/worker.log"
501+
CLP_WORKER_LOG_PATH: "/dev/stdout"
503502
PYTHONPATH: "/opt/clp/lib/python3/site-packages"
504503
RESULT_BACKEND: "redis://default:${CLP_REDIS_PASS:?Please set a value.}\
505504
@redis:${CLP_REDIS_CONNECT_PORT:-6379}\
@@ -519,7 +518,6 @@ services:
519518
"worker",
520519
"--concurrency", "${CLP_QUERY_WORKER_CONCURRENCY:-1}",
521520
"--loglevel", "WARNING",
522-
"-f", "/var/log/query_worker/worker.log",
523521
"-Q", "query",
524522
"-n", "query-worker@%h"
525523
]

0 commit comments

Comments
 (0)