Skip to content

rpc: add DRPC Logger integration routing to DEV channel#169813

Draft
cthumuluru-crdb wants to merge 2 commits intocockroachdb:masterfrom
cthumuluru-crdb:drpc-logging
Draft

rpc: add DRPC Logger integration routing to DEV channel#169813
cthumuluru-crdb wants to merge 2 commits intocockroachdb:masterfrom
cthumuluru-crdb:drpc-logging

Conversation

@cthumuluru-crdb
Copy link
Copy Markdown
Contributor

@cthumuluru-crdb cthumuluru-crdb commented May 6, 2026

Motivation

cockroachdb/drpc#57 adds a structured drpc.Logger interface to replace the old Log func(error) callback. This PR implements a CockroachDB-side logger that satisfies that interface, routing DRPC library log messages into CockroachDB's logging infrastructure.

Changes

drpcLogger implementation

A drpcLogger struct in pkg/rpc that satisfies drpc.Logger and routes all output to the DEV logging channel:

Method CockroachDB routing When visible
Debugf log.Dev.VEventfDepth (verbosity 2) --verbosity>=2 or --vmodule
Infof log.Dev.InfofDepth always
Errorf log.Dev.ErrorfDepth always
Fatalf log.Dev.FatalfDepth always

The logger carries a depth field so that *Depth calls report the correct DRPC callsite, not the logger wrapper.

Logger injection

The logger is injected at both entry points into the DRPC stack:

  • NewDRPCServer — server-side, via drpcserver.Options.Logger
  • DialDRPC — client-side, via drpcpool.Options.Logger

From there, DRPC propagates the logger internally through the manager, streams, and wire writer.

Linter registration

drpcLogger methods are registered in the fmtsafe linter allowlist (pkg/testutils/lint/passes/fmtsafe/functions.go) so the format-string safety checker recognizes them as printf-like functions.

Design decisions

  • DEV channel (not Health or OPS) — matches the precedent set by grpcLogger in pkg/util/grpcutil/grpc_log.go. Library-internal logs go to DEV; RPC health events go to Health.
  • Verbosity level 2 for Debugf — same threshold used for other verbose internal tracing. Keeps default logs clean while allowing operators to enable DRPC debug output for troubleshooting.

Debug logging is off by default

The DRPC drpcdebug build tag is not added to CockroachDB's default Bazel tags. Debug-level DRPC logs (buffer flushes, stream lifecycle, pool operations) are compiled out entirely — zero overhead on hot RPC paths.

To create a build with DRPC debug logging enabled:

./dev build cockroach --tags=drpcdebug

With a debug build, DRPC debug messages can be enabled at runtime via --verbosity=2 or --vmodule="drpc*=2".

Infof, Errorf, and Fatalf are always active regardless of the build tag — only Debugf callsites are gated.

Depends on

  • cockroachdb/drpc#57 — adds the drpc.Logger interface, replaces Log callback, removes the dead drpcdebug package, adds compile-time debug guard.

Epic: none
Release note: None

Implement a `drpcLogger` that satisfies the new `drpc.Logger` interface
(added upstream in cockroachdb/drpc) and routes DRPC library log messages
to CockroachDB's DEV logging channel — the same channel used for gRPC
library logs.

`Debugf` is routed through `log.Dev.VEventfDepth` at verbosity level 2,
so debug messages (buffer flushes, stream lifecycle, pool operations) are
suppressed by default and only appear when an operator enables verbose
logging via `--verbosity` or `--vmodule`. `Infof`, `Errorf`, and `Fatalf`
are routed to their standard `log.Dev` counterparts with proper call
depth so that file:line metadata points to the DRPC callsite.

The logger is injected into both `NewDRPCServer` (server-side) and
`DialDRPC` (client-side pool), and propagated through the entire DRPC
stack (manager, stream, wire writer, pool) via DRPC's new options
plumbing.

Epic: none
Release note: None

Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
@trunk-io
Copy link
Copy Markdown
Contributor

trunk-io Bot commented May 6, 2026

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented May 6, 2026

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Pick up the squashed commit that removes redundant logger fields from
Manager, Stream, and Pool (opts.Logger used directly).

Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants