fix: rollback of add structured JSON logging with Cloud Trace correlation Close #1683

PiperOrigin-RevId: 818844025
This commit is contained in:
George Weale
2025-10-13 14:57:23 -07:00
committed by Copybara-Service
parent d8548aabd2
commit 3c0b99a19e
5 changed files with 8 additions and 175 deletions
-3
View File
@@ -31,13 +31,10 @@ dependencies = [
"authlib>=1.5.1, <2.0.0", # For RestAPI Tool "authlib>=1.5.1, <2.0.0", # For RestAPI Tool
"click>=8.1.8, <9.0.0", # For CLI tools "click>=8.1.8, <9.0.0", # For CLI tools
"fastapi>=0.115.0, <1.0.0", # FastAPI framework "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-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-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-bigtable>=2.32.0", # For Bigtable database
"google-cloud-discoveryengine>=0.13.12, <0.14.0", # For Discovery Engine Search Tool "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-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-spanner>=3.56.0, <4.0.0", # For Spanner database
"google-cloud-speech>=2.30.0, <3.0.0", # For Audio Transcription "google-cloud-speech>=2.30.0, <3.0.0", # For Audio Transcription
+1 -3
View File
@@ -59,7 +59,7 @@ COPY --chown=myuser:myuser "agents/{app_name}/" "/app/agents/{app_name}/"
EXPOSE {port} 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] = """ _AGENT_ENGINE_APP_TEMPLATE: Final[str] = """
@@ -267,7 +267,6 @@ def to_cloud_run(
memory_service_uri, memory_service_uri,
), ),
trace_to_cloud_option='--trace_to_cloud' if trace_to_cloud else '', 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, allow_origins_option=allow_origins_option,
adk_version=adk_version, adk_version=adk_version,
host_option=host_option, host_option=host_option,
@@ -703,7 +702,6 @@ def to_gke(
memory_service_uri, memory_service_uri,
), ),
trace_to_cloud_option='--trace_to_cloud' if trace_to_cloud else '', 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, allow_origins_option=allow_origins_option,
adk_version=adk_version, adk_version=adk_version,
host_option=host_option, host_option=host_option,
+2 -20
View File
@@ -937,7 +937,6 @@ def fast_api_common_options():
"""Decorator to add common fast api options to click commands.""" """Decorator to add common fast api options to click commands."""
def decorator(func): def decorator(func):
@click.option( @click.option(
"--host", "--host",
type=str, type=str,
@@ -987,17 +986,6 @@ def fast_api_common_options():
" Observability services - Cloud Trace and Cloud Logging." " 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( @click.option(
"--reload/--no-reload", "--reload/--no-reload",
default=True, default=True,
@@ -1077,7 +1065,6 @@ def cli_web(
port: int = 8000, port: int = 8000,
trace_to_cloud: bool = False, trace_to_cloud: bool = False,
otel_to_cloud: bool = False, otel_to_cloud: bool = False,
log_to_cloud: bool = False,
reload: bool = True, reload: bool = True,
session_service_uri: Optional[str] = None, session_service_uri: Optional[str] = None,
artifact_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 adk web --session_service_uri=[uri] --port=[port] path/to/agents_dir
""" """
logs.setup_adk_logger( logs.setup_adk_logger(getattr(logging, log_level.upper()))
getattr(logging, log_level.upper()), log_to_cloud=log_to_cloud
)
@asynccontextmanager @asynccontextmanager
async def _lifespan(app: FastAPI): async def _lifespan(app: FastAPI):
@@ -1179,7 +1164,6 @@ def cli_api_server(
port: int = 8000, port: int = 8000,
trace_to_cloud: bool = False, trace_to_cloud: bool = False,
otel_to_cloud: bool = False, otel_to_cloud: bool = False,
log_to_cloud: bool = False,
reload: bool = True, reload: bool = True,
session_service_uri: Optional[str] = None, session_service_uri: Optional[str] = None,
artifact_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 adk api_server --session_service_uri=[uri] --port=[port] path/to/agents_dir
""" """
logs.setup_adk_logger( logs.setup_adk_logger(getattr(logging, log_level.upper()))
getattr(logging, log_level.upper()), log_to_cloud=log_to_cloud
)
session_service_uri = session_service_uri or session_db_url session_service_uri = session_service_uri or session_db_url
artifact_service_uri = artifact_service_uri or artifact_storage_uri artifact_service_uri = artifact_service_uri or artifact_storage_uri
+4 -24
View File
@@ -19,37 +19,17 @@ import os
import tempfile import tempfile
import time 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 = ( LOGGING_FORMAT = (
'%(asctime)s - %(levelname)s - %(filename)s:%(lineno)d - %(message)s' '%(asctime)s - %(levelname)s - %(filename)s:%(lineno)d - %(message)s'
) )
def setup_adk_logger(level=logging.INFO, *, log_to_cloud: bool = False): def setup_adk_logger(level=logging.INFO):
"""Set up ADK logger with optional Google Cloud Logging integration.""" # Configure the root logger format and level.
root_logger = logging.getLogger()
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) 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( def log_to_tmp_folder(
-124
View File
@@ -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)