Repository navigation
[core] Keep absl logging at ERROR unless GRPC_VERBOSITY is set - #67719
Conversation
gRPC >= 1.65 logs through absl, and when GRPC_VERBOSITY is unset it leaves absl's minimum log level at absl's INFO default. With gRPC 1.69 that sends gRPC INFO messages (e.g. chttp2 'Got goaway') to the raylet's stderr, which Ray forwards to the driver; tune's test_progress_reporter fails on them whenever a GOAWAY happens during the test. Older gRPC defaulted to ERROR. Set absl's minimum level to ERROR in StartRayLog (raylet, GCS, core worker) unless the user sets GRPC_VERBOSITY. Ray's own logs go through spdlog and are unaffected. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request configures absl's minimum log level to default to kError when GRPC_VERBOSITY is unset or empty, preventing verbose gRPC INFO logs from flooding stderr. A corresponding unit test was added to verify this behavior. The reviewer noted that the new test is environment-dependent and could fail if GRPC_VERBOSITY is set in the test runner's environment, suggesting to skip the test in such cases.
| TEST(PrintLogTest, TestAbslMinLogLevelDefaultsToError) { | ||
| ASSERT_EQ(std::getenv("GRPC_VERBOSITY"), nullptr); | ||
| RayLog::StartRayLog("/tmp/gcs", RayLogLevel::INFO, /*log_filepath=*/""); | ||
| EXPECT_EQ(absl::MinLogLevel(), absl::LogSeverityAtLeast::kError); | ||
| RayLog::ShutDownRayLog(); | ||
| } |
There was a problem hiding this comment.
The test TestAbslMinLogLevelDefaultsToError is environment-dependent and will fail if the GRPC_VERBOSITY environment variable is set (even to an empty string) in the test runner's environment. To make the test robust and prevent flaky failures in local development or CI environments, we should skip the assertion or the test when GRPC_VERBOSITY is present.
TEST(PrintLogTest, TestAbslMinLogLevelDefaultsToError) {
if (std::getenv("GRPC_VERBOSITY") != nullptr) {
SUCCEED() << "Skipping test because GRPC_VERBOSITY is set in the environment.";
return;
}
RayLog::StartRayLog("/tmp/gcs", RayLogLevel::INFO, /*log_filepath=*/"");
EXPECT_EQ(absl::MinLogLevel(), absl::LogSeverityAtLeast::kError);
RayLog::ShutDownRayLog();
}
|
LGTM! |
Description
gRPC >= 1.65 logs through absl. When
GRPC_VERBOSITYis unset, gRPC leaves absl's minimum log level alone, and absl defaults to INFO. gRPC INFO messages (e.g. "Got goaway") then reach stderr and get forwarded to the driver. Older gRPC defaulted to ERROR.RayLog::StartRayLognow sets absl's minimum log level to ERROR once per process, unlessGRPC_VERBOSITYis set. Ray's own logging goes through spdlog and is unaffected.Note for reviewers: protobuf (23.4 today) also logs through absl, so landing this before the gRPC bump also hides protobuf's absl INFO and WARNING output. Master ends up in that state after the bump anyway.
Related issues
Split out of #66483 (protobuf 29 / gRPC 1.69) to make that PR easier to review.
🤖 Generated with Claude Code