feat(core): add request hook to enrich T4 network spans with GCP resource attributes (E) by chalmerlowe · Pull Request #18272 · googleapis/google-cloud-python

chalmerlowe · 2026-09-03T10:30:24Z

Problem

Low-level gRPC transport spans generated by opentelemetry-instrumentation-grpc capture standard RPC metadata but lack Google Cloud semantic context. Without request- and response-level enrichment, spans cannot identify configured endpoint attributes (server.address, server.port, url.domain), resend count, or the Cloud Observability success status (rpc.response.status_code = "OK").

Solution

This PR enriches wire-level gRPC client spans in google-api-core by attaching lightweight request and response hooks to both synchronous and asynchronous OpenTelemetry gRPC interceptors:

Request Hook (_make_grpc_client_request_hook):
- Captures gcp.grpc.resend_count when present on retryable requests.
- Sets rpc.system.name: "grpc" per OpenTelemetry semantic conventions.
- Extracts server.address, server.port (omitted for standard 443/80 ports), and url.domain from client_options.
Response Hook (_grpc_client_response_hook):
- Records rpc.response.status_code = "OK" strictly on successful RPC completions.
- Does not perform error translation (error names and gcp.errors.* attributes are handled at the client method span level).
Transport Integration:
- Registers both hooks in get_otel_interceptor (sync) and get_otel_async_interceptor (async) within google.api_core._observability.

Notes for Reviewers

Hooks safely short-circuit when the active span is not recording, avoiding overhead when tracing is disabled or unsampled.

gemini-code-assist

Code Review

This pull request introduces OpenTelemetry request hooks to extract and inject Google Cloud semantic and resource attributes (such as resource name, parent, and project ID) from gRPC request objects into OpenTelemetry spans. Specifically, it adds _extract_t4_attributes and _client_request_hook helper functions in _observability.py, registers the hook in the gRPC client interceptors, and includes comprehensive unit tests to verify this behavior. There are no review comments, so I have no feedback to provide.

daniel-sanche · 2026-09-09T16:38:44Z

+        endpoint = getattr(client_options, "api_endpoint", None)
+
+    if endpoint and isinstance(endpoint, str):
+        clean = endpoint.replace("http://", "").replace("https://", "").strip("/")


Is there a reason you're avoiding something like urllib.parse to parse this for us?

Manual parsing can be brittle. E.g.m what if this is a ipv6 address? Or there's a path component after the port?

Replaced the manual approach with urllib.parse. Great catch. Thnaks!

daniel-sanche · 2026-09-09T16:41:47Z

+        attrs["gcp.grpc.resend_count"] = resend_count
+
+    name = getattr(request, "name", None)
+    if isinstance(name, str) and name:


When would you expect this to be non-string? Are you sure we should fall back to the parent in that case?

This is OBE.

daniel-sanche · 2026-09-09T16:43:43Z

+    else:
+        parent = getattr(request, "parent", None)
+        if isinstance(parent, str) and parent:
+            attrs["gcp.resource.destination.id"] = parent


nit: we could reduce some duplication here:

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

This is OBE. We no longer process "gcp.resource.destination.id"

daniel-sanche · 2026-09-09T17:07:01Z

+        _make_grpc_client_request_hook(endpoint_attrs)
+        if endpoint_attrs
+        else _grpc_client_request_hook
+    )


nit: can't this just be _make_grpc_client_request_hook(endpoint_attrs)? It seems like the implementation already handles empty endpoint_attrs, so I'm not sure we need to handle both cases here

Simplified. Good catch. Appreciate it.

daniel-sanche · 2026-09-09T17:52:13Z

    return False


