test(api-core): [Potentially OBE] Adds integration tests using a local grpc server - #18118
test(api-core): [Potentially OBE] Adds integration tests using a local grpc server #18118chalmerlowe wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces OpenTelemetry tracing support for gRPC channels in google-api-core. It adds a tracer_provider option to ClientOptions, integrates OpenTelemetry gRPC client interceptors in grpc_helpers.create_channel when tracing is enabled, and safely discards the tracing configuration in async channels to prevent runtime errors. Additionally, dependencies and tests are updated to support this new functionality. The review feedback suggests improving mock hygiene in the newly added tests by patching the local module import path for grpc.secure_channel instead of patching the global module directly, adhering to the repository's style guide.
| mock.patch( | ||
| "grpc.secure_channel", return_value=mock_channel | ||
| ) as mock_secure_channel, |
There was a problem hiding this comment.
According to the Repository Style Guide (Section 4: Unit Testing and Mock Hygiene), we should mock the local module import path instead of patching third-party or global modules directly. This ensures that mocks are isolated and do not leak or cause side effects in other tests.
| mock.patch( | |
| "grpc.secure_channel", return_value=mock_channel | |
| ) as mock_secure_channel, | |
| mock.patch( | |
| "google.api_core.grpc_helpers.grpc.secure_channel", return_value=mock_channel | |
| ) as mock_secure_channel, |
References
- Localized Mocking: When mocking standard functions or filesystem checks, mock the local module import path instead of patching builtins globally, ensuring mocks are isolated. (link)
|
|
||
| mock_channel = "raw_channel" | ||
| with ( | ||
| mock.patch("grpc.secure_channel", return_value=mock_channel), |
There was a problem hiding this comment.
According to the Repository Style Guide (Section 4: Unit Testing and Mock Hygiene), we should mock the local module import path instead of patching third-party or global modules directly to ensure proper mock isolation.
| mock.patch("grpc.secure_channel", return_value=mock_channel), | |
| mock.patch("google.api_core.grpc_helpers.grpc.secure_channel", return_value=mock_channel), |
References
- Localized Mocking: When mocking standard functions or filesystem checks, mock the local module import path instead of patching builtins globally, ensuring mocks are isolated. (link)
| def mock_secure(*args, **kwargs): | ||
| return grpc.insecure_channel(args[0]) | ||
|
|
||
| monkeypatch.setattr(grpc, "secure_channel", mock_secure) |
There was a problem hiding this comment.
According to the Repository Style Guide (Section 4: Unit Testing and Mock Hygiene), we should mock the local module import path instead of patching third-party or global modules directly to ensure proper mock isolation.
| monkeypatch.setattr(grpc, "secure_channel", mock_secure) | |
| monkeypatch.setattr("google.api_core.grpc_helpers.grpc.secure_channel", mock_secure) |
References
- Localized Mocking: When mocking standard functions or filesystem checks, mock the local module import path instead of patching builtins globally, ensuring mocks are isolated. (link)
c908003 to
9a9e764
Compare
1f12f3c to
ceb17e5
Compare
5607f9b to
2c110ee
Compare
Warning
This description is AI generated and is no longer accurate. NEEDS REVISION.
Problem
grpc.intercept_channelwhich caused aTypeErrorwith modern OpenTelemetry interceptors because they do not satisfy standardgrpcinterceptor type checks.pkg_resources, which was removed insetuptoolsv82+, causingModuleNotFoundErrorin modern test environments.Solution
grpc.intercept_channeltootel_grpc.intercept_channelhelper provided by the OpenTelemetry instrumentation package, which cleanly handles its own interceptor types.opentelemetry-api>= 1.44.0,opentelemetry-instrumentation-grpc>= 0.65b0) to eliminate reliance on the deprecatedpkg_resources.[testing]extra containingopentelemetry-sdkto facilitate test automation.test_otel_integration_with_fake_endpointwhich spins up a local, generic gRPC server (GenericEchoHandler) to verify that spans are recorded during a real request without needing compiled protobuf stubs.Notes to Reviewers
setuptoolsand modern OpenTelemetry practices._pb2.pyfiles.Expands upon #18069 (Adds additional integration testing to supplement the unit testing)