Skip to content

perf(spanner): disable gRPC channelz by default - #9320

Open
olavloite wants to merge 1 commit into
mainfrom
spanner-disable-channelz
Open

olavloite wants to merge 1 commit into
mainfrom
spanner-disable-channelz

Conversation

@olavloite

Copy link
Copy Markdown
Contributor

@grpc/grpc-js has Channelz enabled by default ('grpc.enable_channelz': 1), which instruments every channel, subchannel, socket, and RPC attempt.

On every call start and completion, Channelz updates call counters and allocates timestamps (new Date()) across both channel and transport layers, as well as maintaining trace ring buffers and child reference maps. Cloud Spanner telemetry is handled via OpenTelemetry, and the client does not expose or consume the gRPC Channelz admin service.

This change sets 'grpc.enable_channelz': 0 by default in Spanner and GrpcService channel options, causing @grpc/grpc-js to use no-op stubs and eliminating per-RPC allocations. Users who require live connection introspection (e.g. via grpcdebug) can still explicitly pass 'grpc.enable_channelz': 1 in SpannerOptions to re-enable it.

@olavloite
olavloite requested a review from a team as a code owner September 14, 2026 15:30
@product-auto-label product-auto-label Bot added the api: spanner Issues related to the Spanner API. label Sep 14, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request disables gRPC Channelz by default to reduce per-RPC tracking and allocation overhead, while allowing users to override this behavior via client options. The feedback suggests making the new unit tests more robust by explicitly asserting that the pooled channels are initialized before checking their properties.

Comment thread handwritten/spanner/test/spanner.ts Outdated
Comment thread handwritten/spanner/test/spanner.ts Outdated
@olavloite
olavloite force-pushed the spanner-disable-channelz branch from 5371e26 to 3459f10 Compare September 14, 2026 15:49
@olavloite

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request disables gRPC Channelz by default to reduce per-RPC tracking and allocation overhead, while allowing it to be overridden via client options. It also refactors client instantiation, promise chains, and adds corresponding unit tests. Regarding the feedback, the refactoring of getGrpcCredentials_ to use .catch(callback) after .then() introduces a risk of invoking the callback twice if the callback itself throws an error; it is recommended to use the two-argument form of .then() instead.

Comment thread handwritten/spanner/src/common-grpc/service.ts Outdated
@grpc/grpc-js has Channelz enabled by default ('grpc.enable_channelz': 1),
which instruments every channel, subchannel, socket, and RPC attempt.

On every call start and completion, Channelz updates call counters and
allocates timestamps (`new Date()`) across both channel and transport
layers, as well as maintaining trace ring buffers and child reference maps.
Cloud Spanner telemetry is handled via OpenTelemetry, and the client does
not expose or consume the gRPC Channelz admin service.

This change sets 'grpc.enable_channelz': 0 by default in Spanner and
GrpcService channel options, causing @grpc/grpc-js to use no-op stubs
and eliminating per-RPC allocations. Users who require live connection
introspection (e.g. via grpcdebug) can still explicitly pass
'grpc.enable_channelz': 1 in SpannerOptions to re-enable it.
@olavloite
olavloite force-pushed the spanner-disable-channelz branch from 3459f10 to fe420b1 Compare September 14, 2026 16:46
@olavloite

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the grpc.enable_channelz option to Spanner, disabling it by default to eliminate per-RPC tracking and allocation overhead while allowing users to override it. It also refactors client instantiation, cleans up promise chains, and adds corresponding tests. The review feedback points out that the integration test in test/spanner.ts asserts on deep, undocumented internal properties of @grpc/grpc-js and grpc-gcp, which makes the test suite fragile. It is recommended to simplify this test by only verifying that the option is correctly propagated.

Comment thread handwritten/spanner/test/spanner.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the Spanner API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant