Skip to content

Add GIGL_DISABLE_CLOUD_LOGGING to opt out of GCP JSON log lines - #727

Merged
kmontemayor2-sc merged 10 commits into
mainfrom
kmonte/plain_console_logging_knob
Jul 31, 2026
Merged

Add GIGL_DISABLE_CLOUD_LOGGING to opt out of GCP JSON log lines#727
kmontemayor2-sc merged 10 commits into
mainfrom
kmonte/plain_console_logging_knob

Conversation

@kmontemayor2-sc

Copy link
Copy Markdown
Collaborator

Add opt out of cloud logging. We've found that not all envs work nicely with structured logging

jchmura-sc and others added 10 commits July 29, 2026 20:48
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) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…logging_knob

# Conflicts:
#	gigl/env/constants.py
Reuse is_env_flag_enabled for the GIGL_DISABLE_CLOUD_LOGGING gate rather than
carrying a second env-var parser, and drop the local falsy-value test now that
the helper owns that parsing. Narrows the accepted spellings to "1" and "true",
which the setup guide now states.

@yliu2-sc yliu2-sc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kmontemayor2-sc
kmontemayor2-sc marked this pull request as ready for review July 31, 2026 17:42
@kmontemayor2-sc
kmontemayor2-sc added this pull request to the merge queue Jul 31, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 31, 2026
@kmontemayor2-sc
kmontemayor2-sc added this pull request to the merge queue Jul 31, 2026
@kmontemayor2-sc
kmontemayor2-sc removed this pull request from the merge queue due to a manual request Jul 31, 2026
@kmontemayor2-sc
kmontemayor2-sc added this pull request to the merge queue Jul 31, 2026
Merged via the queue into main with commit 322d351 Jul 31, 2026
13 checks passed
@kmontemayor2-sc
kmontemayor2-sc deleted the kmonte/plain_console_logging_knob branch July 31, 2026 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants