From 0709089be10fe7a2e63e2d7c9c6da4ccef6d0c60 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Wed, 28 Oct 2026 12:14:17 -0400 Subject: [PATCH] feat(api_core): extend ErrorInfo dict accessors, centralize status code extraction, and update StructuredLogFormatter --- .../google/api_core/client_logging.py | 44 ++++- .../google/api_core/exceptions.py | 75 ++++++++- .../google/api_core/gapic_v1/method.py | 52 +----- .../tests/unit/gapic/test_method.py | 50 ------ .../tests/unit/test_client_logging.py | 70 ++++++++ .../tests/unit/test_exceptions.py | 153 ++++++++++++++++++ 6 files changed, 339 insertions(+), 105 deletions(-) diff --git a/packages/google-api-core/google/api_core/client_logging.py b/packages/google-api-core/google/api_core/client_logging.py index f5fe1b199695..8b29e8f451b2 100644 --- a/packages/google-api-core/google/api_core/client_logging.py +++ b/packages/google-api-core/google/api_core/client_logging.py @@ -1,6 +1,7 @@ import json import logging import os +from collections.abc import Mapping from typing import List, Optional _LOGGING_INITIALIZED = False @@ -10,6 +11,7 @@ # # TODO(https://github.com/googleapis/python-api-core/issues/761): Update this list to support additional logging fields. _recognized_logging_fields = [ + # Application payload debugging fields: "httpRequest", "rpcName", "serviceName", @@ -21,8 +23,31 @@ "metadata", "retryAttempt", "httpResponse", + # Diagnostic error and trace correlation fields: + "url.full", + "rpc.method", + "exception.type", + "exception.message", + "exception.stacktrace", + "rpc.system.name", + "rpc.response.status_code", + "error.type", + "status.message", + "gcp.errors.domain", + "gcp.errors.metadata", + "trace_id", + "span_id", + "logging.googleapis.com/trace", + "logging.googleapis.com/spanId", + "logging.googleapis.com/trace_sampled", + # Dynamic metadata prefixes: + "gcp.errors.metadata.*", ] # Additional fields to be Logged. +# Pre-compiled allowlists for O(1) exact lookups and C-level prefix matching. +_EXACT_FIELDS = frozenset(f for f in _recognized_logging_fields if not f.endswith("*")) +_PREFIX_FIELDS = tuple(f[:-1] for f in _recognized_logging_fields if f.endswith("*")) + def logger_configured(logger) -> bool: """Determines whether `logger` has non-default configuration @@ -136,8 +161,21 @@ def format(self, record: logging.LogRecord): "message": record.getMessage(), } - for field_name in _recognized_logging_fields: - value = getattr(record, field_name, None) + # 1. Unpack record.json_fields (keeping api_core and google-cloud-logging formatters consistent) + json_fields = getattr(record, "json_fields", None) + if isinstance(json_fields, Mapping): + for key, value in json_fields.items(): + if value is not None: + log_obj[key] = value + + # 2a. Extract exact allowed fields via getattr() (supports @property and class-level attributes) + for key in _EXACT_FIELDS: + value = getattr(record, key, None) if value is not None: - log_obj[field_name] = value + log_obj[key] = value + + # 2b. Extract dynamic wildcard prefix fields (e.g. gcp.errors.metadata.*) from instance __dict__ + for key, value in vars(record).items(): + if value is not None and key.startswith(_PREFIX_FIELDS): + log_obj[key] = value return json.dumps(log_obj) diff --git a/packages/google-api-core/google/api_core/exceptions.py b/packages/google-api-core/google/api_core/exceptions.py index aa9898c5b9a6..e8cddaca5579 100644 --- a/packages/google-api-core/google/api_core/exceptions.py +++ b/packages/google-api-core/google/api_core/exceptions.py @@ -20,6 +20,7 @@ from __future__ import absolute_import, unicode_literals +import http import http.client import warnings from typing import Dict, Optional @@ -171,7 +172,11 @@ def reason(self): Returns: Union[str, None]: An optional string containing reason of the error. """ - return self._error_info.reason if self._error_info else None + if not self._error_info: + return None + if isinstance(self._error_info, dict): + return self._error_info.get("reason") + return self._error_info.reason @property def domain(self): @@ -183,7 +188,11 @@ def domain(self): Returns: Union[str, None]: An optional string containing a logical grouping to which the "reason" belongs. """ - return self._error_info.domain if self._error_info else None + if not self._error_info: + return None + if isinstance(self._error_info, dict): + return self._error_info.get("domain") + return self._error_info.domain @property def metadata(self): @@ -195,7 +204,11 @@ def metadata(self): Returns: Union[Dict[str, str], None]: An optional object containing structured details about the error. """ - return self._error_info.metadata if self._error_info else None + if not self._error_info: + return None + if isinstance(self._error_info, dict): + return self._error_info.get("metadata") + return self._error_info.metadata @property def errors(self): @@ -631,6 +644,62 @@ def from_grpc_status(status_code, message, **kwargs): return error +def _extract_status_code(exc: BaseException | None, default: str | None = None) -> str: + """Extract canonical status code name string from an exception. + + Status code name strings are resolved by inspecting the following: + * Chained exceptions: Unwraps RetryError or __cause__ to the root error. + * Enum & code attributes: Inspects .grpc_status_code on GoogleAPICallError + or .code on gRPC/HTTP errors. + * Integer status codes: Maps raw gRPC integer status codes or HTTP status + codes to canonical enum names. + * Fallback: Uses ``default`` if provided, otherwise defaults to the + exception class name. + + Args: + exc (BaseException | None): The exception to extract the status code + name from. + default (str | None): Optional fallback status code string when + ``exc`` carries no status code. + + Returns: + str: The canonical status code name (e.g. "NOT_FOUND", "UNAVAILABLE", + "BAD_REQUEST") or class name. + """ + if exc is None: + return default or "" + + # 1. Unwrap chained exceptions: unwrap RetryError or __cause__ to the root failure + target = getattr(exc, "cause", None) or getattr(exc, "__cause__", None) or exc + + # 2. Check enum & code attributes: .grpc_status_code enum or callable/non-callable .code + status = getattr(target, "grpc_status_code", None) + if status is None and hasattr(target, "code"): + try: + status = target.code() if callable(target.code) else target.code + # Defensive fallback: observability status extraction must never raise + # if a third-party or mock exception implements a failing .code() method. + except Exception: + status = None + + name = getattr(status, "name", None) + if name: + return str(name) + + # 3. Check integer status codes: map raw gRPC integer codes or HTTP status codes to canonical names + if isinstance(status, int): + if status in _INT_TO_GRPC_CODE: + grpc_code = _INT_TO_GRPC_CODE[status] + return getattr(grpc_code, "name", str(grpc_code)) + try: + return http.HTTPStatus(status).name + except ValueError: + return str(status) + + # 4. Fallback: default to explicit default or exception class name for standard Python errors + return default or target.__class__.__name__ + + def _is_informative_grpc_error(rpc_exc): return hasattr(rpc_exc, "code") and hasattr(rpc_exc, "details") diff --git a/packages/google-api-core/google/api_core/gapic_v1/method.py b/packages/google-api-core/google/api_core/gapic_v1/method.py index 873e925ec1f7..041bb1cc965c 100644 --- a/packages/google-api-core/google/api_core/gapic_v1/method.py +++ b/packages/google-api-core/google/api_core/gapic_v1/method.py @@ -23,7 +23,7 @@ import functools from typing import Any, List, Optional, Tuple -from google.api_core import _observability, grpc_helpers +from google.api_core import _observability, exceptions, grpc_helpers from google.api_core.gapic_v1 import client_info from google.api_core.gapic_v1.client_info import METRICS_METADATA_KEY from google.api_core.timeout import TimeToDeadlineTimeout @@ -126,50 +126,6 @@ def _extract_rpc_identity( return method_str, service, method -def _extract_status_code(exc: Optional[BaseException]) -> str: - """Extract canonical status code name string from an exception. - - Status code name strings are resolved by inspecting the following locations: - * Chained exceptions: Unwraps RetryError or __cause__ to the root exception. - * Enum & code attributes: Inspects .grpc_status_code on GoogleAPICallError or .code on gRPC errors. - * Integer status codes: Maps raw gRPC integer status codes to canonical enum names. - * Fallback: Defaults to the exception class name for standard Python errors. - - Args: - exc (Optional[BaseException]): The exception to extract the status code name from. - - Returns: - str: The canonical status code name (e.g. "NOT_FOUND", "UNAVAILABLE") or class name. - """ - if exc is None: - return "" - - # 1. Unwrap chained exceptions: unwrap RetryError or __cause__ to the root failure - target = getattr(exc, "cause", None) or getattr(exc, "__cause__", None) or exc - - # 2. Check enum & code attributes: .grpc_status_code enum or callable/non-callable .code - status = getattr(target, "grpc_status_code", None) - if status is None and hasattr(target, "code"): - try: - status = target.code() if callable(target.code) else target.code - except Exception: - status = None - - name = getattr(status, "name", None) - if name: - return str(name) - - # 3. Check integer status codes: map raw gRPC integer status codes to canonical enum names - if isinstance(status, int): - from google.api_core import exceptions - - status = exceptions._INT_TO_GRPC_CODE.get(status, status) - return getattr(status, "name", str(status)) - - # 4. Fallback: default to the exception class name for standard Python errors - return target.__class__.__name__ - - def _extract_error_attributes(exc: Optional[BaseException]) -> dict[str, Any]: """Extract gcp.errors.* and error.type attributes from an exception. @@ -206,8 +162,6 @@ def _extract_error_attributes(exc: Optional[BaseException]) -> dict[str, Any]: ) if rpc_call is not None and hasattr(rpc_call, "trailing_metadata"): try: - from google.api_core import exceptions - _, error_info = exceptions._parse_grpc_error_details(rpc_call) except Exception: pass @@ -239,7 +193,7 @@ def _extract_error_attributes(exc: Optional[BaseException]) -> dict[str, Any]: else: # Fallback per OpenTelemetry Semantic Conventions: every failed span should record # a low-cardinality error.type. Use canonical status code name or exception class name. - status_code = _extract_status_code(target_exc) + status_code = exceptions._extract_status_code(target_exc) attrs["error.type"] = status_code or target_exc.__class__.__name__ if metadata: for k, v in metadata.items(): @@ -425,7 +379,7 @@ def _trace_span(self): except BaseException as exc: if span is not None and hasattr(span, "set_attribute"): span.set_attribute( - "rpc.response.status_code", _extract_status_code(exc) + "rpc.response.status_code", exceptions._extract_status_code(exc) ) for k, v in _extract_error_attributes(exc).items(): span.set_attribute(k, v) diff --git a/packages/google-api-core/tests/unit/gapic/test_method.py b/packages/google-api-core/tests/unit/gapic/test_method.py index a4a5f805ac91..e1eba53d5fc4 100644 --- a/packages/google-api-core/tests/unit/gapic/test_method.py +++ b/packages/google-api-core/tests/unit/gapic/test_method.py @@ -817,56 +817,6 @@ def test_wrap_method_otel_tracing_records_gcp_error_attributes(mock_otel): mock_otel.span.set_attribute.assert_any_call("status.message", "quota exceeded") -def test_extract_status_code_variations(): - """Proves that _extract_status_code handles grpc status, callable/non-callable codes, ints, and exceptions.""" - import types - - from google.api_core.gapic_v1.method import _extract_status_code - - # 1. grpc_status exists but has no name or name is None - exc1 = types.SimpleNamespace(grpc_status_code=types.SimpleNamespace(name=None)) - assert _extract_status_code(exc1) == "SimpleNamespace" - - # 2. callable code_fn returns object with name - exc2 = types.SimpleNamespace(code=lambda: types.SimpleNamespace(name="CANCELLED")) - assert _extract_status_code(exc2) == "CANCELLED" - - # 3. callable code_fn returns object without name - exc3 = types.SimpleNamespace(code=lambda: types.SimpleNamespace(name=None)) - assert _extract_status_code(exc3) == "SimpleNamespace" - - # 4. callable code_fn raises Exception - def raising_code(): - raise RuntimeError("boom") - - exc4 = types.SimpleNamespace(code=raising_code) - assert _extract_status_code(exc4) == "SimpleNamespace" - - # 5. non-callable code_fn with name - exc5 = types.SimpleNamespace(code=types.SimpleNamespace(name="DEADLINE_EXCEEDED")) - assert _extract_status_code(exc5) == "DEADLINE_EXCEEDED" - - # 6. non-callable code_fn that is an int in _INT_TO_GRPC_CODE (5 -> NOT_FOUND) - exc6 = types.SimpleNamespace(code=5) - assert _extract_status_code(exc6) == "NOT_FOUND" - - # 7. non-callable code_fn that is an int not in _INT_TO_GRPC_CODE (999) - exc7 = types.SimpleNamespace(code=999) - assert _extract_status_code(exc7) == "999" - - # 8. non-callable code_fn that is not an int and has no name - exc8 = types.SimpleNamespace(code="unknown_code") - assert _extract_status_code(exc8) == "SimpleNamespace" - - # 9. None exception - assert _extract_status_code(None) == "" - - # 10. __cause__ chaining fallback - inner_exc = types.SimpleNamespace(code=5) - outer_exc = types.SimpleNamespace(__cause__=inner_exc) - assert _extract_status_code(outer_exc) == "NOT_FOUND" - - def test_extract_error_attributes_variations(): """Proves that _extract_error_attributes handles __cause__, gRPC error details parsing, and direct fallbacks.""" import types diff --git a/packages/google-api-core/tests/unit/test_client_logging.py b/packages/google-api-core/tests/unit/test_client_logging.py index e3dadd3cd036..b8ad8359f083 100644 --- a/packages/google-api-core/tests/unit/test_client_logging.py +++ b/packages/google-api-core/tests/unit/test_client_logging.py @@ -137,3 +137,73 @@ def test_structured_log_formatter(): assert parsed_msg["severity"] == "DEBUG" assert parsed_msg["message"] == "This is a test message." assert parsed_msg["rpcName"] == "bar" + + +DEFAULT_CONSUMER_PROJECT = "projects/123" +SAMPLE_TRACE_ID = "0123456789abcdef0123456789abcdef" +SAMPLE_SPAN_ID = "0123456789abcdef" + + +def test_structured_log_formatter_json_fields_and_wildcard_metadata(): + record = logging.LogRecord( + name="google.cloud.secretmanager_v1", + level=logging.ERROR, + msg="RPC failed for /google.cloud.secretmanager.v1.SecretManagerService/GetSecret", + pathname="some/path", + lineno=42, + args=None, + exc_info=None, + ) + extra_fields = { + "rpcName": "/google.cloud.secretmanager.v1.SecretManagerService/GetSecret", + "retryAttempt": 0, + "rpc.system.name": "grpc", + "rpc.response.status_code": "NOT_FOUND", + "error.type": "RESOURCE_NOT_FOUND", + "status.message": "Secret not found", + "gcp.errors.domain": "googleapis.com", + "gcp.errors.metadata": {"service": "secretmanager.googleapis.com"}, + "gcp.errors.metadata.service": "secretmanager.googleapis.com", + "gcp.errors.metadata.consumer": DEFAULT_CONSUMER_PROJECT, + "gcp.errors.metadata.is_retryable": False, + "trace_id": SAMPLE_TRACE_ID, + "span_id": SAMPLE_SPAN_ID, + "logging.googleapis.com/trace": SAMPLE_TRACE_ID, + "logging.googleapis.com/spanId": SAMPLE_SPAN_ID, + "logging.googleapis.com/trace_sampled": False, + } + for k, v in extra_fields.items(): + setattr(record, k, v) + record.json_fields = dict(extra_fields) + record.unrecognized_field = "should_be_ignored" + + formatted_msg = StructuredLogFormatter().format(record) + parsed_msg = json.loads(formatted_msg) + + for k, v in extra_fields.items(): + assert parsed_msg[k] == v + assert "json_fields" not in parsed_msg + assert "unrecognized_field" not in parsed_msg + + +def test_structured_log_formatter_json_fields_only(): + record = logging.LogRecord( + name="google.cloud.secretmanager_v1", + level=logging.DEBUG, + msg="Attempt failed", + pathname="some/path", + lineno=10, + args=None, + exc_info=None, + ) + record.json_fields = { + "custom_from_json_fields": "preserved", + "none_value": None, + } + + formatted_msg = StructuredLogFormatter().format(record) + parsed_msg = json.loads(formatted_msg) + + assert parsed_msg["custom_from_json_fields"] == "preserved" + assert "none_value" not in parsed_msg + assert "json_fields" not in parsed_msg diff --git a/packages/google-api-core/tests/unit/test_exceptions.py b/packages/google-api-core/tests/unit/test_exceptions.py index 6a71ddeb9261..354f0d47f9bd 100644 --- a/packages/google-api-core/tests/unit/test_exceptions.py +++ b/packages/google-api-core/tests/unit/test_exceptions.py @@ -14,6 +14,7 @@ import http.client import json +import types from unittest import mock import pytest @@ -352,6 +353,12 @@ def test_error_details_from_rest_response(): " 'service': 'translate.googleapis.com'}," " 'reason': 'SERVICE_DISABLED'}]" ) + assert exception.reason == "SERVICE_DISABLED" + assert exception.domain == "googleapis.com" + assert exception.metadata == { + "consumer": "projects/455411330361", + "service": "translate.googleapis.com", + } def test_error_details_from_v1_rest_response(): @@ -434,3 +441,149 @@ def test_from_grpc_error_misleading_404(): assert exception.grpc_status_code == grpc.StatusCode.UNIMPLEMENTED assert "Received http2 header with status: 404" in exception.message assert "api_endpoint" in exception.message + + +@pytest.mark.parametrize( + "error_info, expected_reason, expected_domain, expected_metadata", + [ + pytest.param( + { + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + "reason": "SERVICE_DISABLED", + "domain": "googleapis.com", + "metadata": {"service": "translate.googleapis.com"}, + }, + "SERVICE_DISABLED", + "googleapis.com", + {"service": "translate.googleapis.com"}, + id="populated_dict", + ), + pytest.param( + {"@type": "type.googleapis.com/google.rpc.ErrorInfo"}, + None, + None, + None, + id="partial_dict_missing_keys", + ), + pytest.param({}, None, None, None, id="empty_dict"), + ], +) +def test_error_info_dict_accessors_boundary( + error_info, expected_reason, expected_domain, expected_metadata +): + exception = exceptions.GoogleAPICallError("err", error_info=error_info) + assert exception.reason == expected_reason + assert exception.domain == expected_domain + assert exception.metadata == expected_metadata + + +def _raising_code(): + raise RuntimeError("boom") + + +@pytest.mark.parametrize( + "exc_obj, kwargs, expected_code", + [ + pytest.param(None, {}, "", id="none_no_default"), + pytest.param( + None, {"default": "UNAVAILABLE"}, "UNAVAILABLE", id="none_with_default" + ), + pytest.param( + exceptions.NotFound("missing"), + {}, + "NOT_FOUND", + id="google_api_call_error_with_grpc_code", + ), + pytest.param( + exceptions.BadRequest("invalid"), + {}, + "BAD_REQUEST", + id="google_api_call_error_http_only", + ), + pytest.param( + exceptions.RetryError( + "retry failed", cause=exceptions.ServiceUnavailable("down") + ), + {}, + "UNAVAILABLE" if grpc is not None else "SERVICE_UNAVAILABLE", + id="retry_error_unwraps_cause", + ), + pytest.param( + types.SimpleNamespace(__cause__=exceptions.NotFound("root missing")), + {}, + "NOT_FOUND", + id="dunder_cause_unwrapped", + ), + pytest.param( + types.SimpleNamespace(grpc_status_code=types.SimpleNamespace(name=None)), + {}, + "SimpleNamespace", + id="grpc_status_code_without_name_falls_back", + ), + pytest.param( + types.SimpleNamespace( + code=lambda: types.SimpleNamespace(name="DEADLINE_EXCEEDED") + ), + {}, + "DEADLINE_EXCEEDED", + id="callable_code_returns_enum_name", + ), + pytest.param( + types.SimpleNamespace(code=lambda: types.SimpleNamespace(name=None)), + {}, + "SimpleNamespace", + id="callable_code_without_name_falls_back", + ), + pytest.param( + types.SimpleNamespace(code=_raising_code), + {}, + "SimpleNamespace", + id="callable_code_raises_falls_back_to_class_name", + ), + pytest.param( + types.SimpleNamespace(code=_raising_code), + {"default": "UNAVAILABLE"}, + "UNAVAILABLE", + id="callable_code_raises_falls_back_to_default", + ), + pytest.param( + types.SimpleNamespace(code=5), + {}, + "NOT_FOUND", + marks=pytest.mark.skipif(grpc is None, reason="requires grpc"), + id="int_grpc_code_mapped", + ), + pytest.param( + types.SimpleNamespace(code=404), + {}, + "NOT_FOUND", + id="int_http_status_code_mapped", + ), + pytest.param( + types.SimpleNamespace(code=999), + {}, + "999", + id="int_unknown_code_stringified", + ), + pytest.param( + types.SimpleNamespace(code="unknown_code"), + {}, + "SimpleNamespace", + id="non_int_code_without_name_falls_back", + ), + pytest.param( + ValueError("bad val"), + {}, + "ValueError", + id="standard_exception_class_name", + ), + pytest.param( + ValueError("bad val"), + {"default": "503"}, + "503", + id="standard_exception_with_default", + ), + ], +) +def test_extract_status_code_from_exceptions(exc_obj, kwargs, expected_code): + assert exceptions._extract_status_code(exc_obj, **kwargs) == expected_code