From fc4b3c1792cb5562791ff8d4da0ed2a2b1dfe5c8 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Thu, 3 Sep 2026 06:30:13 -0400 Subject: [PATCH 01/12] feat(core): add request hook to inject GCP resource and project attributes --- .../google/api_core/_observability.py | 55 ++++++++++++++++++- .../tests/unit/test_observability.py | 47 +++++++++++++++- 2 files changed, 98 insertions(+), 4 deletions(-) diff --git a/packages/google-api-core/google/api_core/_observability.py b/packages/google-api-core/google/api_core/_observability.py index f101cec28f5c..262b9beda34d 100644 --- a/packages/google-api-core/google/api_core/_observability.py +++ b/packages/google-api-core/google/api_core/_observability.py @@ -64,6 +64,55 @@ def is_otel_capabilities_enabled( return False +def _extract_t4_attributes(request: Any) -> dict[str, Any]: + """Extracts Google Cloud semantic and resource attributes from a gRPC request object. + + Args: + request: The gRPC request object. + + Returns: + dict[str, Any]: A dictionary of semantic attributes. + """ + attrs: dict[str, Any] = {} + if request is None: + return attrs + + name = getattr(request, "name", None) + if name and isinstance(name, str): + attrs["gcp.resource.name"] = name + if "projects/" in name: + parts = name.split("/") + try: + idx = parts.index("projects") + if idx + 1 < len(parts): + attrs["gcp.project_id"] = parts[idx + 1] + except ValueError: + pass + + parent = getattr(request, "parent", None) + if parent and isinstance(parent, str): + attrs["gcp.resource.parent"] = parent + if "gcp.project_id" not in attrs and "projects/" in parent: + parts = parent.split("/") + try: + idx = parts.index("projects") + if idx + 1 < len(parts): + attrs["gcp.project_id"] = parts[idx + 1] + except ValueError: + pass + + return attrs + + +def _client_request_hook(span: Any, request: Any) -> None: + """OpenTelemetry client request hook to inject GCP resource attributes into the span.""" + if span is None or not getattr(span, "is_recording", lambda: True)(): + return + attrs = _extract_t4_attributes(request) + for key, value in attrs.items(): + span.set_attribute(key, value) + + def _get_tracer_provider( client_options: ClientOptions | dict[str, Any] | None = None, ) -> opentelemetry.trace.TracerProvider | None: @@ -102,7 +151,8 @@ def get_otel_interceptor( import opentelemetry.instrumentation.grpc as otel_grpc # type: ignore[import-not-found] interceptor: ClientInterceptor = otel_grpc.client_interceptor( - tracer_provider=_get_tracer_provider(client_options) + tracer_provider=_get_tracer_provider(client_options), + request_hook=_client_request_hook, ) def otel_interceptor(channel: grpc.Channel) -> grpc.Channel: @@ -131,5 +181,6 @@ def get_otel_async_interceptor( import opentelemetry.instrumentation.grpc as otel_grpc # type: ignore[import-not-found] return otel_grpc.aio_client_interceptors( - tracer_provider=_get_tracer_provider(client_options) + tracer_provider=_get_tracer_provider(client_options), + request_hook=_client_request_hook, ) diff --git a/packages/google-api-core/tests/unit/test_observability.py b/packages/google-api-core/tests/unit/test_observability.py index 8e8964e66264..a512ea068981 100644 --- a/packages/google-api-core/tests/unit/test_observability.py +++ b/packages/google-api-core/tests/unit/test_observability.py @@ -162,7 +162,8 @@ def test_get_otel_interceptor_enabled(monkeypatch): assert callable(interceptor) mock_otel_grpc.client_interceptor.assert_called_once_with( - tracer_provider=mock_tracer_provider + tracer_provider=mock_tracer_provider, + request_hook=_observability._client_request_hook, ) result = interceptor(mock_raw_channel) @@ -251,5 +252,47 @@ def test_get_otel_async_interceptor_enabled(monkeypatch): result = _observability.get_otel_async_interceptor(client_options=options) assert result is mock_async_interceptors mock_otel_grpc.aio_client_interceptors.assert_called_once_with( - tracer_provider=mock_tracer_provider + tracer_provider=mock_tracer_provider, + request_hook=_observability._client_request_hook, ) + + +def test_extract_t4_attributes(): + """Proves that _extract_t4_attributes correctly extracts GCP resource name, + parent, and project ID from gRPC request objects. + """ + assert _observability._extract_t4_attributes(None) == {} + + # With name + req_name = mock.Mock(spec=["name"], name="req_name") + req_name.name = "projects/my-project/secrets/my-secret" + attrs = _observability._extract_t4_attributes(req_name) + assert attrs["gcp.resource.name"] == "projects/my-project/secrets/my-secret" + assert attrs["gcp.project_id"] == "my-project" + + # With parent + req_parent = mock.Mock(spec=["parent"], name="req_parent") + req_parent.parent = "projects/parent-project" + attrs = _observability._extract_t4_attributes(req_parent) + assert attrs["gcp.resource.parent"] == "projects/parent-project" + assert attrs["gcp.project_id"] == "parent-project" + + +def test_client_request_hook(): + """Proves that _client_request_hook attaches extracted T4 attributes to recording spans.""" + # Non-recording span should not set attributes + mock_span_non_rec = mock.Mock() + mock_span_non_rec.is_recording.return_value = False + _observability._client_request_hook(mock_span_non_rec, mock.Mock()) + mock_span_non_rec.set_attribute.assert_not_called() + + # Recording span should set attributes + mock_span_rec = mock.Mock() + mock_span_rec.is_recording.return_value = True + req = mock.Mock(name="req") + req.name = "projects/my-proj/secrets/s1" + _observability._client_request_hook(mock_span_rec, req) + mock_span_rec.set_attribute.assert_any_call( + "gcp.resource.name", "projects/my-proj/secrets/s1" + ) + mock_span_rec.set_attribute.assert_any_call("gcp.project_id", "my-proj") From 64965bd69d1a1326105bd331d92e9dd655ad9ba4 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Wed, 9 Sep 2026 08:42:46 -0400 Subject: [PATCH 02/12] feat(core): implement complete T4 gRPC telemetry capture and response hook - Add rpc.system.name: 'grpc' - Extract server.address and server.port from client options endpoint - Extract gcp.grpc.resend_count from request resend count - Extract gcp.resource.destination.id from request name or parent - Add _client_response_hook for status code, error.type, and status.message - Plumb response_hook into get_otel_interceptor and get_otel_async_interceptor --- .../google/api_core/_observability.py | 158 ++++++++++++++---- 1 file changed, 126 insertions(+), 32 deletions(-) diff --git a/packages/google-api-core/google/api_core/_observability.py b/packages/google-api-core/google/api_core/_observability.py index 262b9beda34d..b3309bae8a63 100644 --- a/packages/google-api-core/google/api_core/_observability.py +++ b/packages/google-api-core/google/api_core/_observability.py @@ -25,7 +25,7 @@ if TYPE_CHECKING: # flake8: grpc, trace, and ClientInterceptor are imported only for static analysis and type annotations - # The `# noqa: F401` comment avoids flake8 "imported but not used" errors. + # The 'noqa: F401' comment avoids flake8 "imported but not used" errors. import grpc # noqa: F401 import opentelemetry.trace # noqa: F401 @@ -64,8 +64,56 @@ def is_otel_capabilities_enabled( return False +_STATUS_CODE_NAMES = { + 0: "OK", + 1: "CANCELLED", + 2: "UNKNOWN", + 3: "INVALID_ARGUMENT", + 4: "DEADLINE_EXCEEDED", + 5: "NOT_FOUND", + 6: "ALREADY_EXISTS", + 7: "PERMISSION_DENIED", + 8: "RESOURCE_EXHAUSTED", + 9: "FAILED_PRECONDITION", + 10: "ABORTED", + 11: "OUT_OF_RANGE", + 12: "UNIMPLEMENTED", + 13: "INTERNAL", + 14: "UNAVAILABLE", + 15: "DATA_LOSS", + 16: "UNAUTHENTICATED", +} + + +def _extract_endpoint_attributes( + client_options: ClientOptions | dict[str, Any] | None = None, +) -> dict[str, Any]: + """Extracts server.address and server.port from client options if present.""" + attrs: dict[str, Any] = {} + endpoint = None + if isinstance(client_options, dict): + endpoint = client_options.get("api_endpoint") + elif client_options is not None: + endpoint = getattr(client_options, "api_endpoint", None) + + if endpoint and isinstance(endpoint, str): + clean = endpoint.replace("http://", "").replace("https://", "").strip("/") + if clean: + if ":" in clean: + host, port_str = clean.split(":", 1) + attrs["server.address"] = host + try: + attrs["server.port"] = int(port_str) + except ValueError: + attrs["server.port"] = 443 + else: + attrs["server.address"] = clean + attrs["server.port"] = 443 + return attrs + + def _extract_t4_attributes(request: Any) -> dict[str, Any]: - """Extracts Google Cloud semantic and resource attributes from a gRPC request object. + """Extracts Google Cloud T4 semantic and resource attributes from a gRPC request object. Args: request: The gRPC request object. @@ -73,44 +121,74 @@ def _extract_t4_attributes(request: Any) -> dict[str, Any]: Returns: dict[str, Any]: A dictionary of semantic attributes. """ - attrs: dict[str, Any] = {} + attrs: dict[str, Any] = { + "rpc.system.name": "grpc", + } if request is None: return attrs - name = getattr(request, "name", None) - if name and isinstance(name, str): - attrs["gcp.resource.name"] = name - if "projects/" in name: - parts = name.split("/") - try: - idx = parts.index("projects") - if idx + 1 < len(parts): - attrs["gcp.project_id"] = parts[idx + 1] - except ValueError: - pass + resend_count = getattr(request, "resend_count", None) + if isinstance(resend_count, int) and resend_count > 0: + attrs["gcp.grpc.resend_count"] = resend_count - parent = getattr(request, "parent", None) - if parent and isinstance(parent, str): - attrs["gcp.resource.parent"] = parent - if "gcp.project_id" not in attrs and "projects/" in parent: - parts = parent.split("/") - try: - idx = parts.index("projects") - if idx + 1 < len(parts): - attrs["gcp.project_id"] = parts[idx + 1] - except ValueError: - pass + name = getattr(request, "name", None) + if isinstance(name, str) and name: + attrs["gcp.resource.destination.id"] = name + else: + parent = getattr(request, "parent", None) + if isinstance(parent, str) and parent: + attrs["gcp.resource.destination.id"] = parent return attrs -def _client_request_hook(span: Any, request: Any) -> None: - """OpenTelemetry client request hook to inject GCP resource attributes into the span.""" +def _make_client_request_hook( + endpoint_attrs: dict[str, Any] | None = None, +) -> Callable[[Any, Any], None]: + """Creates an OpenTelemetry client request hook with optional endpoint attributes.""" + + def client_request_hook(span: Any, request: Any) -> None: + if span is None or not getattr(span, "is_recording", lambda: True)(): + return + attrs = _extract_t4_attributes(request) + if endpoint_attrs: + attrs.update(endpoint_attrs) + for key, value in attrs.items(): + span.set_attribute(key, value) + + return client_request_hook + + +_client_request_hook = _make_client_request_hook() + + +def _client_response_hook(span: Any, response: Any) -> None: + """OpenTelemetry client response hook to inject gRPC response status attributes into the span.""" if span is None or not getattr(span, "is_recording", lambda: True)(): return - attrs = _extract_t4_attributes(request) - for key, value in attrs.items(): - span.set_attribute(key, value) + + status_str = "OK" + code_fn = getattr(response, "code", None) + if callable(code_fn): + try: + code_val = code_fn() + status_str = getattr(code_val, "name", None) or _STATUS_CODE_NAMES.get( + code_val, str(code_val) + ) + except Exception: + pass + + span.set_attribute("rpc.response.status_code", status_str) + if status_str != "OK": + span.set_attribute("error.type", status_str) + details_fn = getattr(response, "details", None) + if callable(details_fn): + try: + details = details_fn() + if details: + span.set_attribute("status.message", str(details)) + except Exception: + pass def _get_tracer_provider( @@ -150,9 +228,17 @@ def get_otel_interceptor( import opentelemetry.instrumentation.grpc as otel_grpc # type: ignore[import-not-found] + endpoint_attrs = _extract_endpoint_attributes(client_options) + request_hook = ( + _make_client_request_hook(endpoint_attrs) + if endpoint_attrs + else _client_request_hook + ) + interceptor: ClientInterceptor = otel_grpc.client_interceptor( tracer_provider=_get_tracer_provider(client_options), - request_hook=_client_request_hook, + request_hook=request_hook, + response_hook=_client_response_hook, ) def otel_interceptor(channel: grpc.Channel) -> grpc.Channel: @@ -180,7 +266,15 @@ def get_otel_async_interceptor( # Ignored by mypy: Optional dependency only loaded if early-return is skipped import opentelemetry.instrumentation.grpc as otel_grpc # type: ignore[import-not-found] + endpoint_attrs = _extract_endpoint_attributes(client_options) + request_hook = ( + _make_client_request_hook(endpoint_attrs) + if endpoint_attrs + else _client_request_hook + ) + return otel_grpc.aio_client_interceptors( tracer_provider=_get_tracer_provider(client_options), - request_hook=_client_request_hook, + request_hook=request_hook, + response_hook=_client_response_hook, ) From cc178a248c138580a1f2cbcf88b9e21c71056234 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Wed, 9 Sep 2026 08:42:52 -0400 Subject: [PATCH 03/12] test(core): add comprehensive unit tests for T4 gRPC telemetry and hooks - Test endpoint attribute parsing across host/port variations - Test destination id and resend count extraction - Test client request and response hooks covering all status and error cases - Test interceptor creation and custom endpoint attribute propagation - Achieve 100% statement and branch coverage on _observability.py --- .../tests/unit/test_observability.py | 279 ++++++++++++++++-- 1 file changed, 256 insertions(+), 23 deletions(-) diff --git a/packages/google-api-core/tests/unit/test_observability.py b/packages/google-api-core/tests/unit/test_observability.py index a512ea068981..5bfa72df64cd 100644 --- a/packages/google-api-core/tests/unit/test_observability.py +++ b/packages/google-api-core/tests/unit/test_observability.py @@ -13,6 +13,7 @@ # limitations under the License. import sys +import types from unittest import mock import pytest @@ -164,6 +165,7 @@ def test_get_otel_interceptor_enabled(monkeypatch): mock_otel_grpc.client_interceptor.assert_called_once_with( tracer_provider=mock_tracer_provider, request_hook=_observability._client_request_hook, + response_hook=_observability._client_response_hook, ) result = interceptor(mock_raw_channel) @@ -254,28 +256,84 @@ def test_get_otel_async_interceptor_enabled(monkeypatch): mock_otel_grpc.aio_client_interceptors.assert_called_once_with( tracer_provider=mock_tracer_provider, request_hook=_observability._client_request_hook, + response_hook=_observability._client_response_hook, ) -def test_extract_t4_attributes(): - """Proves that _extract_t4_attributes correctly extracts GCP resource name, - parent, and project ID from gRPC request objects. - """ - assert _observability._extract_t4_attributes(None) == {} - - # With name - req_name = mock.Mock(spec=["name"], name="req_name") - req_name.name = "projects/my-project/secrets/my-secret" - attrs = _observability._extract_t4_attributes(req_name) - assert attrs["gcp.resource.name"] == "projects/my-project/secrets/my-secret" - assert attrs["gcp.project_id"] == "my-project" +def test_extract_endpoint_attributes(): + """Proves that _extract_endpoint_attributes correctly parses server.address and server.port.""" + # None or empty options + assert _observability._extract_endpoint_attributes(None) == {} + assert _observability._extract_endpoint_attributes({}) == {} + assert ( + _observability._extract_endpoint_attributes(ClientOptions(api_endpoint=None)) + == {} + ) - # With parent - req_parent = mock.Mock(spec=["parent"], name="req_parent") - req_parent.parent = "projects/parent-project" - attrs = _observability._extract_t4_attributes(req_parent) - assert attrs["gcp.resource.parent"] == "projects/parent-project" - assert attrs["gcp.project_id"] == "parent-project" + # Dict options with standard endpoint + dict_opts = {"api_endpoint": "secretmanager.googleapis.com"} + attrs = _observability._extract_endpoint_attributes(dict_opts) + assert attrs["server.address"] == "secretmanager.googleapis.com" + assert attrs["server.port"] == 443 + + # ClientOptions with custom port + custom_opts = ClientOptions(api_endpoint="https://my-custom-host.com:8443/") + attrs = _observability._extract_endpoint_attributes(custom_opts) + assert attrs["server.address"] == "my-custom-host.com" + assert attrs["server.port"] == 8443 + + # Invalid port string falls back to 443 + invalid_port_opts = ClientOptions(api_endpoint="my-custom-host.com:invalid_port") + attrs = _observability._extract_endpoint_attributes(invalid_port_opts) + assert attrs["server.address"] == "my-custom-host.com" + assert attrs["server.port"] == 443 + + +@pytest.mark.parametrize( + "req,expected_attrs", + [ + (None, {"rpc.system.name": "grpc"}), + (types.SimpleNamespace(), {"rpc.system.name": "grpc"}), + ( + types.SimpleNamespace(name="projects/p1/secrets/s1"), + { + "rpc.system.name": "grpc", + "gcp.resource.destination.id": "projects/p1/secrets/s1", + }, + ), + ( + types.SimpleNamespace(parent="projects/parent-p1"), + { + "rpc.system.name": "grpc", + "gcp.resource.destination.id": "projects/parent-p1", + }, + ), + ( + types.SimpleNamespace( + name="projects/p1/secrets/s1", parent="projects/parent-p1" + ), + { + "rpc.system.name": "grpc", + "gcp.resource.destination.id": "projects/p1/secrets/s1", + }, + ), + ( + types.SimpleNamespace(name="projects/p1/secrets/s1", resend_count=2), + { + "rpc.system.name": "grpc", + "gcp.resource.destination.id": "projects/p1/secrets/s1", + "gcp.grpc.resend_count": 2, + }, + ), + ( + types.SimpleNamespace(resend_count=0), + {"rpc.system.name": "grpc"}, + ), + ], +) +def test_extract_t4_attributes(req, expected_attrs): + """Proves that _extract_t4_attributes extracts all T4 gRPC attributes.""" + assert _observability._extract_t4_attributes(req) == expected_attrs def test_client_request_hook(): @@ -286,13 +344,188 @@ def test_client_request_hook(): _observability._client_request_hook(mock_span_non_rec, mock.Mock()) mock_span_non_rec.set_attribute.assert_not_called() - # Recording span should set attributes + # None span should safely return + _observability._client_request_hook(None, mock.Mock()) + + # Recording span with default hook mock_span_rec = mock.Mock() mock_span_rec.is_recording.return_value = True - req = mock.Mock(name="req") - req.name = "projects/my-proj/secrets/s1" + req = types.SimpleNamespace(name="projects/my-proj/secrets/s1", resend_count=1) _observability._client_request_hook(mock_span_rec, req) + mock_span_rec.set_attribute.assert_any_call("rpc.system.name", "grpc") mock_span_rec.set_attribute.assert_any_call( - "gcp.resource.name", "projects/my-proj/secrets/s1" + "gcp.resource.destination.id", "projects/my-proj/secrets/s1" + ) + mock_span_rec.set_attribute.assert_any_call("gcp.grpc.resend_count", 1) + + # Custom hook with endpoint attributes + endpoint_hook = _observability._make_client_request_hook( + {"server.address": "custom.api.com", "server.port": 443} + ) + mock_span_custom = mock.Mock() + mock_span_custom.is_recording.return_value = True + endpoint_hook(mock_span_custom, req) + mock_span_custom.set_attribute.assert_any_call("server.address", "custom.api.com") + mock_span_custom.set_attribute.assert_any_call("server.port", 443) + + +def test_client_response_hook(): + """Proves that _client_response_hook sets rpc.response.status_code, error.type, and status.message.""" + # Non-recording span should not set attributes + mock_span_non_rec = mock.Mock() + mock_span_non_rec.is_recording.return_value = False + _observability._client_response_hook(mock_span_non_rec, mock.Mock()) + mock_span_non_rec.set_attribute.assert_not_called() + + # None span should safely return + _observability._client_response_hook(None, mock.Mock()) + + # Response with no code method defaults to OK + mock_span_ok = mock.Mock() + mock_span_ok.is_recording.return_value = True + _observability._client_response_hook(mock_span_ok, mock.Mock(spec=[])) + mock_span_ok.set_attribute.assert_called_once_with("rpc.response.status_code", "OK") + + # Response with StatusCode object having name (e.g. OK) + mock_span_code_obj = mock.Mock() + mock_span_code_obj.is_recording.return_value = True + mock_resp_ok = mock.Mock() + mock_code_ok = mock.Mock() + mock_code_ok.name = "OK" + mock_resp_ok.code.return_value = mock_code_ok + _observability._client_response_hook(mock_span_code_obj, mock_resp_ok) + mock_span_code_obj.set_attribute.assert_called_once_with( + "rpc.response.status_code", "OK" + ) + + # Response with error status (e.g. integer 14 -> UNAVAILABLE) and details + mock_span_err = mock.Mock() + mock_span_err.is_recording.return_value = True + mock_resp_err = mock.Mock() + mock_resp_err.code.return_value = 14 + mock_resp_err.details.return_value = "Service temporarily unavailable" + _observability._client_response_hook(mock_span_err, mock_resp_err) + mock_span_err.set_attribute.assert_any_call( + "rpc.response.status_code", "UNAVAILABLE" + ) + mock_span_err.set_attribute.assert_any_call("error.type", "UNAVAILABLE") + mock_span_err.set_attribute.assert_any_call( + "status.message", "Service temporarily unavailable" + ) + + # Response where code() raises an exception is handled gracefully + mock_span_exc = mock.Mock() + mock_span_exc.is_recording.return_value = True + mock_resp_exc = mock.Mock() + mock_resp_exc.code.side_effect = RuntimeError("Broken call") + _observability._client_response_hook(mock_span_exc, mock_resp_exc) + mock_span_exc.set_attribute.assert_called_once_with( + "rpc.response.status_code", "OK" + ) + + # Response with error status but no details method + mock_span_no_det = mock.Mock() + mock_span_no_det.is_recording.return_value = True + mock_resp_no_det = mock.Mock(spec=["code"]) + mock_resp_no_det.code.return_value = 14 + _observability._client_response_hook(mock_span_no_det, mock_resp_no_det) + mock_span_no_det.set_attribute.assert_any_call( + "rpc.response.status_code", "UNAVAILABLE" + ) + mock_span_no_det.set_attribute.assert_any_call("error.type", "UNAVAILABLE") + + # Response with error status where details() returns empty/None + mock_span_empty_det = mock.Mock() + mock_span_empty_det.is_recording.return_value = True + mock_resp_empty_det = mock.Mock() + mock_resp_empty_det.code.return_value = 14 + mock_resp_empty_det.details.return_value = "" + _observability._client_response_hook(mock_span_empty_det, mock_resp_empty_det) + mock_span_empty_det.set_attribute.assert_any_call( + "rpc.response.status_code", "UNAVAILABLE" + ) + + # Response with error status where details() raises an exception + mock_span_exc_det = mock.Mock() + mock_span_exc_det.is_recording.return_value = True + mock_resp_exc_det = mock.Mock() + mock_resp_exc_det.code.return_value = 14 + mock_resp_exc_det.details.side_effect = RuntimeError("Details broken") + _observability._client_response_hook(mock_span_exc_det, mock_resp_exc_det) + mock_span_exc_det.set_attribute.assert_any_call( + "rpc.response.status_code", "UNAVAILABLE" + ) + + +def test_extract_endpoint_attributes_empty_clean(): + """Proves that endpoint consisting only of slashes/protocol results in empty attrs.""" + assert ( + _observability._extract_endpoint_attributes( + ClientOptions(api_endpoint="http:///") + ) + == {} + ) + + +def test_get_otel_interceptor_with_api_endpoint(monkeypatch): + """Proves that get_otel_interceptor injects server.address and server.port when api_endpoint is set.""" + monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true") + options = ClientOptions(api_endpoint="secretmanager.googleapis.com:443") + + mock_otel = mock.Mock() + mock_otel_grpc = mock_otel.instrumentation.grpc + monkeypatch.setitem(sys.modules, "opentelemetry", mock_otel) + monkeypatch.setitem( + sys.modules, "opentelemetry.instrumentation", mock_otel.instrumentation + ) + monkeypatch.setitem( + sys.modules, "opentelemetry.instrumentation.grpc", mock_otel_grpc + ) + + interceptor = _observability.get_otel_interceptor(client_options=options) + assert callable(interceptor) + + # Verify custom request hook was passed + args, kwargs = mock_otel_grpc.client_interceptor.call_args + req_hook = kwargs["request_hook"] + assert req_hook is not _observability._client_request_hook + + # Test invoking the custom hook + mock_span = mock.Mock() + mock_span.is_recording.return_value = True + req_hook(mock_span, None) + mock_span.set_attribute.assert_any_call( + "server.address", "secretmanager.googleapis.com" + ) + mock_span.set_attribute.assert_any_call("server.port", 443) + + +def test_get_otel_async_interceptor_with_api_endpoint(monkeypatch): + """Proves that get_otel_async_interceptor injects server.address and server.port when api_endpoint is set.""" + monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true") + options = ClientOptions(api_endpoint="secretmanager.googleapis.com:8443") + + mock_otel = mock.Mock() + mock_otel_grpc = mock_otel.instrumentation.grpc + monkeypatch.setitem(sys.modules, "opentelemetry", mock_otel) + monkeypatch.setitem( + sys.modules, "opentelemetry.instrumentation", mock_otel.instrumentation + ) + monkeypatch.setitem( + sys.modules, "opentelemetry.instrumentation.grpc", mock_otel_grpc + ) + + result = _observability.get_otel_async_interceptor(client_options=options) + assert result is not None + + args, kwargs = mock_otel_grpc.aio_client_interceptors.call_args + req_hook = kwargs["request_hook"] + assert req_hook is not _observability._client_request_hook + + mock_span = mock.Mock() + mock_span.is_recording.return_value = True + req_hook(mock_span, None) + mock_span.set_attribute.assert_any_call( + "server.address", "secretmanager.googleapis.com" ) - mock_span_rec.set_attribute.assert_any_call("gcp.project_id", "my-proj") + mock_span.set_attribute.assert_any_call("server.port", 8443) From 402ac26afb2893dcd0a7ae089a6d34c5503b399b Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Wed, 9 Sep 2026 09:50:04 -0400 Subject: [PATCH 04/12] refactor(core): adopt explicit _grpc_* naming for request extraction and hooks - Rename _extract_t4_attributes to _extract_grpc_request_attributes - Rename _make_client_request_hook to _make_grpc_client_request_hook - Rename _client_request_hook to _grpc_client_request_hook - Rename _client_response_hook to _grpc_client_response_hook - Preserve generic _extract_endpoint_attributes for shared transport usage --- .../google/api_core/_observability.py | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/google-api-core/google/api_core/_observability.py b/packages/google-api-core/google/api_core/_observability.py index b3309bae8a63..a537c2756207 100644 --- a/packages/google-api-core/google/api_core/_observability.py +++ b/packages/google-api-core/google/api_core/_observability.py @@ -112,7 +112,7 @@ def _extract_endpoint_attributes( return attrs -def _extract_t4_attributes(request: Any) -> dict[str, Any]: +def _extract_grpc_request_attributes(request: Any) -> dict[str, Any]: """Extracts Google Cloud T4 semantic and resource attributes from a gRPC request object. Args: @@ -142,15 +142,15 @@ def _extract_t4_attributes(request: Any) -> dict[str, Any]: return attrs -def _make_client_request_hook( +def _make_grpc_client_request_hook( endpoint_attrs: dict[str, Any] | None = None, ) -> Callable[[Any, Any], None]: - """Creates an OpenTelemetry client request hook with optional endpoint attributes.""" + """Creates an OpenTelemetry gRPC client request hook with optional endpoint attributes.""" def client_request_hook(span: Any, request: Any) -> None: if span is None or not getattr(span, "is_recording", lambda: True)(): return - attrs = _extract_t4_attributes(request) + attrs = _extract_grpc_request_attributes(request) if endpoint_attrs: attrs.update(endpoint_attrs) for key, value in attrs.items(): @@ -159,11 +159,11 @@ def client_request_hook(span: Any, request: Any) -> None: return client_request_hook -_client_request_hook = _make_client_request_hook() +_grpc_client_request_hook = _make_grpc_client_request_hook() -def _client_response_hook(span: Any, response: Any) -> None: - """OpenTelemetry client response hook to inject gRPC response status attributes into the span.""" +def _grpc_client_response_hook(span: Any, response: Any) -> None: + """OpenTelemetry gRPC client response hook to inject response status attributes into the span.""" if span is None or not getattr(span, "is_recording", lambda: True)(): return @@ -230,15 +230,15 @@ def get_otel_interceptor( endpoint_attrs = _extract_endpoint_attributes(client_options) request_hook = ( - _make_client_request_hook(endpoint_attrs) + _make_grpc_client_request_hook(endpoint_attrs) if endpoint_attrs - else _client_request_hook + else _grpc_client_request_hook ) interceptor: ClientInterceptor = otel_grpc.client_interceptor( tracer_provider=_get_tracer_provider(client_options), request_hook=request_hook, - response_hook=_client_response_hook, + response_hook=_grpc_client_response_hook, ) def otel_interceptor(channel: grpc.Channel) -> grpc.Channel: @@ -268,13 +268,13 @@ def get_otel_async_interceptor( endpoint_attrs = _extract_endpoint_attributes(client_options) request_hook = ( - _make_client_request_hook(endpoint_attrs) + _make_grpc_client_request_hook(endpoint_attrs) if endpoint_attrs - else _client_request_hook + else _grpc_client_request_hook ) return otel_grpc.aio_client_interceptors( tracer_provider=_get_tracer_provider(client_options), request_hook=request_hook, - response_hook=_client_response_hook, + response_hook=_grpc_client_response_hook, ) From 1cf49225f40ed5c4f080188491c9ff4d06dd7fc9 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Wed, 9 Sep 2026 09:50:10 -0400 Subject: [PATCH 05/12] test(core): align test names and assertions with _grpc_* naming convention - Rename test_extract_t4_attributes to test_extract_grpc_request_attributes - Rename test_client_request_hook to test_grpc_client_request_hook - Rename test_client_response_hook to test_grpc_client_response_hook - Update interceptor hook references to _grpc_client_* hooks --- .../tests/unit/test_observability.py | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/packages/google-api-core/tests/unit/test_observability.py b/packages/google-api-core/tests/unit/test_observability.py index 5bfa72df64cd..82dd5daa76f8 100644 --- a/packages/google-api-core/tests/unit/test_observability.py +++ b/packages/google-api-core/tests/unit/test_observability.py @@ -164,8 +164,8 @@ def test_get_otel_interceptor_enabled(monkeypatch): mock_otel_grpc.client_interceptor.assert_called_once_with( tracer_provider=mock_tracer_provider, - request_hook=_observability._client_request_hook, - response_hook=_observability._client_response_hook, + request_hook=_observability._grpc_client_request_hook, + response_hook=_observability._grpc_client_response_hook, ) result = interceptor(mock_raw_channel) @@ -255,8 +255,8 @@ def test_get_otel_async_interceptor_enabled(monkeypatch): assert result is mock_async_interceptors mock_otel_grpc.aio_client_interceptors.assert_called_once_with( tracer_provider=mock_tracer_provider, - request_hook=_observability._client_request_hook, - response_hook=_observability._client_response_hook, + request_hook=_observability._grpc_client_request_hook, + response_hook=_observability._grpc_client_response_hook, ) @@ -331,27 +331,27 @@ def test_extract_endpoint_attributes(): ), ], ) -def test_extract_t4_attributes(req, expected_attrs): - """Proves that _extract_t4_attributes extracts all T4 gRPC attributes.""" - assert _observability._extract_t4_attributes(req) == expected_attrs +def test_extract_grpc_request_attributes(req, expected_attrs): + """Proves that _extract_grpc_request_attributes extracts all T4 gRPC attributes.""" + assert _observability._extract_grpc_request_attributes(req) == expected_attrs -def test_client_request_hook(): - """Proves that _client_request_hook attaches extracted T4 attributes to recording spans.""" +def test_grpc_client_request_hook(): + """Proves that _grpc_client_request_hook attaches extracted T4 attributes to recording spans.""" # Non-recording span should not set attributes mock_span_non_rec = mock.Mock() mock_span_non_rec.is_recording.return_value = False - _observability._client_request_hook(mock_span_non_rec, mock.Mock()) + _observability._grpc_client_request_hook(mock_span_non_rec, mock.Mock()) mock_span_non_rec.set_attribute.assert_not_called() # None span should safely return - _observability._client_request_hook(None, mock.Mock()) + _observability._grpc_client_request_hook(None, mock.Mock()) # Recording span with default hook mock_span_rec = mock.Mock() mock_span_rec.is_recording.return_value = True req = types.SimpleNamespace(name="projects/my-proj/secrets/s1", resend_count=1) - _observability._client_request_hook(mock_span_rec, req) + _observability._grpc_client_request_hook(mock_span_rec, req) mock_span_rec.set_attribute.assert_any_call("rpc.system.name", "grpc") mock_span_rec.set_attribute.assert_any_call( "gcp.resource.destination.id", "projects/my-proj/secrets/s1" @@ -359,7 +359,7 @@ def test_client_request_hook(): mock_span_rec.set_attribute.assert_any_call("gcp.grpc.resend_count", 1) # Custom hook with endpoint attributes - endpoint_hook = _observability._make_client_request_hook( + endpoint_hook = _observability._make_grpc_client_request_hook( {"server.address": "custom.api.com", "server.port": 443} ) mock_span_custom = mock.Mock() @@ -369,21 +369,21 @@ def test_client_request_hook(): mock_span_custom.set_attribute.assert_any_call("server.port", 443) -def test_client_response_hook(): - """Proves that _client_response_hook sets rpc.response.status_code, error.type, and status.message.""" +def test_grpc_client_response_hook(): + """Proves that _grpc_client_response_hook sets rpc.response.status_code, error.type, and status.message.""" # Non-recording span should not set attributes mock_span_non_rec = mock.Mock() mock_span_non_rec.is_recording.return_value = False - _observability._client_response_hook(mock_span_non_rec, mock.Mock()) + _observability._grpc_client_response_hook(mock_span_non_rec, mock.Mock()) mock_span_non_rec.set_attribute.assert_not_called() # None span should safely return - _observability._client_response_hook(None, mock.Mock()) + _observability._grpc_client_response_hook(None, mock.Mock()) # Response with no code method defaults to OK mock_span_ok = mock.Mock() mock_span_ok.is_recording.return_value = True - _observability._client_response_hook(mock_span_ok, mock.Mock(spec=[])) + _observability._grpc_client_response_hook(mock_span_ok, mock.Mock(spec=[])) mock_span_ok.set_attribute.assert_called_once_with("rpc.response.status_code", "OK") # Response with StatusCode object having name (e.g. OK) @@ -393,7 +393,7 @@ def test_client_response_hook(): mock_code_ok = mock.Mock() mock_code_ok.name = "OK" mock_resp_ok.code.return_value = mock_code_ok - _observability._client_response_hook(mock_span_code_obj, mock_resp_ok) + _observability._grpc_client_response_hook(mock_span_code_obj, mock_resp_ok) mock_span_code_obj.set_attribute.assert_called_once_with( "rpc.response.status_code", "OK" ) @@ -404,7 +404,7 @@ def test_client_response_hook(): mock_resp_err = mock.Mock() mock_resp_err.code.return_value = 14 mock_resp_err.details.return_value = "Service temporarily unavailable" - _observability._client_response_hook(mock_span_err, mock_resp_err) + _observability._grpc_client_response_hook(mock_span_err, mock_resp_err) mock_span_err.set_attribute.assert_any_call( "rpc.response.status_code", "UNAVAILABLE" ) @@ -418,7 +418,7 @@ def test_client_response_hook(): mock_span_exc.is_recording.return_value = True mock_resp_exc = mock.Mock() mock_resp_exc.code.side_effect = RuntimeError("Broken call") - _observability._client_response_hook(mock_span_exc, mock_resp_exc) + _observability._grpc_client_response_hook(mock_span_exc, mock_resp_exc) mock_span_exc.set_attribute.assert_called_once_with( "rpc.response.status_code", "OK" ) @@ -428,7 +428,7 @@ def test_client_response_hook(): mock_span_no_det.is_recording.return_value = True mock_resp_no_det = mock.Mock(spec=["code"]) mock_resp_no_det.code.return_value = 14 - _observability._client_response_hook(mock_span_no_det, mock_resp_no_det) + _observability._grpc_client_response_hook(mock_span_no_det, mock_resp_no_det) mock_span_no_det.set_attribute.assert_any_call( "rpc.response.status_code", "UNAVAILABLE" ) @@ -440,7 +440,7 @@ def test_client_response_hook(): mock_resp_empty_det = mock.Mock() mock_resp_empty_det.code.return_value = 14 mock_resp_empty_det.details.return_value = "" - _observability._client_response_hook(mock_span_empty_det, mock_resp_empty_det) + _observability._grpc_client_response_hook(mock_span_empty_det, mock_resp_empty_det) mock_span_empty_det.set_attribute.assert_any_call( "rpc.response.status_code", "UNAVAILABLE" ) @@ -451,7 +451,7 @@ def test_client_response_hook(): mock_resp_exc_det = mock.Mock() mock_resp_exc_det.code.return_value = 14 mock_resp_exc_det.details.side_effect = RuntimeError("Details broken") - _observability._client_response_hook(mock_span_exc_det, mock_resp_exc_det) + _observability._grpc_client_response_hook(mock_span_exc_det, mock_resp_exc_det) mock_span_exc_det.set_attribute.assert_any_call( "rpc.response.status_code", "UNAVAILABLE" ) @@ -488,7 +488,7 @@ def test_get_otel_interceptor_with_api_endpoint(monkeypatch): # Verify custom request hook was passed args, kwargs = mock_otel_grpc.client_interceptor.call_args req_hook = kwargs["request_hook"] - assert req_hook is not _observability._client_request_hook + assert req_hook is not _observability._grpc_client_request_hook # Test invoking the custom hook mock_span = mock.Mock() @@ -520,7 +520,7 @@ def test_get_otel_async_interceptor_with_api_endpoint(monkeypatch): args, kwargs = mock_otel_grpc.aio_client_interceptors.call_args req_hook = kwargs["request_hook"] - assert req_hook is not _observability._client_request_hook + assert req_hook is not _observability._grpc_client_request_hook mock_span = mock.Mock() mock_span.is_recording.return_value = True From b514c8a2bd7b7d95c6b457aea1fead54d7ef09d2 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Wed, 9 Sep 2026 20:24:40 -0400 Subject: [PATCH 06/12] feat(core): add url.domain, error attributes, and streamline T4 hooks - Add url.domain extraction from universe_domain or default to googleapis.com - Add _extract_error_attributes helper to extract gcp.errors.domain and gcp.errors.metadata. - Omit server.port when port matches scheme defaults (443 for https/grpc, 80 for http) - Remove redundant _grpc_client_response_hook and _STATUS_CODE_NAMES - Deduplicate name and parent resource lookup for gcp.resource.destination.id - Add comprehensive parametrized unit tests and update interceptor test suites --- .../google/api_core/_observability.py | 165 ++++++----- .../tests/unit/test_observability.py | 269 ++++++++++-------- 2 files changed, 229 insertions(+), 205 deletions(-) diff --git a/packages/google-api-core/google/api_core/_observability.py b/packages/google-api-core/google/api_core/_observability.py index a537c2756207..33838cf19f01 100644 --- a/packages/google-api-core/google/api_core/_observability.py +++ b/packages/google-api-core/google/api_core/_observability.py @@ -18,6 +18,7 @@ from __future__ import annotations +import urllib.parse from typing import TYPE_CHECKING, Any, Callable, Sequence from google.api_core import _feature_gating_helpers @@ -64,51 +65,43 @@ def is_otel_capabilities_enabled( return False -_STATUS_CODE_NAMES = { - 0: "OK", - 1: "CANCELLED", - 2: "UNKNOWN", - 3: "INVALID_ARGUMENT", - 4: "DEADLINE_EXCEEDED", - 5: "NOT_FOUND", - 6: "ALREADY_EXISTS", - 7: "PERMISSION_DENIED", - 8: "RESOURCE_EXHAUSTED", - 9: "FAILED_PRECONDITION", - 10: "ABORTED", - 11: "OUT_OF_RANGE", - 12: "UNIMPLEMENTED", - 13: "INTERNAL", - 14: "UNAVAILABLE", - 15: "DATA_LOSS", - 16: "UNAUTHENTICATED", -} - - def _extract_endpoint_attributes( client_options: ClientOptions | dict[str, Any] | None = None, ) -> dict[str, Any]: - """Extracts server.address and server.port from client options if present.""" + """Extracts server.address, server.port (if non-default), and url.domain from client options if present. + + Args: + client_options: The client options object or dictionary. + + Returns: + dict[str, Any]: A dictionary containing url.domain and, if an api_endpoint is configured, + server.address and non-default server.port. + """ attrs: dict[str, Any] = {} endpoint = None + universe_domain = None + if isinstance(client_options, dict): endpoint = client_options.get("api_endpoint") + universe_domain = client_options.get("universe_domain") elif client_options is not None: endpoint = getattr(client_options, "api_endpoint", None) + universe_domain = getattr(client_options, "universe_domain", None) + + attrs["url.domain"] = universe_domain or "googleapis.com" if endpoint and isinstance(endpoint, str): - clean = endpoint.replace("http://", "").replace("https://", "").strip("/") - if clean: - if ":" in clean: - host, port_str = clean.split(":", 1) - attrs["server.address"] = host - try: - attrs["server.port"] = int(port_str) - except ValueError: - attrs["server.port"] = 443 - else: - attrs["server.address"] = clean - attrs["server.port"] = 443 + target = endpoint if "//" in endpoint else f"//{endpoint}" + parsed = urllib.parse.urlsplit(target) + if parsed.hostname: + attrs["server.address"] = parsed.hostname + if parsed.port: + scheme = parsed.scheme.lower() + is_default_port = (parsed.port == 443 and scheme in ("https", "")) or ( + parsed.port == 80 and scheme == "http" + ) + if not is_default_port: + attrs["server.port"] = parsed.port return attrs @@ -131,13 +124,46 @@ def _extract_grpc_request_attributes(request: Any) -> dict[str, Any]: if isinstance(resend_count, int) and resend_count > 0: attrs["gcp.grpc.resend_count"] = resend_count - name = getattr(request, "name", None) - if isinstance(name, str) and name: - attrs["gcp.resource.destination.id"] = name - else: - parent = getattr(request, "parent", None) - if isinstance(parent, str) and parent: - attrs["gcp.resource.destination.id"] = parent + resource_id = getattr(request, "name", None) or getattr(request, "parent", None) + if isinstance(resource_id, str) and resource_id: + attrs["gcp.resource.destination.id"] = resource_id + + return attrs + + +def _extract_error_attributes(exc: Any) -> dict[str, Any]: + """Extracts gcp.errors.domain, gcp.errors.metadata.*, and error.type from an exception or ErrorInfo. + + Args: + exc: An exception (such as GoogleAPICallError or grpc.RpcError) or ErrorInfo object. + + Returns: + dict[str, Any]: Extracted error attributes. + """ + attrs: dict[str, Any] = {} + if exc is None: + return attrs + + error_info = getattr(exc, "error_info", None) + if error_info is None and hasattr(exc, "trailing_metadata"): + try: + from google.api_core import exceptions + + _, error_info = exceptions._parse_grpc_error_details(exc) + except Exception: + pass + + if error_info is not None: + domain = getattr(error_info, "domain", None) + if domain and isinstance(domain, str): + attrs["gcp.errors.domain"] = domain + reason = getattr(error_info, "reason", None) + if reason and isinstance(reason, str): + attrs["error.type"] = reason + metadata = getattr(error_info, "metadata", None) + if metadata and hasattr(metadata, "items"): + for k, v in metadata.items(): + attrs[f"gcp.errors.metadata.{k}"] = str(v) return attrs @@ -145,14 +171,22 @@ def _extract_grpc_request_attributes(request: Any) -> dict[str, Any]: def _make_grpc_client_request_hook( endpoint_attrs: dict[str, Any] | None = None, ) -> Callable[[Any, Any], None]: - """Creates an OpenTelemetry gRPC client request hook with optional endpoint attributes.""" + """Creates an OpenTelemetry gRPC client request hook with optional endpoint attributes. + + Args: + endpoint_attrs: Optional static endpoint attributes to attach to every span. + + Returns: + Callable[[Any, Any], None]: The request hook callback. + """ + static_attrs = dict(endpoint_attrs) if endpoint_attrs else {} def client_request_hook(span: Any, request: Any) -> None: if span is None or not getattr(span, "is_recording", lambda: True)(): return attrs = _extract_grpc_request_attributes(request) - if endpoint_attrs: - attrs.update(endpoint_attrs) + if static_attrs: + attrs.update(static_attrs) for key, value in attrs.items(): span.set_attribute(key, value) @@ -162,35 +196,6 @@ def client_request_hook(span: Any, request: Any) -> None: _grpc_client_request_hook = _make_grpc_client_request_hook() -def _grpc_client_response_hook(span: Any, response: Any) -> None: - """OpenTelemetry gRPC client response hook to inject response status attributes into the span.""" - if span is None or not getattr(span, "is_recording", lambda: True)(): - return - - status_str = "OK" - code_fn = getattr(response, "code", None) - if callable(code_fn): - try: - code_val = code_fn() - status_str = getattr(code_val, "name", None) or _STATUS_CODE_NAMES.get( - code_val, str(code_val) - ) - except Exception: - pass - - span.set_attribute("rpc.response.status_code", status_str) - if status_str != "OK": - span.set_attribute("error.type", status_str) - details_fn = getattr(response, "details", None) - if callable(details_fn): - try: - details = details_fn() - if details: - span.set_attribute("status.message", str(details)) - except Exception: - pass - - def _get_tracer_provider( client_options: ClientOptions | dict[str, Any] | None = None, ) -> opentelemetry.trace.TracerProvider | None: @@ -229,16 +234,11 @@ def get_otel_interceptor( import opentelemetry.instrumentation.grpc as otel_grpc # type: ignore[import-not-found] endpoint_attrs = _extract_endpoint_attributes(client_options) - request_hook = ( - _make_grpc_client_request_hook(endpoint_attrs) - if endpoint_attrs - else _grpc_client_request_hook - ) + request_hook = _make_grpc_client_request_hook(endpoint_attrs) interceptor: ClientInterceptor = otel_grpc.client_interceptor( tracer_provider=_get_tracer_provider(client_options), request_hook=request_hook, - response_hook=_grpc_client_response_hook, ) def otel_interceptor(channel: grpc.Channel) -> grpc.Channel: @@ -267,14 +267,9 @@ def get_otel_async_interceptor( import opentelemetry.instrumentation.grpc as otel_grpc # type: ignore[import-not-found] endpoint_attrs = _extract_endpoint_attributes(client_options) - request_hook = ( - _make_grpc_client_request_hook(endpoint_attrs) - if endpoint_attrs - else _grpc_client_request_hook - ) + request_hook = _make_grpc_client_request_hook(endpoint_attrs) return otel_grpc.aio_client_interceptors( tracer_provider=_get_tracer_provider(client_options), request_hook=request_hook, - response_hook=_grpc_client_response_hook, ) diff --git a/packages/google-api-core/tests/unit/test_observability.py b/packages/google-api-core/tests/unit/test_observability.py index 82dd5daa76f8..4ae31e9c163e 100644 --- a/packages/google-api-core/tests/unit/test_observability.py +++ b/packages/google-api-core/tests/unit/test_observability.py @@ -164,9 +164,13 @@ def test_get_otel_interceptor_enabled(monkeypatch): mock_otel_grpc.client_interceptor.assert_called_once_with( tracer_provider=mock_tracer_provider, - request_hook=_observability._grpc_client_request_hook, - response_hook=_observability._grpc_client_response_hook, + request_hook=mock.ANY, ) + req_hook = mock_otel_grpc.client_interceptor.call_args[1]["request_hook"] + mock_span = mock.Mock() + mock_span.is_recording.return_value = True + req_hook(mock_span, None) + mock_span.set_attribute.assert_any_call("url.domain", "googleapis.com") result = interceptor(mock_raw_channel) assert result is mock_wrapped_channel @@ -255,38 +259,76 @@ def test_get_otel_async_interceptor_enabled(monkeypatch): assert result is mock_async_interceptors mock_otel_grpc.aio_client_interceptors.assert_called_once_with( tracer_provider=mock_tracer_provider, - request_hook=_observability._grpc_client_request_hook, - response_hook=_observability._grpc_client_response_hook, - ) - - -def test_extract_endpoint_attributes(): - """Proves that _extract_endpoint_attributes correctly parses server.address and server.port.""" - # None or empty options - assert _observability._extract_endpoint_attributes(None) == {} - assert _observability._extract_endpoint_attributes({}) == {} - assert ( - _observability._extract_endpoint_attributes(ClientOptions(api_endpoint=None)) - == {} + request_hook=mock.ANY, ) + req_hook = mock_otel_grpc.aio_client_interceptors.call_args[1]["request_hook"] + mock_span = mock.Mock() + mock_span.is_recording.return_value = True + req_hook(mock_span, None) + mock_span.set_attribute.assert_any_call("url.domain", "googleapis.com") - # Dict options with standard endpoint - dict_opts = {"api_endpoint": "secretmanager.googleapis.com"} - attrs = _observability._extract_endpoint_attributes(dict_opts) - assert attrs["server.address"] == "secretmanager.googleapis.com" - assert attrs["server.port"] == 443 - - # ClientOptions with custom port - custom_opts = ClientOptions(api_endpoint="https://my-custom-host.com:8443/") - attrs = _observability._extract_endpoint_attributes(custom_opts) - assert attrs["server.address"] == "my-custom-host.com" - assert attrs["server.port"] == 8443 - # Invalid port string falls back to 443 - invalid_port_opts = ClientOptions(api_endpoint="my-custom-host.com:invalid_port") - attrs = _observability._extract_endpoint_attributes(invalid_port_opts) - assert attrs["server.address"] == "my-custom-host.com" - assert attrs["server.port"] == 443 +@pytest.mark.parametrize( + "client_options,expected_attrs", + [ + (None, {"url.domain": "googleapis.com"}), + ({}, {"url.domain": "googleapis.com"}), + (ClientOptions(api_endpoint=None), {"url.domain": "googleapis.com"}), + ({"universe_domain": "myuniverse.com"}, {"url.domain": "myuniverse.com"}), + ( + ClientOptions(universe_domain="custom.domain"), + {"url.domain": "custom.domain"}, + ), + ( + {"api_endpoint": "secretmanager.googleapis.com"}, + { + "server.address": "secretmanager.googleapis.com", + "url.domain": "googleapis.com", + }, + ), + ( + {"api_endpoint": "secretmanager.googleapis.com:443"}, + { + "server.address": "secretmanager.googleapis.com", + "url.domain": "googleapis.com", + }, + ), + ( + {"api_endpoint": "https://secretmanager.googleapis.com:443"}, + { + "server.address": "secretmanager.googleapis.com", + "url.domain": "googleapis.com", + }, + ), + ( + {"api_endpoint": "http://localhost:80"}, + {"server.address": "localhost", "url.domain": "googleapis.com"}, + ), + ( + ClientOptions(api_endpoint="https://my-custom-host.com:8443/"), + { + "server.address": "my-custom-host.com", + "server.port": 8443, + "url.domain": "googleapis.com", + }, + ), + ( + ClientOptions(api_endpoint="http://[::1]:8080"), + { + "server.address": "::1", + "server.port": 8080, + "url.domain": "googleapis.com", + }, + ), + ( + ClientOptions(api_endpoint="http:///"), + {"url.domain": "googleapis.com"}, + ), + ], +) +def test_extract_endpoint_attributes(client_options, expected_attrs): + """Proves that _extract_endpoint_attributes correctly parses server.address, non-default server.port, and url.domain.""" + assert _observability._extract_endpoint_attributes(client_options) == expected_attrs @pytest.mark.parametrize( @@ -369,108 +411,90 @@ def test_grpc_client_request_hook(): mock_span_custom.set_attribute.assert_any_call("server.port", 443) -def test_grpc_client_response_hook(): - """Proves that _grpc_client_response_hook sets rpc.response.status_code, error.type, and status.message.""" - # Non-recording span should not set attributes - mock_span_non_rec = mock.Mock() - mock_span_non_rec.is_recording.return_value = False - _observability._grpc_client_response_hook(mock_span_non_rec, mock.Mock()) - mock_span_non_rec.set_attribute.assert_not_called() +def test_extract_error_attributes_none(): + """Proves that _extract_error_attributes returns an empty dict when exception is None.""" + assert _observability._extract_error_attributes(None) == {} - # None span should safely return - _observability._grpc_client_response_hook(None, mock.Mock()) - - # Response with no code method defaults to OK - mock_span_ok = mock.Mock() - mock_span_ok.is_recording.return_value = True - _observability._grpc_client_response_hook(mock_span_ok, mock.Mock(spec=[])) - mock_span_ok.set_attribute.assert_called_once_with("rpc.response.status_code", "OK") - - # Response with StatusCode object having name (e.g. OK) - mock_span_code_obj = mock.Mock() - mock_span_code_obj.is_recording.return_value = True - mock_resp_ok = mock.Mock() - mock_code_ok = mock.Mock() - mock_code_ok.name = "OK" - mock_resp_ok.code.return_value = mock_code_ok - _observability._grpc_client_response_hook(mock_span_code_obj, mock_resp_ok) - mock_span_code_obj.set_attribute.assert_called_once_with( - "rpc.response.status_code", "OK" - ) - # Response with error status (e.g. integer 14 -> UNAVAILABLE) and details - mock_span_err = mock.Mock() - mock_span_err.is_recording.return_value = True - mock_resp_err = mock.Mock() - mock_resp_err.code.return_value = 14 - mock_resp_err.details.return_value = "Service temporarily unavailable" - _observability._grpc_client_response_hook(mock_span_err, mock_resp_err) - mock_span_err.set_attribute.assert_any_call( - "rpc.response.status_code", "UNAVAILABLE" - ) - mock_span_err.set_attribute.assert_any_call("error.type", "UNAVAILABLE") - mock_span_err.set_attribute.assert_any_call( - "status.message", "Service temporarily unavailable" +def test_extract_error_attributes_standard_exception(): + """Proves that _extract_error_attributes returns an empty dict for standard exceptions without ErrorInfo.""" + assert ( + _observability._extract_error_attributes(ValueError("unexpected error")) == {} ) - # Response where code() raises an exception is handled gracefully - mock_span_exc = mock.Mock() - mock_span_exc.is_recording.return_value = True - mock_resp_exc = mock.Mock() - mock_resp_exc.code.side_effect = RuntimeError("Broken call") - _observability._grpc_client_response_hook(mock_span_exc, mock_resp_exc) - mock_span_exc.set_attribute.assert_called_once_with( - "rpc.response.status_code", "OK" - ) - # Response with error status but no details method - mock_span_no_det = mock.Mock() - mock_span_no_det.is_recording.return_value = True - mock_resp_no_det = mock.Mock(spec=["code"]) - mock_resp_no_det.code.return_value = 14 - _observability._grpc_client_response_hook(mock_span_no_det, mock_resp_no_det) - mock_span_no_det.set_attribute.assert_any_call( - "rpc.response.status_code", "UNAVAILABLE" +def test_extract_error_attributes_with_error_info(): + """Proves that _extract_error_attributes extracts domain, error.type, and metadata from ErrorInfo.""" + error_info = types.SimpleNamespace( + domain="googleapis.com", + reason="SERVICE_DISABLED", + metadata={ + "service": "secretmanager.googleapis.com", + "consumer": "projects/123", + }, ) - mock_span_no_det.set_attribute.assert_any_call("error.type", "UNAVAILABLE") - - # Response with error status where details() returns empty/None - mock_span_empty_det = mock.Mock() - mock_span_empty_det.is_recording.return_value = True - mock_resp_empty_det = mock.Mock() - mock_resp_empty_det.code.return_value = 14 - mock_resp_empty_det.details.return_value = "" - _observability._grpc_client_response_hook(mock_span_empty_det, mock_resp_empty_det) - mock_span_empty_det.set_attribute.assert_any_call( - "rpc.response.status_code", "UNAVAILABLE" + exc = types.SimpleNamespace(error_info=error_info) + attrs = _observability._extract_error_attributes(exc) + assert attrs == { + "gcp.errors.domain": "googleapis.com", + "error.type": "SERVICE_DISABLED", + "gcp.errors.metadata.service": "secretmanager.googleapis.com", + "gcp.errors.metadata.consumer": "projects/123", + } + + +def test_extract_error_attributes_from_grpc_trailing_metadata(monkeypatch): + """Proves that _extract_error_attributes parses error_info from gRPC trailing metadata.""" + from google.api_core import exceptions + + mock_exc = mock.Mock() + mock_exc.error_info = None + mock_exc.trailing_metadata = mock.Mock() + + parsed_error_info = types.SimpleNamespace( + domain="googleapis.com", + reason="RESOURCE_EXHAUSTED", + metadata={"quota_limit": "100"}, ) - # Response with error status where details() raises an exception - mock_span_exc_det = mock.Mock() - mock_span_exc_det.is_recording.return_value = True - mock_resp_exc_det = mock.Mock() - mock_resp_exc_det.code.return_value = 14 - mock_resp_exc_det.details.side_effect = RuntimeError("Details broken") - _observability._grpc_client_response_hook(mock_span_exc_det, mock_resp_exc_det) - mock_span_exc_det.set_attribute.assert_any_call( - "rpc.response.status_code", "UNAVAILABLE" + monkeypatch.setattr( + exceptions, + "_parse_grpc_error_details", + mock.Mock(return_value=(None, parsed_error_info)), ) + attrs = _observability._extract_error_attributes(mock_exc) + assert attrs == { + "gcp.errors.domain": "googleapis.com", + "error.type": "RESOURCE_EXHAUSTED", + "gcp.errors.metadata.quota_limit": "100", + } -def test_extract_endpoint_attributes_empty_clean(): - """Proves that endpoint consisting only of slashes/protocol results in empty attrs.""" - assert ( - _observability._extract_endpoint_attributes( - ClientOptions(api_endpoint="http:///") - ) - == {} + +def test_extract_error_attributes_trailing_metadata_failure(monkeypatch): + """Proves that _extract_error_attributes safely handles exceptions during trailing metadata parsing.""" + from google.api_core import exceptions + + mock_exc = mock.Mock() + mock_exc.error_info = None + mock_exc.trailing_metadata = mock.Mock() + + monkeypatch.setattr( + exceptions, + "_parse_grpc_error_details", + mock.Mock(side_effect=RuntimeError("Parse failed")), ) + assert _observability._extract_error_attributes(mock_exc) == {} + def test_get_otel_interceptor_with_api_endpoint(monkeypatch): - """Proves that get_otel_interceptor injects server.address and server.port when api_endpoint is set.""" + """Proves that get_otel_interceptor injects server.address, server.port, and url.domain when api_endpoint is set.""" monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true") - options = ClientOptions(api_endpoint="secretmanager.googleapis.com:443") + options = ClientOptions( + api_endpoint="secretmanager.googleapis.com:8443", + universe_domain="custom-domain.com", + ) mock_otel = mock.Mock() mock_otel_grpc = mock_otel.instrumentation.grpc @@ -497,13 +521,17 @@ def test_get_otel_interceptor_with_api_endpoint(monkeypatch): mock_span.set_attribute.assert_any_call( "server.address", "secretmanager.googleapis.com" ) - mock_span.set_attribute.assert_any_call("server.port", 443) + mock_span.set_attribute.assert_any_call("server.port", 8443) + mock_span.set_attribute.assert_any_call("url.domain", "custom-domain.com") def test_get_otel_async_interceptor_with_api_endpoint(monkeypatch): - """Proves that get_otel_async_interceptor injects server.address and server.port when api_endpoint is set.""" + """Proves that get_otel_async_interceptor injects server.address, server.port, and url.domain when api_endpoint is set.""" monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true") - options = ClientOptions(api_endpoint="secretmanager.googleapis.com:8443") + options = ClientOptions( + api_endpoint="secretmanager.googleapis.com:8443", + universe_domain="custom-domain.com", + ) mock_otel = mock.Mock() mock_otel_grpc = mock_otel.instrumentation.grpc @@ -529,3 +557,4 @@ def test_get_otel_async_interceptor_with_api_endpoint(monkeypatch): "server.address", "secretmanager.googleapis.com" ) mock_span.set_attribute.assert_any_call("server.port", 8443) + mock_span.set_attribute.assert_any_call("url.domain", "custom-domain.com") From bce7090c0623a29ffc0aa884c8525bc81400ed17 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Thu, 10 Sep 2026 05:42:47 -0400 Subject: [PATCH 07/12] feat(core): normalize gRPC span names and eliminate duplicate rpc.system attribute - Strip leading slash from gRPC attempt span names via span.update_name - Set rpc.method to the fully qualified method name per PRD specification - Retain rpc.system.name: 'grpc' and remove legacy rpc.system attribute to avoid duplication - Update unit tests to verify span name normalization and attribute deduplication --- .../google/api_core/_observability.py | 19 +++++++++++++ .../tests/unit/test_observability.py | 27 ++++++++++++++++--- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/packages/google-api-core/google/api_core/_observability.py b/packages/google-api-core/google/api_core/_observability.py index 33838cf19f01..5e8f574ba06f 100644 --- a/packages/google-api-core/google/api_core/_observability.py +++ b/packages/google-api-core/google/api_core/_observability.py @@ -184,7 +184,26 @@ def _make_grpc_client_request_hook( def client_request_hook(span: Any, request: Any) -> None: if span is None or not getattr(span, "is_recording", lambda: True)(): return + + # Upstream opentelemetry-instrumentation-grpc names spans with a leading slash + # (e.g. "/package.Service/Method") and sets only the short name on rpc.method. + # Normalize span.name and rpc.method to the fully-qualified name without leading slash. + span_name = getattr(span, "name", None) + clean_method_name = None + if isinstance(span_name, str) and span_name.startswith("/"): + clean_method_name = span_name.lstrip("/") + if hasattr(span, "update_name"): + span.update_name(clean_method_name) + + # Remove duplicate legacy rpc.system attribute set by stock instrumentation + # in favor of modern rpc.system.name ("grpc") per PRD changelog. + span_attributes = getattr(span, "_attributes", None) + if hasattr(span_attributes, "pop"): + span_attributes.pop("rpc.system", None) + attrs = _extract_grpc_request_attributes(request) + if clean_method_name: + attrs["rpc.method"] = clean_method_name if static_attrs: attrs.update(static_attrs) for key, value in attrs.items(): diff --git a/packages/google-api-core/tests/unit/test_observability.py b/packages/google-api-core/tests/unit/test_observability.py index 4ae31e9c163e..982797aed4ef 100644 --- a/packages/google-api-core/tests/unit/test_observability.py +++ b/packages/google-api-core/tests/unit/test_observability.py @@ -379,7 +379,9 @@ def test_extract_grpc_request_attributes(req, expected_attrs): def test_grpc_client_request_hook(): - """Proves that _grpc_client_request_hook attaches extracted T4 attributes to recording spans.""" + """Proves that _grpc_client_request_hook attaches extracted T4 attributes to recording spans, + normalizes span names, sets fully qualified rpc.method, and removes legacy rpc.system. + """ # Non-recording span should not set attributes mock_span_non_rec = mock.Mock() mock_span_non_rec.is_recording.return_value = False @@ -389,26 +391,45 @@ def test_grpc_client_request_hook(): # None span should safely return _observability._grpc_client_request_hook(None, mock.Mock()) - # Recording span with default hook + # Recording span with default hook, leading slash in span.name, and legacy rpc.system mock_span_rec = mock.Mock() mock_span_rec.is_recording.return_value = True + mock_span_rec.name = ( + "/google.cloud.secretmanager.v1.SecretManagerService/ListSecrets" + ) + mock_span_rec._attributes = {"rpc.system": "grpc"} req = types.SimpleNamespace(name="projects/my-proj/secrets/s1", resend_count=1) + _observability._grpc_client_request_hook(mock_span_rec, req) + + # Verify span name normalized and rpc.method set to fully qualified name + mock_span_rec.update_name.assert_called_once_with( + "google.cloud.secretmanager.v1.SecretManagerService/ListSecrets" + ) + mock_span_rec.set_attribute.assert_any_call( + "rpc.method", "google.cloud.secretmanager.v1.SecretManagerService/ListSecrets" + ) + + # Verify rpc.system.name set and legacy rpc.system popped mock_span_rec.set_attribute.assert_any_call("rpc.system.name", "grpc") + assert "rpc.system" not in mock_span_rec._attributes + mock_span_rec.set_attribute.assert_any_call( "gcp.resource.destination.id", "projects/my-proj/secrets/s1" ) mock_span_rec.set_attribute.assert_any_call("gcp.grpc.resend_count", 1) - # Custom hook with endpoint attributes + # Custom hook with endpoint attributes and already-clean span name endpoint_hook = _observability._make_grpc_client_request_hook( {"server.address": "custom.api.com", "server.port": 443} ) mock_span_custom = mock.Mock() mock_span_custom.is_recording.return_value = True + mock_span_custom.name = "already_clean_name" endpoint_hook(mock_span_custom, req) mock_span_custom.set_attribute.assert_any_call("server.address", "custom.api.com") mock_span_custom.set_attribute.assert_any_call("server.port", 443) + mock_span_custom.update_name.assert_not_called() def test_extract_error_attributes_none(): From da83c5447dacdd4d2be1a732bdffd7e497dd6f01 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Thu, 10 Sep 2026 05:49:18 -0400 Subject: [PATCH 08/12] refactor(core): remove deferred gcp.resource.destination.id attribute - Remove gcp.resource.destination.id extraction from _extract_grpc_request_attributes - Update unit tests to reflect attribute removal per July Strategy Update --- .../google/api_core/_observability.py | 4 ---- .../tests/unit/test_observability.py | 23 ++----------------- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/packages/google-api-core/google/api_core/_observability.py b/packages/google-api-core/google/api_core/_observability.py index 5e8f574ba06f..b8daf298111e 100644 --- a/packages/google-api-core/google/api_core/_observability.py +++ b/packages/google-api-core/google/api_core/_observability.py @@ -124,10 +124,6 @@ def _extract_grpc_request_attributes(request: Any) -> dict[str, Any]: if isinstance(resend_count, int) and resend_count > 0: attrs["gcp.grpc.resend_count"] = resend_count - resource_id = getattr(request, "name", None) or getattr(request, "parent", None) - if isinstance(resource_id, str) and resource_id: - attrs["gcp.resource.destination.id"] = resource_id - return attrs diff --git a/packages/google-api-core/tests/unit/test_observability.py b/packages/google-api-core/tests/unit/test_observability.py index 982797aed4ef..7adc333fa8e5 100644 --- a/packages/google-api-core/tests/unit/test_observability.py +++ b/packages/google-api-core/tests/unit/test_observability.py @@ -338,32 +338,16 @@ def test_extract_endpoint_attributes(client_options, expected_attrs): (types.SimpleNamespace(), {"rpc.system.name": "grpc"}), ( types.SimpleNamespace(name="projects/p1/secrets/s1"), - { - "rpc.system.name": "grpc", - "gcp.resource.destination.id": "projects/p1/secrets/s1", - }, + {"rpc.system.name": "grpc"}, ), ( types.SimpleNamespace(parent="projects/parent-p1"), - { - "rpc.system.name": "grpc", - "gcp.resource.destination.id": "projects/parent-p1", - }, - ), - ( - types.SimpleNamespace( - name="projects/p1/secrets/s1", parent="projects/parent-p1" - ), - { - "rpc.system.name": "grpc", - "gcp.resource.destination.id": "projects/p1/secrets/s1", - }, + {"rpc.system.name": "grpc"}, ), ( types.SimpleNamespace(name="projects/p1/secrets/s1", resend_count=2), { "rpc.system.name": "grpc", - "gcp.resource.destination.id": "projects/p1/secrets/s1", "gcp.grpc.resend_count": 2, }, ), @@ -414,9 +398,6 @@ def test_grpc_client_request_hook(): mock_span_rec.set_attribute.assert_any_call("rpc.system.name", "grpc") assert "rpc.system" not in mock_span_rec._attributes - mock_span_rec.set_attribute.assert_any_call( - "gcp.resource.destination.id", "projects/my-proj/secrets/s1" - ) mock_span_rec.set_attribute.assert_any_call("gcp.grpc.resend_count", 1) # Custom hook with endpoint attributes and already-clean span name From 656e875f1efab1528aa070cd93344c4fb1c69798 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Thu, 10 Sep 2026 06:33:05 -0400 Subject: [PATCH 09/12] feat(core): record rpc.response.status_code on wire attempt spans --- .../google/api_core/_observability.py | 39 +++++++++++++++++++ .../tests/unit/test_observability.py | 32 +++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/packages/google-api-core/google/api_core/_observability.py b/packages/google-api-core/google/api_core/_observability.py index b8daf298111e..98ca898c4e7b 100644 --- a/packages/google-api-core/google/api_core/_observability.py +++ b/packages/google-api-core/google/api_core/_observability.py @@ -211,6 +211,43 @@ def client_request_hook(span: Any, request: Any) -> None: _grpc_client_request_hook = _make_grpc_client_request_hook() +def _grpc_client_response_hook(span: Any, response: Any) -> None: + """OpenTelemetry gRPC client response hook to record response status code. + + Args: + span: The OpenTelemetry span. + response: The gRPC response object or details. + """ + if span is None or not hasattr(span, "set_attribute"): + return + + status = getattr(span, "status", None) + status_code = getattr(status, "status_code", None) + try: + from opentelemetry.trace.status import StatusCode + + if status_code == StatusCode.ERROR: + span_attrs = ( + getattr(span, "attributes", None) + or getattr(span, "_attributes", None) + or {} + ) + grpc_code = span_attrs.get("rpc.grpc.status_code") + if grpc_code is not None: + from google.api_core import exceptions + + if grpc_code in exceptions._INT_TO_GRPC_CODE: + span.set_attribute( + "rpc.response.status_code", + exceptions._INT_TO_GRPC_CODE[grpc_code].name, + ) + return + except Exception: + pass + + span.set_attribute("rpc.response.status_code", "OK") + + def _get_tracer_provider( client_options: ClientOptions | dict[str, Any] | None = None, ) -> opentelemetry.trace.TracerProvider | None: @@ -254,6 +291,7 @@ def get_otel_interceptor( interceptor: ClientInterceptor = otel_grpc.client_interceptor( tracer_provider=_get_tracer_provider(client_options), request_hook=request_hook, + response_hook=_grpc_client_response_hook, ) def otel_interceptor(channel: grpc.Channel) -> grpc.Channel: @@ -287,4 +325,5 @@ def get_otel_async_interceptor( return otel_grpc.aio_client_interceptors( tracer_provider=_get_tracer_provider(client_options), request_hook=request_hook, + response_hook=_grpc_client_response_hook, ) diff --git a/packages/google-api-core/tests/unit/test_observability.py b/packages/google-api-core/tests/unit/test_observability.py index 7adc333fa8e5..e25139ce0a39 100644 --- a/packages/google-api-core/tests/unit/test_observability.py +++ b/packages/google-api-core/tests/unit/test_observability.py @@ -165,6 +165,7 @@ def test_get_otel_interceptor_enabled(monkeypatch): mock_otel_grpc.client_interceptor.assert_called_once_with( tracer_provider=mock_tracer_provider, request_hook=mock.ANY, + response_hook=_observability._grpc_client_response_hook, ) req_hook = mock_otel_grpc.client_interceptor.call_args[1]["request_hook"] mock_span = mock.Mock() @@ -260,7 +261,9 @@ def test_get_otel_async_interceptor_enabled(monkeypatch): mock_otel_grpc.aio_client_interceptors.assert_called_once_with( tracer_provider=mock_tracer_provider, request_hook=mock.ANY, + response_hook=_observability._grpc_client_response_hook, ) + req_hook = mock_otel_grpc.aio_client_interceptors.call_args[1]["request_hook"] mock_span = mock.Mock() mock_span.is_recording.return_value = True @@ -515,6 +518,7 @@ def test_get_otel_interceptor_with_api_endpoint(monkeypatch): args, kwargs = mock_otel_grpc.client_interceptor.call_args req_hook = kwargs["request_hook"] assert req_hook is not _observability._grpc_client_request_hook + assert kwargs["response_hook"] is _observability._grpc_client_response_hook # Test invoking the custom hook mock_span = mock.Mock() @@ -551,6 +555,7 @@ def test_get_otel_async_interceptor_with_api_endpoint(monkeypatch): args, kwargs = mock_otel_grpc.aio_client_interceptors.call_args req_hook = kwargs["request_hook"] assert req_hook is not _observability._grpc_client_request_hook + assert kwargs["response_hook"] is _observability._grpc_client_response_hook mock_span = mock.Mock() mock_span.is_recording.return_value = True @@ -560,3 +565,30 @@ def test_get_otel_async_interceptor_with_api_endpoint(monkeypatch): ) mock_span.set_attribute.assert_any_call("server.port", 8443) mock_span.set_attribute.assert_any_call("url.domain", "custom-domain.com") + + +def test_grpc_client_response_hook_success(): + """Proves that _grpc_client_response_hook sets rpc.response.status_code to 'OK' on success.""" + mock_span = mock.Mock() + _observability._grpc_client_response_hook(mock_span, mock.Mock()) + mock_span.set_attribute.assert_called_once_with("rpc.response.status_code", "OK") + + +def test_grpc_client_response_hook_error_mapped(): + """Proves that _grpc_client_response_hook maps status code when span has error status.""" + from opentelemetry.trace.status import StatusCode + + mock_span = mock.Mock() + mock_span.status.status_code = StatusCode.ERROR + mock_span.attributes = {"rpc.grpc.status_code": 5} + + _observability._grpc_client_response_hook(mock_span, None) + mock_span.set_attribute.assert_called_once_with( + "rpc.response.status_code", "NOT_FOUND" + ) + + +def test_grpc_client_response_hook_none_or_missing_set_attribute(): + """Proves that _grpc_client_response_hook handles None or invalid span gracefully.""" + _observability._grpc_client_response_hook(None, mock.Mock()) + _observability._grpc_client_response_hook(object(), mock.Mock()) From e906881625e8e0ee1e581a93206e09ada0898c03 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Thu, 10 Sep 2026 07:48:11 -0400 Subject: [PATCH 10/12] refactor(core): remove duplicate error attribute extraction in favor of method spans --- .../google/api_core/_observability.py | 37 --------- .../tests/unit/test_observability.py | 77 ------------------- 2 files changed, 114 deletions(-) diff --git a/packages/google-api-core/google/api_core/_observability.py b/packages/google-api-core/google/api_core/_observability.py index 98ca898c4e7b..4ad2b60b6e1e 100644 --- a/packages/google-api-core/google/api_core/_observability.py +++ b/packages/google-api-core/google/api_core/_observability.py @@ -127,43 +127,6 @@ def _extract_grpc_request_attributes(request: Any) -> dict[str, Any]: return attrs -def _extract_error_attributes(exc: Any) -> dict[str, Any]: - """Extracts gcp.errors.domain, gcp.errors.metadata.*, and error.type from an exception or ErrorInfo. - - Args: - exc: An exception (such as GoogleAPICallError or grpc.RpcError) or ErrorInfo object. - - Returns: - dict[str, Any]: Extracted error attributes. - """ - attrs: dict[str, Any] = {} - if exc is None: - return attrs - - error_info = getattr(exc, "error_info", None) - if error_info is None and hasattr(exc, "trailing_metadata"): - try: - from google.api_core import exceptions - - _, error_info = exceptions._parse_grpc_error_details(exc) - except Exception: - pass - - if error_info is not None: - domain = getattr(error_info, "domain", None) - if domain and isinstance(domain, str): - attrs["gcp.errors.domain"] = domain - reason = getattr(error_info, "reason", None) - if reason and isinstance(reason, str): - attrs["error.type"] = reason - metadata = getattr(error_info, "metadata", None) - if metadata and hasattr(metadata, "items"): - for k, v in metadata.items(): - attrs[f"gcp.errors.metadata.{k}"] = str(v) - - return attrs - - def _make_grpc_client_request_hook( endpoint_attrs: dict[str, Any] | None = None, ) -> Callable[[Any, Any], None]: diff --git a/packages/google-api-core/tests/unit/test_observability.py b/packages/google-api-core/tests/unit/test_observability.py index e25139ce0a39..3e720535f4bd 100644 --- a/packages/google-api-core/tests/unit/test_observability.py +++ b/packages/google-api-core/tests/unit/test_observability.py @@ -416,83 +416,6 @@ def test_grpc_client_request_hook(): mock_span_custom.update_name.assert_not_called() -def test_extract_error_attributes_none(): - """Proves that _extract_error_attributes returns an empty dict when exception is None.""" - assert _observability._extract_error_attributes(None) == {} - - -def test_extract_error_attributes_standard_exception(): - """Proves that _extract_error_attributes returns an empty dict for standard exceptions without ErrorInfo.""" - assert ( - _observability._extract_error_attributes(ValueError("unexpected error")) == {} - ) - - -def test_extract_error_attributes_with_error_info(): - """Proves that _extract_error_attributes extracts domain, error.type, and metadata from ErrorInfo.""" - error_info = types.SimpleNamespace( - domain="googleapis.com", - reason="SERVICE_DISABLED", - metadata={ - "service": "secretmanager.googleapis.com", - "consumer": "projects/123", - }, - ) - exc = types.SimpleNamespace(error_info=error_info) - attrs = _observability._extract_error_attributes(exc) - assert attrs == { - "gcp.errors.domain": "googleapis.com", - "error.type": "SERVICE_DISABLED", - "gcp.errors.metadata.service": "secretmanager.googleapis.com", - "gcp.errors.metadata.consumer": "projects/123", - } - - -def test_extract_error_attributes_from_grpc_trailing_metadata(monkeypatch): - """Proves that _extract_error_attributes parses error_info from gRPC trailing metadata.""" - from google.api_core import exceptions - - mock_exc = mock.Mock() - mock_exc.error_info = None - mock_exc.trailing_metadata = mock.Mock() - - parsed_error_info = types.SimpleNamespace( - domain="googleapis.com", - reason="RESOURCE_EXHAUSTED", - metadata={"quota_limit": "100"}, - ) - - monkeypatch.setattr( - exceptions, - "_parse_grpc_error_details", - mock.Mock(return_value=(None, parsed_error_info)), - ) - - attrs = _observability._extract_error_attributes(mock_exc) - assert attrs == { - "gcp.errors.domain": "googleapis.com", - "error.type": "RESOURCE_EXHAUSTED", - "gcp.errors.metadata.quota_limit": "100", - } - - -def test_extract_error_attributes_trailing_metadata_failure(monkeypatch): - """Proves that _extract_error_attributes safely handles exceptions during trailing metadata parsing.""" - from google.api_core import exceptions - - mock_exc = mock.Mock() - mock_exc.error_info = None - mock_exc.trailing_metadata = mock.Mock() - - monkeypatch.setattr( - exceptions, - "_parse_grpc_error_details", - mock.Mock(side_effect=RuntimeError("Parse failed")), - ) - - assert _observability._extract_error_attributes(mock_exc) == {} - - def test_get_otel_interceptor_with_api_endpoint(monkeypatch): """Proves that get_otel_interceptor injects server.address, server.port, and url.domain when api_endpoint is set.""" monkeypatch.setenv("GOOGLE_SDK_EXPERIMENTAL_PYTHON_TRACING_ENABLED", "true") From 1cc2c2584a0f4e4eda7a96b0ca34fa0cdea781fe Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Thu, 10 Sep 2026 10:29:31 -0400 Subject: [PATCH 11/12] fix(observability): resolve mypy union-attr error and support environments without grpc --- .../google/api_core/_observability.py | 41 ++++++++++++++++--- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/packages/google-api-core/google/api_core/_observability.py b/packages/google-api-core/google/api_core/_observability.py index 4ad2b60b6e1e..a14e2f43ddc7 100644 --- a/packages/google-api-core/google/api_core/_observability.py +++ b/packages/google-api-core/google/api_core/_observability.py @@ -157,8 +157,10 @@ def client_request_hook(span: Any, request: Any) -> None: # Remove duplicate legacy rpc.system attribute set by stock instrumentation # in favor of modern rpc.system.name ("grpc") per PRD changelog. span_attributes = getattr(span, "_attributes", None) - if hasattr(span_attributes, "pop"): - span_attributes.pop("rpc.system", None) + if span_attributes is not None: + pop_fn = getattr(span_attributes, "pop", None) + if callable(pop_fn): + pop_fn("rpc.system", None) attrs = _extract_grpc_request_attributes(request) if clean_method_name: @@ -173,6 +175,29 @@ def client_request_hook(span: Any, request: Any) -> None: _grpc_client_request_hook = _make_grpc_client_request_hook() +# Mapping of standard gRPC integer status codes to their canonical status name strings. +# Used when stock gRPC wire spans encounter errors, guaranteeing mapping even in environments +# where the optional `grpc` package is not installed (e.g. REST-only environments). +_GRPC_INT_STATUS_CODE_TO_NAME = { + 0: "OK", + 1: "CANCELLED", + 2: "UNKNOWN", + 3: "INVALID_ARGUMENT", + 4: "DEADLINE_EXCEEDED", + 5: "NOT_FOUND", + 6: "ALREADY_EXISTS", + 7: "PERMISSION_DENIED", + 8: "RESOURCE_EXHAUSTED", + 9: "FAILED_PRECONDITION", + 10: "ABORTED", + 11: "OUT_OF_RANGE", + 12: "UNIMPLEMENTED", + 13: "INTERNAL", + 14: "UNAVAILABLE", + 15: "DATA_LOSS", + 16: "UNAUTHENTICATED", +} + def _grpc_client_response_hook(span: Any, response: Any) -> None: """OpenTelemetry gRPC client response hook to record response status code. @@ -199,12 +224,16 @@ def _grpc_client_response_hook(span: Any, response: Any) -> None: if grpc_code is not None: from google.api_core import exceptions + name = None if grpc_code in exceptions._INT_TO_GRPC_CODE: - span.set_attribute( - "rpc.response.status_code", - exceptions._INT_TO_GRPC_CODE[grpc_code].name, - ) + name = exceptions._INT_TO_GRPC_CODE[grpc_code].name + elif grpc_code in _GRPC_INT_STATUS_CODE_TO_NAME: + name = _GRPC_INT_STATUS_CODE_TO_NAME[grpc_code] + if name: + span.set_attribute("rpc.response.status_code", name) return + span.set_attribute("rpc.response.status_code", "ERROR") + return except Exception: pass From 414c166317824543d6fd088d8e066c39ea61ef12 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Thu, 10 Sep 2026 10:50:35 -0400 Subject: [PATCH 12/12] refactor(observability): simplify response hook to record OK on successful RPCs --- .../google/api_core/_observability.py | 60 ++----------------- .../tests/unit/test_observability.py | 14 ----- 2 files changed, 5 insertions(+), 69 deletions(-) diff --git a/packages/google-api-core/google/api_core/_observability.py b/packages/google-api-core/google/api_core/_observability.py index a14e2f43ddc7..07cc352cd31c 100644 --- a/packages/google-api-core/google/api_core/_observability.py +++ b/packages/google-api-core/google/api_core/_observability.py @@ -175,69 +175,19 @@ def client_request_hook(span: Any, request: Any) -> None: _grpc_client_request_hook = _make_grpc_client_request_hook() -# Mapping of standard gRPC integer status codes to their canonical status name strings. -# Used when stock gRPC wire spans encounter errors, guaranteeing mapping even in environments -# where the optional `grpc` package is not installed (e.g. REST-only environments). -_GRPC_INT_STATUS_CODE_TO_NAME = { - 0: "OK", - 1: "CANCELLED", - 2: "UNKNOWN", - 3: "INVALID_ARGUMENT", - 4: "DEADLINE_EXCEEDED", - 5: "NOT_FOUND", - 6: "ALREADY_EXISTS", - 7: "PERMISSION_DENIED", - 8: "RESOURCE_EXHAUSTED", - 9: "FAILED_PRECONDITION", - 10: "ABORTED", - 11: "OUT_OF_RANGE", - 12: "UNIMPLEMENTED", - 13: "INTERNAL", - 14: "UNAVAILABLE", - 15: "DATA_LOSS", - 16: "UNAUTHENTICATED", -} - def _grpc_client_response_hook(span: Any, response: Any) -> None: """OpenTelemetry gRPC client response hook to record response status code. + Note: Upstream OpenTelemetry gRPC instrumentation only invokes this response_hook + on successful RPC invocations. Failed RPCs raise an exception before this hook is reached. + Args: span: The OpenTelemetry span. response: The gRPC response object or details. """ - if span is None or not hasattr(span, "set_attribute"): - return - - status = getattr(span, "status", None) - status_code = getattr(status, "status_code", None) - try: - from opentelemetry.trace.status import StatusCode - - if status_code == StatusCode.ERROR: - span_attrs = ( - getattr(span, "attributes", None) - or getattr(span, "_attributes", None) - or {} - ) - grpc_code = span_attrs.get("rpc.grpc.status_code") - if grpc_code is not None: - from google.api_core import exceptions - - name = None - if grpc_code in exceptions._INT_TO_GRPC_CODE: - name = exceptions._INT_TO_GRPC_CODE[grpc_code].name - elif grpc_code in _GRPC_INT_STATUS_CODE_TO_NAME: - name = _GRPC_INT_STATUS_CODE_TO_NAME[grpc_code] - if name: - span.set_attribute("rpc.response.status_code", name) - return - span.set_attribute("rpc.response.status_code", "ERROR") - return - except Exception: - pass - - span.set_attribute("rpc.response.status_code", "OK") + if span is not None and hasattr(span, "set_attribute"): + span.set_attribute("rpc.response.status_code", "OK") def _get_tracer_provider( diff --git a/packages/google-api-core/tests/unit/test_observability.py b/packages/google-api-core/tests/unit/test_observability.py index 3e720535f4bd..635505dea8c3 100644 --- a/packages/google-api-core/tests/unit/test_observability.py +++ b/packages/google-api-core/tests/unit/test_observability.py @@ -497,20 +497,6 @@ def test_grpc_client_response_hook_success(): mock_span.set_attribute.assert_called_once_with("rpc.response.status_code", "OK") -def test_grpc_client_response_hook_error_mapped(): - """Proves that _grpc_client_response_hook maps status code when span has error status.""" - from opentelemetry.trace.status import StatusCode - - mock_span = mock.Mock() - mock_span.status.status_code = StatusCode.ERROR - mock_span.attributes = {"rpc.grpc.status_code": 5} - - _observability._grpc_client_response_hook(mock_span, None) - mock_span.set_attribute.assert_called_once_with( - "rpc.response.status_code", "NOT_FOUND" - ) - - def test_grpc_client_response_hook_none_or_missing_set_attribute(): """Proves that _grpc_client_response_hook handles None or invalid span gracefully.""" _observability._grpc_client_response_hook(None, mock.Mock())