feat(gapic): add async gRPC OpenTelemetry channel tracing - #18367
chalmerlowe wants to merge 1 commit into
Conversation
Add complete end-to-end support for asynchronous gRPC OpenTelemetry tracing: - packages/google-api-core: - Add coroutine-aware Tier 3 client method tracing in method_async.py - Add async gRPC channel interceptor support in grpc_helpers_async.py - Add tracing_enabled option and async interceptor factory in _observability.py / client_options.py - Add comprehensive unit tests in test_method_async.py and test_grpc_helpers_async.py - packages/gapic-generator: - Wire async OpenTelemetry interceptor into generated GrpcAsyncIOTransport and client - Update unit test template to mock async interceptors - Regenerate and verify all 8 Bazel integration test goldens - Add async gRPC tracing test suite in tests/system/test_tracing.py
There was a problem hiding this comment.
Code Review
This pull request adds OpenTelemetry (OTel) tracing support for asynchronous gRPC clients (grpc_asyncio) across the GAPIC generator templates and google-api-core. Key changes include updating client and transport templates to inject async interceptors, introducing the _AsyncGapicCallable class to wrap async RPCs with tracing capabilities, and adding comprehensive unit and system tests. The review feedback suggests simplifying the logic in is_otel_capabilities_enabled to eliminate duplicate feature flag resolution calls, and using a cleaner conditional expression to retrieve the tracer_provider from client_options in method_async.py.
| if client_options is not None: | ||
| tracing_opt = ( | ||
| client_options.get("tracing_enabled") | ||
| if isinstance(client_options, dict) | ||
| else getattr(client_options, "tracing_enabled", None) | ||
| ) | ||
| if tracing_opt is False: | ||
| return False | ||
| if tracing_opt is True: | ||
| is_tracing_enabled = True | ||
| else: | ||
| is_tracing_enabled = _feature_gating_helpers.resolve_feature_flags( | ||
| env_var=env_var, | ||
| feature_key=_TRACER_PROVIDER, | ||
| configuration=client_options, | ||
| ) | ||
| else: | ||
| is_tracing_enabled = _feature_gating_helpers.resolve_feature_flags( | ||
| env_var=env_var, | ||
| feature_key=_TRACER_PROVIDER, | ||
| configuration=client_options, | ||
| ) |
There was a problem hiding this comment.
We can simplify the logic in is_otel_capabilities_enabled to eliminate the duplicated call to _feature_gating_helpers.resolve_feature_flags. By extracting tracing_opt first and then evaluating it, we make the code more concise and maintainable.
tracing_opt = None
if client_options is not None:
tracing_opt = (
client_options.get("tracing_enabled")
if isinstance(client_options, dict)
else getattr(client_options, "tracing_enabled", None)
)
if tracing_opt is False:
return False
elif tracing_opt is True:
is_tracing_enabled = True
else:
is_tracing_enabled = _feature_gating_helpers.resolve_feature_flags(
env_var=env_var,
feature_key=_TRACER_PROVIDER,
configuration=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) |
There was a problem hiding this comment.
We can simplify the retrieval of tracer_provider from client_options using a single conditional expression. This removes the nested if/elif blocks and makes the initialization cleaner. However, since this is a performance-critical code path executed on every request, please validate and benchmark this proposed readability simplification to ensure it does not degrade performance or eliminate fast-path optimizations.
| 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) | |
| tracer_provider = ( | |
| client_options.get("tracer_provider") | |
| if isinstance(client_options, dict) | |
| else getattr(client_options, "tracer_provider", None) | |
| ) |
References
- For performance-critical code paths executed on every request, validate and benchmark any proposed readability simplifications to ensure they do not degrade performance or eliminate fast-path optimizations.
Summary
Extends OpenTelemetry client-side tracing support to asynchronous gRPC (
asyncio) acrossgoogle-api-coreandgapic-generator, achieving full parity with synchronous gRPC tracing.Motivation & Design
Asynchronous workloads using
asyncioclients require trace propagation and lifecycle tracking matching synchronous calls:_AsyncGapicCallable) in coroutine-aware spans that remain open across async retries and backoffs, accurately recording status codes, canonical descriptions, and GCP error attributes.aio.ClientInterceptorintoGrpcAsyncIOTransportchannels viagoogle.api_core.grpc_helpers_async.apply_channel_interceptorswhen OpenTelemetry tracing is enabled.ClientOptions.tracing_enabledand the existingGOOGLE_API_CORE_EXPERIMENTAL_OPEN_TELEMETRY_ENABLEDenvironment flag. Defaults to disabled with zero performance overhead when OpenTelemetry packages are absent.Testing & Verification
packages/google-api-core/tests/asyncio/for method tracing and channel creation/interceptor attachment (1,283 passed).bazel test //tests/integration:all- 15/15 passed).packages/gapic-generator/tests/system/test_tracing.pycovering unary calls, retries, custom tracer providers, opt-in/opt-out, and direct transport initialization (12 passed).