Skip to content

feat(google-api-core): centralize telemetry error attribute and trace context builders in _observability - #18893

Draft
chalmerlowe wants to merge 1 commit into
feat/otel-logging-m1-foundationfrom
feat/otel-logging-m2-builders
Draft

chalmerlowe wants to merge 1 commit into
feat/otel-logging-m1-foundationfrom
feat/otel-logging-m2-builders

Conversation

@chalmerlowe

Copy link
Copy Markdown
Contributor

Problem / Context

To emit consistent error metadata and trace correlation across both GAPIC method wrappers (gapic_v1.method) and transport-level interceptors (_observability), google-api-core needs shared helpers that work identically on wrapped GoogleAPICallError exceptions and raw gRPC RpcError exceptions (before gapic_v1.method's wrap_errors runs):

  1. Duplicate Error Attribute Extraction: _extract_error_attributes previously lived inside google.api_core.gapic_v1.method and only inspected wrapped GoogleAPICallError properties (.reason, .domain, .metadata, .message). At the L4 gRPC interceptor boundary, failures are still raw grpc.RpcError / grpc.aio.AioRpcError instances whose ErrorInfo details live in trailing_metadata() (grpc-status-details-bin) and whose message comes from exc.details().
  2. Trace Context Correlation: Structured error logs require Cloud Logging trace correlation fields (logging.googleapis.com/trace, logging.googleapis.com/spanId, logging.googleapis.com/trace_sampled) extracted from an active OpenTelemetry span without raising if OpenTelemetry is not installed or the span is non-recording.

Solution

  • google/api_core/_observability.py:
    • Added has_opentelemetry() with module-level caching to check whether opentelemetry.trace is importable.
    • Added extract_trace_context(span=None) to extract trace_id (32-char lowercase hex), span_id (16-char lowercase hex), and trace_sampled (bool) from an explicit or active OpenTelemetry span.
    • Added _build_telemetry_error_attributes(exc, status_code=None, status_message=None, include_exception=False) to extract error.type, status.message, gcp.errors.domain, gcp.errors.metadata, and optional exception.* fields from both GoogleAPICallError and raw gRPC RpcError trailing metadata (google.rpc.status_pb2.Status).
  • google/api_core/gapic_v1/method.py:
    • Aliased _extract_error_attributes = _observability._build_telemetry_error_attributes, removing 73 lines of duplicated error-attribute extraction logic.
  • Unit Tests:
    • Added unit tests in tests/unit/test_observability.py achieving 100% line and branch coverage across all new helpers.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request centralizes and enhances telemetry and error attribute extraction by introducing has_opentelemetry, extract_trace_context, and _build_telemetry_error_attributes helpers in _observability.py, replacing the redundant _extract_error_attributes implementation in method.py, and adding comprehensive unit tests. However, a critical issue was identified in _observability.py where exceptions._extract_status_code is called with an unsupported default keyword argument, which will cause a TypeError at runtime; this should be resolved using a logical or fallback instead.

# a low-cardinality error.type. Use canonical status code name, fallback status_code, or exception class name.
from google.api_core import exceptions

resolved_code = exceptions._extract_status_code(target_exc, default=status_code)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The helper function exceptions._extract_status_code does not accept a default keyword argument in google-api-core. Calling it with default=status_code will raise a TypeError at runtime. You can safely achieve the same fallback behavior by using the or operator.

Suggested change
resolved_code = exceptions._extract_status_code(target_exc, default=status_code)
resolved_code = exceptions._extract_status_code(target_exc) or status_code

@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m2-builders branch from 5659093 to 25b8205 Compare October 29, 2026 10:13
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m1-foundation branch from f4d9d6a to 3240601 Compare October 29, 2026 10:13
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m2-builders branch from 25b8205 to d05a1be Compare October 29, 2026 11:29
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m1-foundation branch from 3240601 to 7fad61b Compare October 29, 2026 11:29
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m2-builders branch from d05a1be to 703466a Compare October 29, 2026 13:37
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m1-foundation branch from 7fad61b to 858b5dc Compare October 29, 2026 13:37
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m2-builders branch from 703466a to 915abfc Compare October 29, 2026 14:01
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m1-foundation branch from 858b5dc to f6ab8ea Compare October 29, 2026 14:01
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m2-builders branch from 915abfc to 4f0a450 Compare October 29, 2026 14:19
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m1-foundation branch from f6ab8ea to 024c8b8 Compare October 29, 2026 14:19
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m2-builders branch from 4f0a450 to 9d4a85f Compare October 29, 2026 14:59
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m1-foundation branch from 024c8b8 to b1c49d0 Compare October 29, 2026 14:59
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m2-builders branch from 9d4a85f to 59e1573 Compare October 30, 2026 14:38
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m1-foundation branch from b1c49d0 to 95e34e8 Compare October 30, 2026 14:38
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m2-builders branch from 59e1573 to f396cf3 Compare October 30, 2026 16:43
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m1-foundation branch from 95e34e8 to eae240a Compare October 30, 2026 16:43
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m2-builders branch from f396cf3 to 37c22d1 Compare October 30, 2026 17:50
@chalmerlowe
chalmerlowe force-pushed the feat/otel-logging-m1-foundation branch from eae240a to 0709089 Compare October 30, 2026 17:50

This branch has not been deployed

No deployments
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.

1 participant