From 8a938eefd6f0fc20cc895dbd1e852a313b9b784a Mon Sep 17 00:00:00 2001 From: jchmura Date: Wed, 29 Jul 2026 20:48:21 +0000 Subject: [PATCH 1/6] Add is_env_flag_enabled helper --- gigl/env/constants.py | 6 ++++++ tests/unit/env/constants.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 tests/unit/env/constants.py diff --git a/gigl/env/constants.py b/gigl/env/constants.py index 1396ab670..a1446397b 100644 --- a/gigl/env/constants.py +++ b/gigl/env/constants.py @@ -23,6 +23,7 @@ GIGL_CUDA_DOCKER_URI_ENV_KEY: Final[str] = "GIGL_CUDA_DOCKER_URI" GIGL_COMPONENT_ENV_KEY: Final[str] = "GIGL_COMPONENT" GIGL_BIGQUERY_QUOTA_PROJECT_ENV_KEY: Final[str] = "GIGL_BIGQUERY_QUOTA_PROJECT" +GIGL_DEBUG: Final[str] = "GIGL_DEBUG" def read_resource_config_uri_from_env() -> Optional[str]: @@ -32,3 +33,8 @@ def read_resource_config_uri_from_env() -> Optional[str]: The URI string if the variable is set, else ``None``. """ return os.environ.get(GIGL_RESOURCE_CONFIG_URI_ENV_KEY) + + +def is_env_flag_enabled(env_var_name: str) -> bool: + """Parses an environment variable and returns True iff the variable is set to '1' or 'True' (case-insensitive).""" + return os.environ.get(env_var_name, "").strip().lower() in ("1", "true") diff --git a/tests/unit/env/constants.py b/tests/unit/env/constants.py new file mode 100644 index 000000000..d138b184a --- /dev/null +++ b/tests/unit/env/constants.py @@ -0,0 +1,32 @@ +import os +from unittest.mock import patch + +from gigl.env.constants import is_env_flag_enabled +from tests.test_assets.test_case import TestCase + + +class EnvFlagEnabledTest(TestCase): + def test_env_flag_disabled(self): + env_var = "TEST_ENV_VAR" + with patch.dict(os.environ, {}): + self.assertFalse(is_env_flag_enabled(env_var)) + + def test_env_flag_enabled_1(self): + env_var = "TEST_ENV_VAR" + with patch.dict(os.environ, {env_var: "1"}): + self.assertTrue(is_env_flag_enabled(env_var)) + + def test_env_flag_enabled_True(self): + env_var = "TEST_ENV_VAR" + with patch.dict(os.environ, {env_var: "True"}): + self.assertTrue(is_env_flag_enabled(env_var)) + + def test_env_flag_enabled_true(self): + env_var = "TEST_ENV_VAR" + with patch.dict(os.environ, {env_var: "true"}): + self.assertTrue(is_env_flag_enabled(env_var)) + + def test_env_flag_enabled_true_with_whitespace(self): + env_var = "TEST_ENV_VAR" + with patch.dict(os.environ, {env_var: " true "}): + self.assertTrue(is_env_flag_enabled(env_var)) From a0ccf3b9738531e5cffad1309d1c934bb8b9415e Mon Sep 17 00:00:00 2001 From: jchmura Date: Wed, 29 Jul 2026 20:48:35 +0000 Subject: [PATCH 2/6] Use GIGL_DEBUG to optionally set log level to logging.DEBUG --- gigl/common/logger.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/gigl/common/logger.py b/gigl/common/logger.py index 3dd98bb81..a8ffa8b41 100644 --- a/gigl/common/logger.py +++ b/gigl/common/logger.py @@ -6,6 +6,8 @@ from google.cloud import logging as google_cloud_logging +from gigl.env.constants import GIGL_DEBUG, is_env_flag_enabled + _BASE_LOG_FILE_PATH = "/tmp/research/gbml/logs" @@ -37,12 +39,17 @@ def _setup_logger( ) -> None: handler: logging.Handler if not logger.handlers: + if is_env_flag_enabled(GIGL_DEBUG): + log_level = logging.DEBUG + else: + log_level = logging.INFO + if os.getenv("GAE_APPLICATION") or os.environ.get( "KUBERNETES_SERVICE_HOST" ): # Google Cloud Logging client = google_cloud_logging.Client() - client.setup_logging(log_level=logging.INFO) + client.setup_logging(log_level=log_level) else: # Logging locally. Set up logging to console or file if log_to_file: @@ -61,7 +68,7 @@ def _setup_logger( ) handler.setFormatter(formatter) logger.addHandler(handler) - logger.setLevel(logging.INFO) + logger.setLevel(log_level) def process(self, msg: str, kwargs: MutableMapping[str, Any]) -> Any: if "extra" in kwargs: From 1b46d6c2577c0b7c21d2c8c4ae786911be27a9cc Mon Sep 17 00:00:00 2001 From: jchmura Date: Thu, 30 Jul 2026 16:07:11 +0000 Subject: [PATCH 3/6] Record log level to user --- gigl/common/logger.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/gigl/common/logger.py b/gigl/common/logger.py index a8ffa8b41..67184b7e2 100644 --- a/gigl/common/logger.py +++ b/gigl/common/logger.py @@ -21,6 +21,8 @@ class Logger(logging.LoggerAdapter): extra (Optional[dict[str, Any]]): Extra information to be added to the log message. """ + _DID_ALERT_FOR_LOG_LEVEL: bool = False + def __init__( self, logger: Optional[logging.Logger] = None, @@ -28,22 +30,32 @@ def __init__( log_to_file: bool = False, extra: Optional[dict[str, Any]] = None, ): + gigl_debug = is_env_flag_enabled(GIGL_DEBUG) + if gigl_debug: + log_level = logging.DEBUG + else: + log_level = logging.INFO + if logger is None: logger = logging.getLogger(name) - self._setup_logger(logger, name, log_to_file) + self._setup_logger(logger, name, log_to_file, log_level) super().__init__(logger, extra or {}) + if not Logger._DID_ALERT_FOR_LOG_LEVEL: + Logger._DID_ALERT_FOR_LOG_LEVEL = True + level_name = logging.getLevelName(log_level) + self.info(f"{GIGL_DEBUG}={gigl_debug}, using log level {level_name}") + def _setup_logger( - self, logger: logging.Logger, name: Optional[str], log_to_file: bool + self, + logger: logging.Logger, + name: Optional[str], + log_to_file: bool, + log_level: int, ) -> None: handler: logging.Handler if not logger.handlers: - if is_env_flag_enabled(GIGL_DEBUG): - log_level = logging.DEBUG - else: - log_level = logging.INFO - if os.getenv("GAE_APPLICATION") or os.environ.get( "KUBERNETES_SERVICE_HOST" ): From de8060a8602d15e9ed2a1b599b9ac5a531750b18 Mon Sep 17 00:00:00 2001 From: kmontemayor Date: Thu, 30 Jul 2026 17:32:47 +0000 Subject: [PATCH 4/6] Add GIGL_DISABLE_CLOUD_LOGGING to opt out of GCP JSON log lines On App Engine and Kubernetes, Logger routes records through google.cloud.logging, which on GKE installs StructuredLogHandler -- a handler that renders every record as a single-line GCP JSON envelope and hardcodes that format, so setFormatter cannot change it. Under Ray that envelope is a pure loss. Ray relays worker output to the driver behind a "(RayTrainWorker pid=...)" prefix, which makes the line invalid JSON, so Cloud Logging stores it as a plain textPayload and drops severity, sourceLocation, and trace correlation anyway. The reader is left with an unreadable line and none of the structured fields it paid for. It bites hardest in mp.spawn children. Ray Train configures the worker root logger with a message-only handler, so the parent process is already readable, but a spawned child starts a fresh interpreter with no handlers -- Logger's setup runs there and installs the JSON handler. The knob is read from the environment rather than passed as an argument so that spawned children, which inherit os.environ but not logging config, pick it up without every spawn entrypoint having to opt in. Also renames test_logger.py to logger_test.py: the unit-test runner's default pattern is *_test.py, so these tests were never running. Co-Authored-By: Claude Opus 5 (1M context) --- gigl/common/logger.py | 37 ++++++++++++++-- gigl/env/constants.py | 1 + tests/unit/common/logger_test.py | 76 ++++++++++++++++++++++++++++++++ tests/unit/common/test_logger.py | 38 ---------------- 4 files changed, 111 insertions(+), 41 deletions(-) create mode 100644 tests/unit/common/logger_test.py delete mode 100644 tests/unit/common/test_logger.py diff --git a/gigl/common/logger.py b/gigl/common/logger.py index 3dd98bb81..1056488be 100644 --- a/gigl/common/logger.py +++ b/gigl/common/logger.py @@ -6,12 +6,41 @@ from google.cloud import logging as google_cloud_logging +from gigl.env.constants import GIGL_DISABLE_CLOUD_LOGGING_ENV_KEY + _BASE_LOG_FILE_PATH = "/tmp/research/gbml/logs" +# Values of GIGL_DISABLE_CLOUD_LOGGING that leave cloud logging on, so that setting +# the variable to "0" reads as "off" rather than as any-value-means-on. +_FALSY_ENV_VALUES = frozenset({"", "0", "false"}) + + +def _is_cloud_logging_disabled() -> bool: + """Whether GIGL_DISABLE_CLOUD_LOGGING opts this process out of cloud logging. + + On GKE, ``google.cloud.logging`` attaches a handler that renders every record as + a single-line GCP JSON envelope. Under Ray that envelope is unreadable and its + structured fields are dropped anyway: Ray prefixes each relayed worker line with + ``(RayTrainWorker pid=...)``, which makes the line invalid JSON, so Cloud Logging + stores it as a plain ``textPayload``. Set this variable on such processes to get + the console format instead. + + Returns: + True when the variable is set to anything other than "", "0", or "false" + (case-insensitive). + """ + value = os.environ.get(GIGL_DISABLE_CLOUD_LOGGING_ENV_KEY, "") + return value.lower() not in _FALSY_ENV_VALUES + class Logger(logging.LoggerAdapter): """ GiGL's custom logger class used for local and cloud logging (VertexAI, Dataflow, etc.) + + On App Engine and Kubernetes, records are routed to Google Cloud Logging, which + renders them as GCP JSON. Set ``GIGL_DISABLE_CLOUD_LOGGING`` to fall back to the + console format -- see :func:`_is_cloud_logging_disabled`. + Args: logger (Optional[logging.Logger]): A custom logger to use. If not provided, the default logger will be created. name (Optional[str]): The name to be used for the logger. By default uses "root". @@ -37,9 +66,11 @@ def _setup_logger( ) -> None: handler: logging.Handler if not logger.handlers: - if os.getenv("GAE_APPLICATION") or os.environ.get( - "KUBERNETES_SERVICE_HOST" - ): + is_cloud_environment = bool( + os.getenv("GAE_APPLICATION") + or os.environ.get("KUBERNETES_SERVICE_HOST") + ) + if is_cloud_environment and not _is_cloud_logging_disabled(): # Google Cloud Logging client = google_cloud_logging.Client() client.setup_logging(log_level=logging.INFO) diff --git a/gigl/env/constants.py b/gigl/env/constants.py index 1396ab670..e969ac00a 100644 --- a/gigl/env/constants.py +++ b/gigl/env/constants.py @@ -23,6 +23,7 @@ GIGL_CUDA_DOCKER_URI_ENV_KEY: Final[str] = "GIGL_CUDA_DOCKER_URI" GIGL_COMPONENT_ENV_KEY: Final[str] = "GIGL_COMPONENT" GIGL_BIGQUERY_QUOTA_PROJECT_ENV_KEY: Final[str] = "GIGL_BIGQUERY_QUOTA_PROJECT" +GIGL_DISABLE_CLOUD_LOGGING_ENV_KEY: Final[str] = "GIGL_DISABLE_CLOUD_LOGGING" def read_resource_config_uri_from_env() -> Optional[str]: diff --git a/tests/unit/common/logger_test.py b/tests/unit/common/logger_test.py new file mode 100644 index 000000000..2ff5897bc --- /dev/null +++ b/tests/unit/common/logger_test.py @@ -0,0 +1,76 @@ +import copy +import logging +import os +import pickle +from unittest import mock + +from absl.testing import absltest + +from gigl.common.logger import Logger, _is_cloud_logging_disabled +from tests.test_assets.test_case import TestCase + + +class TestLogger(TestCase): + def test_undefined_attribute_raises_attribute_error(self) -> None: + # __getattr__ must surface a clean AttributeError for unknown attributes + # rather than recursing forever (the wrapped logger is stored as + # ``self.logger``, not ``self._logger``). + logger = Logger(name="test_undefined_attr") + with self.assertRaises(AttributeError): + logger.this_attribute_does_not_exist + + def test_delegates_to_wrapped_logger(self) -> None: + logger = Logger(name="test_delegation") + # ``level`` is defined on the wrapped logging.Logger, reached via __getattr__. + self.assertEqual(logger.level, logger.logger.level) + + def test_deepcopy_round_trips(self) -> None: + logger = Logger(name="test_deepcopy") + clone = copy.deepcopy(logger) + self.assertIsInstance(clone, Logger) + clone.info("deepcopy works") + + def test_pickle_round_trips(self) -> None: + logger = Logger(name="test_pickle") + clone = pickle.loads(pickle.dumps(logger)) + self.assertIsInstance(clone, Logger) + clone.info("pickle works") + + def test_disabling_cloud_logging_uses_console_handler(self) -> None: + # KUBERNETES_SERVICE_HOST alone would send records to Google Cloud Logging as + # GCP JSON; GIGL_DISABLE_CLOUD_LOGGING must win and give the console format. + with mock.patch.dict( + os.environ, + { + "KUBERNETES_SERVICE_HOST": "10.0.0.1", + "GIGL_DISABLE_CLOUD_LOGGING": "1", + }, + ): + logger = Logger(name="test_cloud_logging_disabled") + + handlers = logger.logger.handlers + self.assertEqual(len(handlers), 1) + self.assertIsInstance(handlers[0], logging.StreamHandler) + record = logging.LogRecord( + name="test_cloud_logging_disabled", + level=logging.INFO, + pathname="export.py", + lineno=197, + msg="upload took 8.82 seconds", + args=None, + exc_info=None, + func="_flush", + ) + formatted = handlers[0].format(record) + self.assertIn("[INFO] : upload took 8.82 seconds", formatted) + self.assertIn("(export.py:_flush:197)", formatted) + + def test_cloud_logging_stays_enabled_for_falsy_values(self) -> None: + for value in ("", "0", "false", "False"): + with self.subTest(value=value): + with mock.patch.dict(os.environ, {"GIGL_DISABLE_CLOUD_LOGGING": value}): + self.assertFalse(_is_cloud_logging_disabled()) + + +if __name__ == "__main__": + absltest.main() diff --git a/tests/unit/common/test_logger.py b/tests/unit/common/test_logger.py deleted file mode 100644 index 2ff4a7de9..000000000 --- a/tests/unit/common/test_logger.py +++ /dev/null @@ -1,38 +0,0 @@ -import copy -import pickle - -from absl.testing import absltest - -from gigl.common.logger import Logger -from tests.test_assets.test_case import TestCase - - -class TestLogger(TestCase): - def test_undefined_attribute_raises_attribute_error(self) -> None: - # __getattr__ must surface a clean AttributeError for unknown attributes - # rather than recursing forever (the wrapped logger is stored as - # ``self.logger``, not ``self._logger``). - logger = Logger(name="test_undefined_attr") - with self.assertRaises(AttributeError): - logger.this_attribute_does_not_exist - - def test_delegates_to_wrapped_logger(self) -> None: - logger = Logger(name="test_delegation") - # ``level`` is defined on the wrapped logging.Logger, reached via __getattr__. - self.assertEqual(logger.level, logger.logger.level) - - def test_deepcopy_round_trips(self) -> None: - logger = Logger(name="test_deepcopy") - clone = copy.deepcopy(logger) - self.assertIsInstance(clone, Logger) - clone.info("deepcopy works") - - def test_pickle_round_trips(self) -> None: - logger = Logger(name="test_pickle") - clone = pickle.loads(pickle.dumps(logger)) - self.assertIsInstance(clone, Logger) - clone.info("pickle works") - - -if __name__ == "__main__": - absltest.main() From 43fbffaa80225c1e59243b9375928e8b7af02211 Mon Sep 17 00:00:00 2001 From: kmontemayor Date: Thu, 30 Jul 2026 18:08:09 +0000 Subject: [PATCH 5/6] Document GIGL_DISABLE_CLOUD_LOGGING in the cloud setup guide Co-Authored-By: Claude Opus 5 (1M context) --- .../getting_started/cloud_setup_guide.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/user_guide/getting_started/cloud_setup_guide.md b/docs/user_guide/getting_started/cloud_setup_guide.md index 0a3f420dd..6b4c31fb5 100644 --- a/docs/user_guide/getting_started/cloud_setup_guide.md +++ b/docs/user_guide/getting_started/cloud_setup_guide.md @@ -143,6 +143,24 @@ gcloud storage buckets add-iam-policy-binding $BUCKET_NAME \ 11. Give your SA `roles/bigquery.dataOwner` on the datasets you created. See [instructions](https://cloud.google.com/bigquery/docs/control-access-to-resources-iam#bq_2). +### Console log format + +On App Engine and Kubernetes, `gigl.common.logger.Logger` routes records to Google Cloud Logging, which on GKE renders +each one as a single-line JSON envelope. To get the plain console format instead, set: + +```bash +export GIGL_DISABLE_CLOUD_LOGGING="1" +``` + +Any value other than an empty string, `0`, or `false` disables cloud logging. + +This is worth setting on processes whose stdout is relayed by another system before it reaches Cloud Logging. Ray is the +motivating case: it prefixes each worker line it forwards to the driver with `(RayTrainWorker pid=...)`, which makes the +line invalid JSON, so Cloud Logging stores it as a plain `textPayload` and the structured fields (`severity`, +`sourceLocation`, trace correlation) are dropped anyway. The variable is read from the environment so that processes +started with the `spawn` method — which inherit `os.environ` but not the parent's logging configuration — pick it up +without each entry point having to opt in. + ## AWS Project Setup Guide - Not yet supported From 31d2cd242b47e18a3eaad8e28eeca84734300fbd Mon Sep 17 00:00:00 2001 From: Kyle Montemayor Date: Fri, 31 Jul 2026 10:31:58 -0700 Subject: [PATCH 6/6] Update logger.py --- gigl/common/logger.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/gigl/common/logger.py b/gigl/common/logger.py index d997039b9..110eefb72 100644 --- a/gigl/common/logger.py +++ b/gigl/common/logger.py @@ -69,12 +69,6 @@ def _setup_logger( os.getenv("GAE_APPLICATION") or os.environ.get("KUBERNETES_SERVICE_HOST") ) - # Cloud Logging's handler renders each record as a single-line GCP JSON - # envelope, which is a loss wherever another system reframes the line before - # Cloud Logging sees it: Ray relays worker output behind a - # "(RayTrainWorker pid=...)" prefix that makes the line invalid JSON, so it - # is stored as a plain textPayload and the structured fields are dropped - # anyway. GIGL_DISABLE_CLOUD_LOGGING selects the console format instead. if is_cloud_environment and not is_env_flag_enabled( GIGL_DISABLE_CLOUD_LOGGING_ENV_KEY ):