From bf5acb869fef37051b14608f0335ff6fd832f320 Mon Sep 17 00:00:00 2001 From: Sukun Date: Thu, 7 Mar 2024 12:34:57 +0530 Subject: [PATCH] webrtc: use a common logger for all pion logging (#2718) Internally pion uses a lot of logger objects. It's better to just share one logger across everything. --- go.mod | 2 +- p2p/transport/webrtc/listener.go | 18 +--------- p2p/transport/webrtc/logger.go | 58 +++++++++++++++++++++++++++++++ p2p/transport/webrtc/transport.go | 13 ++----- 4 files changed, 63 insertions(+), 28 deletions(-) create mode 100644 p2p/transport/webrtc/logger.go diff --git a/go.mod b/go.mod index e71119b217..1ba2f7ac34 100644 --- a/go.mod +++ b/go.mod @@ -57,7 +57,6 @@ require ( go.uber.org/fx v1.20.1 go.uber.org/goleak v1.3.0 go.uber.org/mock v0.4.0 - go.uber.org/zap v1.27.0 golang.org/x/crypto v0.19.0 golang.org/x/exp v0.0.0-20240213143201-ec583247a57a golang.org/x/sync v0.6.0 @@ -121,6 +120,7 @@ require ( go.uber.org/atomic v1.11.0 // indirect go.uber.org/dig v1.17.1 // indirect go.uber.org/multierr v1.11.0 // indirect + go.uber.org/zap v1.27.0 // indirect golang.org/x/mod v0.15.0 // indirect golang.org/x/net v0.21.0 // indirect golang.org/x/text v0.14.0 // indirect diff --git a/p2p/transport/webrtc/listener.go b/p2p/transport/webrtc/listener.go index af2991ed83..13fdcab05e 100644 --- a/p2p/transport/webrtc/listener.go +++ b/p2p/transport/webrtc/listener.go @@ -20,9 +20,7 @@ import ( manet "github.com/multiformats/go-multiaddr/net" "github.com/multiformats/go-multibase" "github.com/multiformats/go-multihash" - pionlogger "github.com/pion/logging" "github.com/pion/webrtc/v3" - "go.uber.org/zap/zapcore" ) type connMultiaddrs struct { @@ -194,21 +192,7 @@ func (l *listener) setupConnection( } }() - loggerFactory := pionlogger.NewDefaultLoggerFactory() - pionLogLevel := pionlogger.LogLevelDisabled - switch log.Level() { - case zapcore.DebugLevel: - pionLogLevel = pionlogger.LogLevelDebug - case zapcore.InfoLevel: - pionLogLevel = pionlogger.LogLevelInfo - case zapcore.WarnLevel: - pionLogLevel = pionlogger.LogLevelWarn - case zapcore.ErrorLevel: - pionLogLevel = pionlogger.LogLevelError - } - loggerFactory.DefaultLogLevel = pionLogLevel - - settingEngine := webrtc.SettingEngine{LoggerFactory: loggerFactory} + settingEngine := webrtc.SettingEngine{LoggerFactory: pionLoggerFactory} settingEngine.SetAnsweringDTLSRole(webrtc.DTLSRoleServer) settingEngine.SetICECredentials(candidate.Ufrag, candidate.Ufrag) settingEngine.SetLite(true) diff --git a/p2p/transport/webrtc/logger.go b/p2p/transport/webrtc/logger.go new file mode 100644 index 0000000000..d6866a6181 --- /dev/null +++ b/p2p/transport/webrtc/logger.go @@ -0,0 +1,58 @@ +package libp2pwebrtc + +import ( + logging "github.com/ipfs/go-log/v2" + pionLogging "github.com/pion/logging" +) + +var log = logging.Logger("webrtc-transport") + +// pionLog is the logger provided to pion for internal logging +var pionLog = logging.Logger("webrtc-transport-pion") + +// pionLogger wraps the StandardLogger interface to provide a LeveledLogger interface +// as expected by pion +type pionLogger struct { + logging.StandardLogger +} + +var pLog = pionLogger{pionLog} + +var _ pionLogging.LeveledLogger = pLog + +func (l pionLogger) Debug(s string) { + l.StandardLogger.Debug(s) +} + +func (l pionLogger) Error(s string) { + l.StandardLogger.Error(s) +} + +func (l pionLogger) Info(s string) { + l.StandardLogger.Info(s) +} +func (l pionLogger) Warn(s string) { + l.StandardLogger.Warn(s) +} + +func (l pionLogger) Trace(s string) { + l.StandardLogger.Debug(s) +} + +func (l pionLogger) Tracef(s string, args ...interface{}) { + l.StandardLogger.Debugf(s, args) +} + +// loggerFactory returns pLog for all new logger instances +type loggerFactory struct{} + +// NewLogger returns pLog for all new logger instances. Internally pion creates lots of +// separate logging objects unnecessarily. To avoid the allocations we use a single log +// object for all of pion logging. +func (loggerFactory) NewLogger(scope string) pionLogging.LeveledLogger { + return pLog +} + +var _ pionLogging.LoggerFactory = loggerFactory{} + +var pionLoggerFactory = loggerFactory{} diff --git a/p2p/transport/webrtc/transport.go b/p2p/transport/webrtc/transport.go index ae7cc3a5d7..20bfdf98eb 100644 --- a/p2p/transport/webrtc/transport.go +++ b/p2p/transport/webrtc/transport.go @@ -39,19 +39,15 @@ import ( "github.com/libp2p/go-libp2p/p2p/transport/webrtc/pb" "github.com/libp2p/go-msgio" - logging "github.com/ipfs/go-log/v2" ma "github.com/multiformats/go-multiaddr" mafmt "github.com/multiformats/go-multiaddr-fmt" manet "github.com/multiformats/go-multiaddr/net" "github.com/multiformats/go-multihash" "github.com/pion/datachannel" - pionlogger "github.com/pion/logging" "github.com/pion/webrtc/v3" ) -var log = logging.Logger("webrtc-transport") - var dialMatcher = mafmt.And(mafmt.UDP, mafmt.Base(ma.P_WEBRTC_DIRECT), mafmt.Base(ma.P_CERTHASH)) var webrtcComponent *ma.Component @@ -300,12 +296,9 @@ func (t *WebRTCTransport) dial(ctx context.Context, scope network.ConnManagement // the password using the STUN message. ufrag := genUfrag() - settingEngine := webrtc.SettingEngine{} - // suppress pion logs - loggerFactory := pionlogger.NewDefaultLoggerFactory() - loggerFactory.DefaultLogLevel = pionlogger.LogLevelDisabled - settingEngine.LoggerFactory = loggerFactory - + settingEngine := webrtc.SettingEngine{ + LoggerFactory: pionLoggerFactory, + } settingEngine.SetICECredentials(ufrag, ufrag) settingEngine.DetachDataChannels() // use the first best address candidate