mirror of
https://github.com/hanzoai/telemetry.git
synced 2026-10-30 20:01:49 +00:00
Fixes #5169 ## Problem When a gRPC client disconnects normally (e.g., a .NET app shutting down its `TracerProvider`), the collector logs spurious WARN messages from the gRPC transport layer: These are expected during graceful client shutdown but appear alarming to operators and pollute production logs. ## Root cause grpc-go is doing the right thing: messages like this are gated behind `LoggerV2.V(2)` in `internal/transport/http2_server.go`, i.e. they are explicitly classified as supplemental verbosity, not severity-level warnings. They should never reach a WARN sink. The bug is in zapgrpc. `zapgrpc.Logger.V` is implemented as `levelEnabler.Enabled(zapcore.Level(level - 1))`, which conflates grpclog's *verbosity* with zap's *severity*. With WARN enabled, `V(2)` returns true, so grpc-go thinks verbosity level 2 is permitted and emits the message — which then gets formatted by zapgrpc as a WARN entry. See [uber-go/zap#1544](https://github.com/uber-go/zap/issues/1544). ## Solution Wrap the installed `grpclog.LoggerV2` with a `fixedVerbosityLogger` whose `V()` compares the requested level against a fixed threshold (default 0) instead of the underlying zap severity enabler. This restores grpclog's intended verbosity gating, so verbose messages stop at grpc-go before formatting rather than being filtered out of the log stream after the fact. The default threshold of `0` matches grpclog when `GRPC_GO_LOG_VERBOSITY_LEVEL` is unset. This approach: - Addresses the root cause (broken `V()` mapping) rather than string-matching specific message contents. - Stops chatty messages at the source, before grpc-go formats them. - Is generic — kills *all* `V(2)+` noise, including future grpc-go additions, not just two cherry-picked patterns. - Leaves severity-level emission (`Info` / `Warning` / `Error`) completely unchanged. - Tiny: ~10 lines of wrapper plus comment. ## Testing - `TestFixedVerbosity` — asserts `V(0)` is enabled and `V(1)`/`V(2)` are not at the default threshold. - `TestSeverityLevelsUnaffected` — direct `Warning("…")` still fires at WARN, confirming the wrapper does not gate severity. - Existing `TestGRPCLogger` cases continue to pass unchanged. --------- Signed-off-by: Sai Kruthi Wusirika <[email protected]>
141 lines
3.6 KiB
Go
141 lines
3.6 KiB
Go
// Copyright The OpenTelemetry Authors
|
|
// SPDX-License-Identifier: Apache-2.0
|
|
|
|
package grpclog
|
|
|
|
import (
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"go.uber.org/zap"
|
|
"go.uber.org/zap/zapcore"
|
|
"google.golang.org/grpc/grpclog"
|
|
)
|
|
|
|
func TestGRPCLogger(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
cfg zap.Config
|
|
infoLogged bool
|
|
warnLogged bool
|
|
}{
|
|
{
|
|
"collector_info_level_grpc_log_warn",
|
|
zap.Config{
|
|
Level: zap.NewAtomicLevelAt(zapcore.InfoLevel),
|
|
Encoding: "console",
|
|
},
|
|
false,
|
|
true,
|
|
},
|
|
{
|
|
"collector_debug_level_grpc_log_debug",
|
|
zap.Config{
|
|
Level: zap.NewAtomicLevelAt(zapcore.DebugLevel),
|
|
Encoding: "console",
|
|
},
|
|
true,
|
|
true,
|
|
},
|
|
{
|
|
"collector_warn_level_grpc_log_warn",
|
|
zap.Config{
|
|
Development: false, // this must set the grpc loggerV2 to loggerV2
|
|
Level: zap.NewAtomicLevelAt(zapcore.WarnLevel),
|
|
Encoding: "console",
|
|
},
|
|
false,
|
|
true,
|
|
},
|
|
}
|
|
for _, test := range tests {
|
|
t.Run(test.name, func(t *testing.T) {
|
|
obsInfo, obsWarn := false, false
|
|
callerInfo := ""
|
|
hook := zap.Hooks(func(entry zapcore.Entry) error {
|
|
switch entry.Level {
|
|
case zapcore.InfoLevel:
|
|
obsInfo = true
|
|
case zapcore.WarnLevel:
|
|
obsWarn = true
|
|
}
|
|
callerInfo = entry.Caller.String()
|
|
return nil
|
|
})
|
|
|
|
// create new collector zap logger
|
|
logger, err := test.cfg.Build(hook)
|
|
require.NoError(t, err)
|
|
|
|
// create GRPCLogger
|
|
glogger := SetLogger(logger)
|
|
assert.NotNil(t, glogger)
|
|
// grpc does not usually call the logger directly, but through various wrappers that add extra depth
|
|
component := &mockComponent{logger: grpclog.Component("channelz")}
|
|
component.Info(test.name)
|
|
component.Warning(test.name)
|
|
assert.Equal(t, obsInfo, test.infoLogged)
|
|
assert.Equal(t, obsWarn, test.warnLogged)
|
|
// match the file name and line number of Warning() call above
|
|
assert.Contains(t, callerInfo, "internal/grpclog/logger_test.go:77")
|
|
})
|
|
}
|
|
}
|
|
|
|
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
|
|
}
|
|
|
|
func (c *mockComponent) Info(args ...any) {
|
|
c.logger.Info(args...)
|
|
}
|
|
|
|
func (c *mockComponent) Warning(args ...any) {
|
|
c.logger.Warning(args...)
|
|
}
|