feat(api-core): add channel orchestration for OpenTelemetry (B) - #18237
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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.
| 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.
| 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)
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.
| 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, ...)
chalmerlowe
changed the title
1a04d27 to
cf95523
Compare
394451b to
8b2dc1f
Compare
cf95523 to
9d8d542
Compare
chalmerlowe
changed the title
- 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
d751865 to
92c18b6
Compare
3687533
into
feat/otel-tracing-centralized-interceptor
2 participants
This pull request introduces OpenTelemetry helper functions in
google.api_core._observabilityto produce channel wrappers for synchronous gRPC channels and interceptors for asynchronous gRPC channels.Problem
Generated client libraries need a consistent and maintainable way to instrument gRPC channels with OpenTelemetry tracing when enabled via environment variables or client options.
Because synchronous gRPC channels can be wrapped post-creation while asynchronous gRPC channels require interceptors at channel creation time, client transports need helpers that return the appropriate channel wrapper or async interceptors without duplicating OpenTelemetry resolution logic across client libraries.
Solution
This pull request introduces the following helper functions in
google.api_core._observability:get_otel_channel_wrapper(client_options):Callable[[Channel], Channel]) for synchronous gRPC channels when OpenTelemetry tracing is enabled and installed.grpc_helpers.apply_channel_wrappersto wrap raw channels using OpenTelemetry'sintercept_channel.get_otel_async_interceptor(client_options):aio_client_interceptors) for use when constructinggrpc.aiochannels._get_otel_interceptor(client_options, is_async):tracer_providerfromClientOptionsand creates the appropriate OpenTelemetry sync or async interceptors.Testing
tests/unit/test_observability.pycovering:tracer_providerconfiguration.get_otel_channel_wrapperbehavior when tracing is disabled, when OpenTelemetry is not installed, and when tracing is enabled.get_otel_channel_wrapperandgrpc_helpers.apply_channel_wrappers.get_otel_async_interceptorbehavior across disabled, missing, and enabled states.Notes for Reviewers
ChannelWrapperandapply_channel_wrappers).get_otel_channel_wrapperreturns a callable rather than modifying the channel immediately, allowing transport layers to combine OpenTelemetry wrapping with user-supplied custom channel wrappers.