feat(api-core): lay groundwork for stock OpenTelemetry gRPC tracing - #18026
feat(api-core): lay groundwork for stock OpenTelemetry gRPC tracing#18026chalmerlowe wants to merge 16 commits into
Conversation
Refactored OtelSpanEnricher to OtelUnaryClientInterceptor to act as a span creator (Pure API approach). Integrated feature gating helpers to control tracing. Added dynamic attribute extraction from gRPC metadata. Cleaned up dead code and added samples.
Removed Translate samples as we are shifting focus to Secret Manager and avoiding auto-instrumentation. Fixed a TypeError in pytest.skip usage in test_tracing.py.
Added tests for __init__.py import failures and expanded metadata parsing tests in test_tracing.py to cover all branches and error handling.
![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 introduces OpenTelemetry integration for gRPC channels in google-api-core, adding opentelemetry-api as a dependency and implementing unit tests to verify the behavior when tracing is enabled, disabled, or when OpenTelemetry is not installed. The review feedback focuses on improving the robustness of the unit tests by replacing the custom clean_sys_modules fixture with pytest's built-in monkeypatch to safely manage sys.modules modifications, and properly simulating a missing package by mapping its module to None instead of deleting it.
| @pytest.fixture | ||
| def clean_sys_modules(): | ||
| """Fixture to ensure opentelemetry modules are unloaded before and after tests.""" | ||
| modules_to_remove = [ | ||
| "opentelemetry.instrumentation.grpc", | ||
| "opentelemetry.instrumentation", | ||
| "opentelemetry", | ||
| ] | ||
| for mod in modules_to_remove: | ||
| if mod in sys.modules: | ||
| del sys.modules[mod] | ||
| yield | ||
| for mod in modules_to_remove: | ||
| if mod in sys.modules: | ||
| del sys.modules[mod] |
There was a problem hiding this comment.
Instead of using a custom fixture clean_sys_modules that manually deletes keys from sys.modules (which can permanently remove modules imported by other tests and cause side effects), we can leverage pytest's built-in monkeypatch fixture. Using monkeypatch.setitem(sys.modules, ...) automatically restores the original state of sys.modules after each test runs, making the tests isolated and robust.
| del sys.modules[mod] | ||
|
|
||
|
|
||
| def test_create_channel_otel_installed_and_enabled(monkeypatch, clean_sys_modules): |
There was a problem hiding this comment.
Remove the clean_sys_modules fixture from the test signature since we can use monkeypatch to safely manage sys.modules changes.
| def test_create_channel_otel_installed_and_enabled(monkeypatch, clean_sys_modules): | |
| def test_create_channel_otel_installed_and_enabled(monkeypatch): |
| mock_otel_grpc.client_interceptor.return_value = mock_interceptor | ||
| mock_otel_grpc.intercept_channel.side_effect = lambda ch, inc: f"wrapped_{ch}" | ||
|
|
||
| sys.modules["opentelemetry.instrumentation.grpc"] = mock_otel_grpc |
There was a problem hiding this comment.
Use monkeypatch.setitem to mock the module in sys.modules. This ensures that the modification is automatically reverted after the test completes, preventing side effects on other tests.
| sys.modules["opentelemetry.instrumentation.grpc"] = mock_otel_grpc | |
| monkeypatch.setitem(sys.modules, "opentelemetry.instrumentation.grpc", mock_otel_grpc) |
| assert channel == f"wrapped_{mock_channel}" | ||
|
|
||
|
|
||
| def test_create_channel_otel_installed_but_disabled(monkeypatch, clean_sys_modules): |
There was a problem hiding this comment.
Remove the clean_sys_modules fixture from the test signature.
| def test_create_channel_otel_installed_but_disabled(monkeypatch, clean_sys_modules): | |
| def test_create_channel_otel_installed_but_disabled(monkeypatch): |
| """Verify that create_channel does NOT wrap the channel if tracing is disabled.""" | ||
|
|
||
| mock_otel_grpc = mock.Mock() | ||
| sys.modules["opentelemetry.instrumentation.grpc"] = mock_otel_grpc |
There was a problem hiding this comment.
Use monkeypatch.setitem to mock the module in sys.modules to ensure automatic cleanup after the test runs.
| sys.modules["opentelemetry.instrumentation.grpc"] = mock_otel_grpc | |
| monkeypatch.setitem(sys.modules, "opentelemetry.instrumentation.grpc", mock_otel_grpc) |
| assert channel == mock_channel | ||
|
|
||
|
|
||
| def test_create_channel_otel_not_installed_fails_open(monkeypatch, clean_sys_modules): |
There was a problem hiding this comment.
Remove the clean_sys_modules fixture from the test signature.
| def test_create_channel_otel_not_installed_fails_open(monkeypatch, clean_sys_modules): | |
| def test_create_channel_otel_not_installed_fails_open(monkeypatch): |
| # Ensure it's not in sys.modules | ||
| if "opentelemetry.instrumentation.grpc" in sys.modules: | ||
| del sys.modules["opentelemetry.instrumentation.grpc"] |
There was a problem hiding this comment.
Simply deleting "opentelemetry.instrumentation.grpc" from sys.modules is not sufficient to simulate the package not being installed. If the package is installed in the test environment, Python's import system will find it on sys.path and load it.
To reliably simulate a package not being installed, map its name to None in sys.modules. Combined with monkeypatch.setitem, this is both robust and safe.
# Simulate OTel not being installed by mapping it to None
monkeypatch.setitem(sys.modules, "opentelemetry.instrumentation.grpc", None)… Manager Includes:\n- Soft dependency gRPC interceptor wiring in google-api-core\n- Custom T3 span enrichment in Secret Manager client\n- Unit tests verifying behavior
1 participant
Problem
We are integrating observability (o11y) tracing support into the Google API Core library. The goal for this phase is to allow automatic tracing of gRPC calls when the user has enabled it and installed the required OpenTelemetry packages. We need to ensure the system fails open (does not crash) if the OpenTelemetry packages are missing.
Solution
This Pull Request lays the groundwork by:
opentelemetry-apias a hard dependency.opentelemetry-instrumentation-grpcas a soft dependency.create_channelwhen OpenTelemetry is:Notes to Reviewers
This is a work in progress Pull Request focusing exclusively on gRPC support and o11y infrastructure. We will be building on this foundation in future PRs.
The tests are expected to fail at this stage because the actual implementation of the channel wrapping logic has not been added yet. This Pull Request is being pushed to show progress and align on the testing strategy.
No features are fully implemented yet.