+_STATUS_CODE_NAMES = {


In the PR description, you say " Without request- and response-level enrichment, spans cannot identify ... normalized string status codes".

Can you expand on that? Does the default instrumenter add code numbers, but not strings? It feels strange that we would have to add the cost of an extra callback layer to do that kind of transformation

Could we add this somewhere else in the stack? Or just stick with error numbers instead of names?

the PR description was outdated and described an earlier prototype where the response hook was trying to do too much.
To answer your questions directly:

Does the default instrumenter add code numbers, but not strings?
Yes. Upstream opentelemetry-instrumentation-grpc sets rpc.grpc.status_code as an integer (0, 5, etc.) per OTel gRPC conventions. It does not set rpc.response.status_code at all.

Can we do this elsewhere / avoid callback translation overhead?
Yes, we moved error resolution out of this hook. All error status mapping ("NOT_FOUND", "RESOURCE_EXHAUSTED", etc.) and gcp.errors.* attributes are handled directly on the client method span in google.api_core.gapic_v1.method (PR feat(gapic): add OpenTelemetry T3 client method span wrapping in gapic_v1.method (D) #18274) where exceptions are caught naturally.

What _grpc_client_response_hook actually does in this PR:
The response hook here does zero error parsing, zero translation, and zero dictionary lookups. It only stamps rpc.response.status_code = "OK" on successful wire spans (checking span.status first to bail out if an error occurred).
I've updated the PR description to reflect the current, simplified implementation.

daniel-sanche

My main comment is around the response callback. Are you sure it can do what we need? And do we really need a callback for status names?

daniel-sanche · 2026-09-11T18:17:30Z

+
+
+def _grpc_client_response_hook(span: Any, response: Any) -> None:
+    """OpenTelemetry gRPC client response hook to record response status code.


It seems like this is just being used to record sufccessful states now, right? Can we reflect that in the docstring?

As an aside, I'm a little surprised we'd need to add hooks to do this manually. I'd expect the interceptor to handle this. Gemini says it may be because the interceptor was written against an older version of the spec, and might be improved in the future. So we should make sure the doc strings are clear about what this hook is doing here, in case we can remove it in the future.

daniel-sanche · 2026-09-11T18:18:37Z

+
+    if endpoint and isinstance(endpoint, str):
+        target = endpoint if "//" in endpoint else f"//{endpoint}"
+        parsed = urllib.parse.urlsplit(target)


It looks like this could raise an exception. Can we either put this within the try block, or add a Raises section to the docstring, and make sure any exceptions are handled at a higher level?

Done: We moved the core steps that could cause an exception (the parsing steps) into the try/except.

daniel-sanche · 2026-09-11T18:21:35Z

+    if request is None:
+        return attrs
+
+    resend_count = getattr(request, "resend_count", None)


My understanding is that request will be a protobuf message, which wouldn't have this field. What do you expect it to be? Can we have any clearer typing here?

daniel-sanche · 2026-09-11T18:25:02Z

+
+        # 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)


Is there a way to do this without reaching into the private API?

It looks like this is done with extensive null checks, so it's probably ok to leave this as-is. But if Otel doesn't provide a way to overwrite this attribute, maybe we should just not touch it

daniel-sanche · 2026-09-11T18:35:40Z

+        if span is None or not getattr(span, "is_recording", lambda: True)():
+            return
+
+        # Upstream opentelemetry-instrumentation-grpc names spans with a leading slash


I looked into this, and it seems like this isn't part of the formal spec, even if it's how the upstream package is behaving.

It looks like this logic only sets clean_method_name if it has the slash. Can we re-work it so the method name is always captured?

…butes

… 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

- 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

…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

…ntion - 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

- 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.<key> - 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

…tem 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

- Remove gcp.resource.destination.id extraction from _extract_grpc_request_attributes - Update unit tests to reflect attribute removal per July Strategy Update

…of method spans

…ments without grpc

…ssful RPCs

…ch coverage

…l calls

…parsing, and attribute handling

gemini-code-assist Bot reviewed Sep 3, 2026

View reviewed changes

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from a7be0e4 to 9efe757 Compare September 3, 2026 14:49

chalmerlowe changed the base branch from feat/otel-tracing-transport-logic to feat/otel-tracing-t3-method-spans September 3, 2026 14:50

chalmerlowe added this to the [o11y] Tracing milestone Sep 3, 2026

chalmerlowe changed the title ~~feat(core): add request hook to inject GCP resource and project attributes into OpenTelemetry spans~~ feat(core): add OpenTelemetry T4 span wrapping (E) Sep 3, 2026

chalmerlowe self-assigned this Sep 3, 2026

chalmerlowe changed the title ~~feat(core): add OpenTelemetry T4 span wrapping (E)~~ feat(core): add request hook to enrich spans with GCP resource attributes (E) Sep 3, 2026

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from b5a883a to 83963a7 Compare September 3, 2026 15:25

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from 9efe757 to 40071ff Compare September 3, 2026 15:25

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from 83963a7 to bf82825 Compare September 4, 2026 09:04

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from 40071ff to 8c9d21a Compare September 4, 2026 09:04

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from bf82825 to bc94977 Compare September 4, 2026 09:13

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from 8c9d21a to 558f8fd Compare September 4, 2026 09:13

chalmerlowe changed the title ~~feat(core): add request hook to enrich spans with GCP resource attributes (E)~~ feat(core): add request hook to enrich T4 network spans with GCP resource attributes (E) Sep 4, 2026

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from bc94977 to 67e3879 Compare September 4, 2026 11:42

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from 558f8fd to a6f3b3a Compare September 4, 2026 11:49

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from 67e3879 to 6b9bfab Compare September 4, 2026 14:55

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from a6f3b3a to 7e1e498 Compare September 4, 2026 14:55

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from fb16695 to 9b46ef7 Compare September 4, 2026 17:41

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from 7e1e498 to 131a80a Compare September 4, 2026 17:41

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from 8c5d285 to 62a8261 Compare September 8, 2026 19:06

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from 131a80a to 0da0216 Compare September 8, 2026 19:06

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from 007f6b0 to 5b39cff Compare September 9, 2026 08:24

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from 0da0216 to 1c3834e Compare September 9, 2026 08:24

chalmerlowe commented Sep 9, 2026

View reviewed changes

Comment thread packages/google-api-core/google/api_core/_observability.py Outdated

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from 5b39cff to c15d70a Compare September 9, 2026 17:13

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from de053b3 to 051b3d7 Compare September 9, 2026 17:13

daniel-sanche requested changes Sep 9, 2026

View reviewed changes

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from 051b3d7 to 2460284 Compare September 10, 2026 11:52

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from 4739860 to ef5771d Compare September 11, 2026 11:20

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch 3 times, most recently from 8775743 to d6307d6 Compare September 11, 2026 14:18

chalmerlowe marked this pull request as ready for review September 11, 2026 16:00

chalmerlowe requested a review from a team as a code owner September 11, 2026 16:00

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from 3610162 to b1d7f5d Compare September 11, 2026 18:41

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from d6307d6 to b3931e9 Compare September 11, 2026 18:41

daniel-sanche reviewed Sep 11, 2026

View reviewed changes

chalmerlowe force-pushed the feat/otel-tracing-t3-method-spans branch from b1d7f5d to 67c8ae7 Compare September 11, 2026 22:09

chalmerlowe removed this pull request from stack #18275 September 11, 2026 22:17

chalmerlowe changed the base branch from feat/otel-tracing-t3-method-spans to feat/otel-tracing-transport-logic September 11, 2026 22:25

chalmerlowe changed the base branch from feat/otel-tracing-transport-logic to main September 11, 2026 22:53

chalmerlowe added 15 commits September 11, 2026 19:01

feat(core): add request hook to inject GCP resource and project attri…

bfb40b0

…butes

refactor(core): remove deferred gcp.resource.destination.id attribute

ef7d77d

- Remove gcp.resource.destination.id extraction from _extract_grpc_request_attributes - Update unit tests to reflect attribute removal per July Strategy Update

feat(core): record rpc.response.status_code on wire attempt spans

aa2bead

refactor(core): remove duplicate error attribute extraction in favor …

894b380

…of method spans

fix(observability): resolve mypy union-attr error and support environ…

1c1b9af

…ments without grpc

refactor(observability): simplify response hook to record OK on succe…

81b686c

…ssful RPCs

test(observability): cover request hook span edge cases for 100% bran…

324b866

…ch coverage

fix(observability): safely handle invalid port in endpoint attributes

abeaf04

fix(observability): ensure response hook only records OK on successfu…

99a4d3d

…l calls

chalmerlowe force-pushed the feat/otel-tracing-t4-resource-attributes branch from b3931e9 to 99a4d3d Compare September 11, 2026 23:01

refactor(observability): address review feedback on method name, url …

4b82c9c

…parsing, and attribute handling



		def _grpc_client_response_hook(span: Any, response: Any) -> None:
		"""OpenTelemetry gRPC client response hook to record response status code.

Uh oh!

Conversation

chalmerlowe commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Notes for Reviewers

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chalmerlowe commented Sep 3, 2026 •

edited

Loading