From 3c0b99a19e3773b8441bffe2891113568202f9d8 Mon Sep 17 00:00:00 2001 From: George Weale Date: Mon, 13 Oct 2025 14:56:53 -0700 Subject: [PATCH] fix: rollback of add structured JSON logging with Cloud Trace correlation Close #1683 PiperOrigin-RevId: 818844025 --- pyproject.toml | 3 - src/google/adk/cli/cli_deploy.py | 4 +- src/google/adk/cli/cli_tools_click.py | 22 +---- src/google/adk/cli/utils/logs.py | 30 +----- tests/unittests/cli/utils/test_logs.py | 124 ------------------------- 5 files changed, 8 insertions(+), 175 deletions(-) delete mode 100644 tests/unittests/cli/utils/test_logs.py diff --git a/pyproject.toml b/pyproject.toml index be34270b..95c69f1f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,13 +31,10 @@ dependencies = [ "authlib>=1.5.1, <2.0.0", # For RestAPI Tool "click>=8.1.8, <9.0.0", # For CLI tools "fastapi>=0.115.0, <1.0.0", # FastAPI framework - "google-api-core>=2.20.0, <3.0.0", # Core Google API helpers "google-api-python-client>=2.157.0, <3.0.0", # Google API client discovery - "google-auth>=2.24.0, <3.0.0", # Google auth helpers "google-cloud-aiplatform[agent_engines]>=1.112.0, <2.0.0",# For VertexAI integrations, e.g. example store. "google-cloud-bigtable>=2.32.0", # For Bigtable database "google-cloud-discoveryengine>=0.13.12, <0.14.0", # For Discovery Engine Search Tool - "google-cloud-logging>=3.8.0, <4.0.0", # For structured logging with trace correlation "google-cloud-secret-manager>=2.22.0, <3.0.0", # Fetching secrets in RestAPI Tool "google-cloud-spanner>=3.56.0, <4.0.0", # For Spanner database "google-cloud-speech>=2.30.0, <3.0.0", # For Audio Transcription diff --git a/src/google/adk/cli/cli_deploy.py b/src/google/adk/cli/cli_deploy.py index 66097216..d26b3c96 100644 --- a/src/google/adk/cli/cli_deploy.py +++ b/src/google/adk/cli/cli_deploy.py @@ -59,7 +59,7 @@ COPY --chown=myuser:myuser "agents/{app_name}/" "/app/agents/{app_name}/" EXPOSE {port} -CMD adk {command} --port={port} {host_option} {service_option} {trace_to_cloud_option} {log_to_cloud_option} {allow_origins_option} {a2a_option} "/app/agents" +CMD adk {command} --port={port} {host_option} {service_option} {trace_to_cloud_option} {allow_origins_option} {a2a_option} "/app/agents" """ _AGENT_ENGINE_APP_TEMPLATE: Final[str] = """ @@ -267,7 +267,6 @@ def to_cloud_run( memory_service_uri, ), trace_to_cloud_option='--trace_to_cloud' if trace_to_cloud else '', - log_to_cloud_option='--log_to_cloud', allow_origins_option=allow_origins_option, adk_version=adk_version, host_option=host_option, @@ -703,7 +702,6 @@ def to_gke( memory_service_uri, ), trace_to_cloud_option='--trace_to_cloud' if trace_to_cloud else '', - log_to_cloud_option='--log_to_cloud', allow_origins_option=allow_origins_option, adk_version=adk_version, host_option=host_option, diff --git a/src/google/adk/cli/cli_tools_click.py b/src/google/adk/cli/cli_tools_click.py index 90db3b0d..7115b5fc 100644 --- a/src/google/adk/cli/cli_tools_click.py +++ b/src/google/adk/cli/cli_tools_click.py @@ -937,7 +937,6 @@ def fast_api_common_options(): """Decorator to add common fast api options to click commands.""" def decorator(func): - @click.option( "--host", type=str, @@ -987,17 +986,6 @@ def fast_api_common_options(): " Observability services - Cloud Trace and Cloud Logging." ), ) - @click.option( - "--log_to_cloud", - is_flag=True, - show_default=True, - default=False, - help=( - "Optional. Emit structured JSON logs via Google Cloud Logging," - " keeping multiline messages in a single entry. Only set this to" - " True when deployed to Google Cloud environment." - ), - ) @click.option( "--reload/--no-reload", default=True, @@ -1077,7 +1065,6 @@ def cli_web( port: int = 8000, trace_to_cloud: bool = False, otel_to_cloud: bool = False, - log_to_cloud: bool = False, reload: bool = True, session_service_uri: Optional[str] = None, artifact_service_uri: Optional[str] = None, @@ -1099,9 +1086,7 @@ def cli_web( adk web --session_service_uri=[uri] --port=[port] path/to/agents_dir """ - logs.setup_adk_logger( - getattr(logging, log_level.upper()), log_to_cloud=log_to_cloud - ) + logs.setup_adk_logger(getattr(logging, log_level.upper())) @asynccontextmanager async def _lifespan(app: FastAPI): @@ -1179,7 +1164,6 @@ def cli_api_server( port: int = 8000, trace_to_cloud: bool = False, otel_to_cloud: bool = False, - log_to_cloud: bool = False, reload: bool = True, session_service_uri: Optional[str] = None, artifact_service_uri: Optional[str] = None, @@ -1199,9 +1183,7 @@ def cli_api_server( adk api_server --session_service_uri=[uri] --port=[port] path/to/agents_dir """ - logs.setup_adk_logger( - getattr(logging, log_level.upper()), log_to_cloud=log_to_cloud - ) + logs.setup_adk_logger(getattr(logging, log_level.upper())) session_service_uri = session_service_uri or session_db_url artifact_service_uri = artifact_service_uri or artifact_storage_uri diff --git a/src/google/adk/cli/utils/logs.py b/src/google/adk/cli/utils/logs.py index 4b0bfd9c..a9abae18 100644 --- a/src/google/adk/cli/utils/logs.py +++ b/src/google/adk/cli/utils/logs.py @@ -19,37 +19,17 @@ import os import tempfile import time -from google.api_core import exceptions as api_core_exceptions -from google.auth import exceptions as auth_exceptions -from google.cloud import logging as cloud_logging - LOGGING_FORMAT = ( '%(asctime)s - %(levelname)s - %(filename)s:%(lineno)d - %(message)s' ) -def setup_adk_logger(level=logging.INFO, *, log_to_cloud: bool = False): - """Set up ADK logger with optional Google Cloud Logging integration.""" - root_logger = logging.getLogger() +def setup_adk_logger(level=logging.INFO): + # Configure the root logger format and level. + logging.basicConfig(level=level, format=LOGGING_FORMAT) - if log_to_cloud: - # Remove the default StreamHandler to avoid duplicate stdout logs. - root_logger.handlers = [] - client = cloud_logging.Client() - client.setup_logging(log_level=level) - root_logger.setLevel(level) - else: - if root_logger.handlers: - # Uvicorn installs handlers ahead of application code, so basicConfig is a no-op. - formatter = logging.Formatter(LOGGING_FORMAT) - for handler in root_logger.handlers: - handler.setLevel(level) - handler.setFormatter(formatter) - else: - logging.basicConfig(level=level, format=LOGGING_FORMAT) - root_logger.setLevel(level) - - logging.getLogger('google_adk').setLevel(level) + adk_logger = logging.getLogger('google_adk') + adk_logger.setLevel(level) def log_to_tmp_folder( diff --git a/tests/unittests/cli/utils/test_logs.py b/tests/unittests/cli/utils/test_logs.py deleted file mode 100644 index 0a61459e..00000000 --- a/tests/unittests/cli/utils/test_logs.py +++ /dev/null @@ -1,124 +0,0 @@ -# Copyright 2025 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from __future__ import annotations - -from importlib import reload -import io -import json -import logging -from unittest import mock -from unittest import TestCase - -from google.adk.cli.utils import logs as logs_module - -logs = reload(logs_module) - - -class TestSetupAdkLogger(TestCase): - """Tests for setup_adk_logger helper.""" - - def setUp(self): - super().setUp() - self._reset_logging() - self.addCleanup(self._reset_logging) - - def _reset_logging(self): - root_logger = logging.getLogger() - for handler in list(root_logger.handlers): - root_logger.removeHandler(handler) - try: - handler.close() - except Exception: # pylint: disable=broad-except - pass - root_logger.setLevel(logging.WARNING) - root_logger.propagate = True - - def test_log_to_cloud_uses_google_cloud_logging_client(self): - """Log setup delegates to google.cloud.logging client when available.""" - buffer = io.StringIO() - - class _JsonFormatter(logging.Formatter): - - def format(self, record): - return json.dumps({'message': record.getMessage()}, ensure_ascii=False) - - class FakeClient: - - def __init__(self): - self.called_with = None - - def setup_logging(self, log_level=logging.INFO): - self.called_with = log_level - handler = logging.StreamHandler(buffer) - handler.setLevel(log_level) - handler.setFormatter(_JsonFormatter()) - root = logging.getLogger() - root.handlers = [handler] - root.setLevel(log_level) - root.propagate = False - - fake_client = FakeClient() - client_factory = mock.Mock(return_value=fake_client) - with mock.patch.object(logs, 'cloud_logging', autospec=True) as mock_module: - mock_module.Client = client_factory - logs.setup_adk_logger(level=logging.INFO, log_to_cloud=True) - logging.getLogger('google_adk.test').info('hello\nworld') - - client_factory.assert_called_once() - self.assertEqual(logging.getLogger('google_adk').level, logging.INFO) - self.assertEqual(fake_client.called_with, logging.INFO) - output_lines = [ - line for line in buffer.getvalue().splitlines() if line.strip() - ] - self.assertEqual(len(output_lines), 1) - entry = json.loads(output_lines[0]) - self.assertEqual(entry['message'], 'hello\nworld') - - def test_log_to_cloud_client_failure_surfaces_error(self): - """Cloud logging setup failures surface as actionable errors.""" - - class FailingClient: - - def setup_logging(self, log_level=logging.INFO): - del log_level - raise OSError('boom') - - client_factory = mock.Mock(return_value=FailingClient()) - with mock.patch.object(logs, 'cloud_logging', autospec=True) as mock_module: - mock_module.Client = client_factory - with self.assertRaises(OSError): - logs.setup_adk_logger(level=logging.INFO, log_to_cloud=True) - - client_factory.assert_called_once() - - def test_text_logging_configures_basic_logging(self): - """Fallback text logging configures default formatter and handlers.""" - logs.setup_adk_logger(level=logging.ERROR, log_to_cloud=False) - - root_logger = logging.getLogger() - self.assertEqual(root_logger.level, logging.ERROR) - self.assertTrue(root_logger.handlers) - handler = root_logger.handlers[0] - formatter = handler.formatter - self.assertIsInstance(formatter, logging.Formatter) - self.assertEqual( - formatter._style._fmt, # pylint: disable=protected-access - logs.LOGGING_FORMAT, - ) - - def test_text_logging_sets_adk_logger_level(self): - """ADK logger level is aligned when text logging is used.""" - logs.setup_adk_logger(level=logging.WARNING, log_to_cloud=False) - self.assertEqual(logging.getLogger('google_adk').level, logging.WARNING)