feat(api-core): add channel orchestration for OpenTelemetry - #18237
feat(api-core): add channel orchestration for OpenTelemetry#18237chalmerlowe wants to merge 4 commits into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Code Review
This pull request refactors OpenTelemetry channel instrumentation in google/api_core/_observability.py by replacing apply_otel_capabilities_to_channel with dedicated helpers for creating synchronous and asynchronous channels with OTel capabilities (create_channel_with_otel and create_async_channel_with_otel). Unit tests are updated accordingly. The review feedback highlights an inconsistency in interceptor execution order between the sync and async implementations, suggesting that the async OTel interceptor should be prepended rather than appended to the interceptors list to maintain consistent tracing semantics across both environments.
Sorry, something went wrong.
| mock_channel = mock.Mock() | ||
| mock_intercepted_channel = mock.Mock() | ||
| def test_get_otel_interceptor_sync_default(monkeypatch): | ||
| mock_otel = mock.Mock() |
There was a problem hiding this comment.
There is plenty of room for deduplicating some of the inner workings of these tests (using fixtures, reusable functions, etc). Happy to revise these but would prefer to get some initial buy-in on the overall approach in the body of the code before investing in what might end up being premature optimization.
Sorry, something went wrong.
| def create_channel_with_otel( | ||
| channel_factory: Callable[..., Any], | ||
| client_options: Optional[Union[ClientOptions, dict[str, Any]]] = None, | ||
| **channel_kwargs: Any, |
There was a problem hiding this comment.
Can we also accept *channel_args? That would make this easier to pass into the transport init: #18188 (comment)
Sorry, something went wrong.
There was a problem hiding this comment.
Adding *channel_args to create_channel_with_otel (and create_async_channel_with_otel) is a good improvement.
Transports pass self._host positionally to channel_init(self._host, ...). Supporting *channel_args allows us to pass functools.partial(_observability.create_channel_with_otel, Transport.create_channel, client_options=self._client_options) as the channel argument in the client.
This preserves true lazy channel initialization in the transport and eliminates the need for the client to duplicate extracting and passing credentials, scopes, quota_project_id, etc.
Sorry, something went wrong.
| return otel_grpc.client_interceptor(tracer_provider=tracer_provider) | ||
|
|
||
|
|
||
| def create_channel_with_otel( |
There was a problem hiding this comment.
I left some comments in your other PR, but if it's possible to decouple the interceptor more from the channel, that could make thinks a lot easier for composition in the future.
I think the previous apply_otel_capabilities_to_channel would be better suited for this. If we go with option A, the client could do something like
grpc_interceptor = functools.partial(apply_otel_capabilities_to_channel, client_options=options) interceptor_list = [grpc_interceptor, logging_interceptor] Transport(interceptors=interceptor_list, ...)
Sorry, something went wrong.
- Implement create_channel_with_otel and create_async_channel_with_otel helpers - Deduplicate interceptor instantiation via internal _get_otel_interceptor - Add unit tests in test_observability.py
… tests
- Use list(channel_kwargs.pop('interceptors', None) or []) in create_async_channel_with_otel
- Add unit tests for None and omitted interceptors arguments
…ion in channel factories - Add *channel_args to create_channel_with_otel and create_async_channel_with_otel - Make client_options keyword-only to prevent argument collision with functools.partial - Add TDD unit tests with detailed docstrings for positional forwarding and partial binding
This pull request introduces channel orchestration helper functions in google.api_core._observability to support OpenTelemetry (OTel) client interceptors for synchronous gRPC channels (asynchronous gRPC channels are included for comparison).
Problem
Generated client libraries need a centralized, maintainable way to create and instrument gRPC channels with OpenTelemetry tracing when enabled via environment variables or client options.
Because synchronous gRPC (grpc) and asynchronous gRPC (grpc.aio) have fundamentally different channel creation lifecycles (sync channels can be intercepted post-creation via OpenTelemetry's custom applier, whereas async grpc.aio channels are immutable and require interceptors at construction time), handling these differences directly in every generated client or transport creates boilerplate and risk of drift.
Solution
This PR introduces channel creation orchestration helpers in google.api_core._observability:
Testing
Notes for Reviewers