Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .chloggen/fix-zapgrpc-client-close-warning.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. receiver/otlp)
component: pkg/otelcol

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Stop emitting verbose gRPC transport messages at WARN during normal client disconnect.

# One or more tracking issues or pull requests related to the change
issues: [5169]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
grpc-go gates chatty per-RPC notices (e.g. "HandleStreams failed to read frame:
connection reset by peer") behind `LoggerV2.V(2)`. zapgrpc.Logger.V conflates

Check warning on line 20 in .chloggen/fix-zapgrpc-client-close-warning.yaml

View workflow job for this annotation

GitHub Actions / spell-check

Unknown word (zapgrpc)
grpclog verbosity with zap severity, so V(2) returns true whenever WARN is

Check warning on line 21 in .chloggen/fix-zapgrpc-client-close-warning.yaml

View workflow job for this annotation

GitHub Actions / spell-check

Unknown word (grpclog)
enabled and these messages emit at WARN. Wrap the installed grpclog.LoggerV2

Check warning on line 22 in .chloggen/fix-zapgrpc-client-close-warning.yaml

View workflow job for this annotation

GitHub Actions / spell-check

Unknown word (grpclog)
with a corrected V() that compares against a fixed verbosity threshold,
matching grpclog's intended semantics. See uber-go/zap#1544.

Check warning on line 24 in .chloggen/fix-zapgrpc-client-close-warning.yaml

View workflow job for this annotation

GitHub Actions / spell-check

Unknown word (grpclog's)

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [user]
22 changes: 21 additions & 1 deletion otelcol/internal/grpclog/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,26 @@ import (
"google.golang.org/grpc/grpclog"
)

// fixedVerbosityLogger wraps a grpclog.LoggerV2 and reports verbosity against a
// fixed threshold instead of zap's severity enabler.
//
// zapgrpc.Logger.V(level) is implemented as levelEnabler.Enabled(zapcore.Level(level-1)),
// which conflates grpclog's supplemental verbosity with zap's severity. With WARN
// enabled, V(2) returns true, so grpc-go emits chatty per-RPC messages — including
// transport-layer notices logged on normal client disconnect — at WARN. See
// uber-go/zap#1544.
//
// Comparing against a fixed threshold restores grpclog's intended semantics.
// The default of 0 matches grpclog when GRPC_GO_LOG_VERBOSITY_LEVEL is unset.
type fixedVerbosityLogger struct {
grpclog.LoggerV2
verbosity int
}

func (l *fixedVerbosityLogger) V(level int) bool {
return level <= l.verbosity
}

// SetLogger constructs a zapgrpc.Logger instance, and installs it as grpc logger, cloned from baseLogger with
// exact configuration. The minimum level of gRPC logs is set to WARN should the loglevel of the collector is set to
// INFO to avoid copious logging from grpc framework.
Expand All @@ -30,6 +50,6 @@ func SetLogger(baseLogger *zap.Logger) *zapgrpc.Logger {
return c.With([]zapcore.Field{zap.Bool("grpc_log", true)})
}), zap.AddCallerSkip(5)))

grpclog.SetLoggerV2(logger)
grpclog.SetLoggerV2(&fixedVerbosityLogger{LoggerV2: logger})
return logger
}
44 changes: 44 additions & 0 deletions otelcol/internal/grpclog/logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,50 @@ func TestGRPCLogger(t *testing.T) {
}
}

func TestFixedVerbosity(t *testing.T) {
// After SetLogger, the installed grpclog.LoggerV2 must report verbosity
// against a fixed threshold (default 0), not against the zap severity
// enabler. With the default threshold, V(0) is enabled and V(1+) is not,
// regardless of the underlying zap level.
logger, err := zap.NewProduction()
require.NoError(t, err)

SetLogger(logger)

v2 := grpclog.V(0)
assert.True(t, v2, "V(0) should be enabled at the default verbosity threshold")

v2 = grpclog.V(1)
assert.False(t, v2, "V(1) should be disabled at the default verbosity threshold")

v2 = grpclog.V(2)
assert.False(t, v2, "V(2) should be disabled at the default verbosity threshold")
}

func TestSeverityLevelsUnaffected(t *testing.T) {
// Severity-level emission must remain unchanged: a direct Warning call
// still fires regardless of the verbosity threshold.
warnLogged := false
hook := zap.Hooks(func(entry zapcore.Entry) error {
if entry.Level == zapcore.WarnLevel {
warnLogged = true
}
return nil
})

cfg := zap.Config{
Level: zap.NewAtomicLevelAt(zapcore.WarnLevel),
Encoding: "console",
}
logger, err := cfg.Build(hook)
require.NoError(t, err)

SetLogger(logger)
component := &mockComponent{logger: grpclog.Component("channelz")}
component.Warning("real warning unrelated to verbosity")
assert.True(t, warnLogged, "severity-level warnings must still be emitted")
}

type mockComponent struct {
logger grpclog.DepthLoggerV2
}
Expand Down
Loading