Experiment(secret-manager): add tracing transport logic to google-cloud-secret-manager - #18188
Experiment(secret-manager): add tracing transport logic to google-cloud-secret-manager#18188chalmerlowe wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces OpenTelemetry (OTel) tracing support by adding a helper module _otel_helpers.py to resolve and instantiate OTel gRPC interceptors, extending ClientOptions to accept a tracer_provider, and updating the Secret Manager gRPC transport to inject these interceptors. The review feedback highlights Python compatibility issues in _otel_helpers.py due to the use of the | union operator, which is unsupported in older Python versions, and suggests using typing.Union instead. Additionally, it recommends adding a defensive None check for client_options to avoid calling getattr on a None object.
| if isinstance(client_options, dict): | ||
| tracer_provider = client_options.get("tracer_provider") | ||
| else: | ||
| tracer_provider = getattr(client_options, "tracer_provider", None) |
There was a problem hiding this comment.
When processing optional parameters with a helper function, prefer placing the null or None check within the function body rather than at the call site to simplify the calling code and improve encapsulation. This also avoids confusing or non-idiomatic getattr(None, ...) calls.
| if isinstance(client_options, dict): | |
| tracer_provider = client_options.get("tracer_provider") | |
| else: | |
| tracer_provider = getattr(client_options, "tracer_provider", None) | |
| if isinstance(client_options, dict): | |
| tracer_provider = client_options.get("tracer_provider") | |
| elif client_options is not None: | |
| tracer_provider = getattr(client_options, "tracer_provider", None) | |
| else: | |
| tracer_provider = None |
References
- When processing optional parameters with a helper function, prefer placing the null or None check within the function body rather than at the call site to simplify the calling code and improve encapsulation.
916213b to
d1a7a0f
Compare
Problem
Client libraries currently lack built-in support for OpenTelemetry tracing interceptors. We need a flexible, explicit mechanism to inject these interceptors into the transport pipeline without tying the core low-level helpers too tightly to specific observability features.
Solution
This PR implements explicit interceptor injection in the
SecretManagerServiceClientand its gRPC transport.SecretManagerServiceClientto resolve OpenTelemetry interceptors usinggoogle-api-corehelpers and explicitly pass them to the transport.SecretManagerServiceGrpcTransportto accept aninterceptorslist and explicitly apply them to the gRPC channel usinggrpc.intercept_channel.test_secret_manager_service_client_otel_interceptor_injectionand*_disabledvariant) to verify that interceptors are correctly injected and applied.Notes to Reviewers
google-api-corePR (Phase 1).Fixes #18139 (partially, this is Phase 2 of the larger effort)