Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

client.go: allow transaction logs to use a specific logger #361

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+34 −4
Draft

client.go: allow transaction logs to use a specific logger #361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 24 additions & 4 deletions client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ type ovsdbClient struct {

trafficSeen chan struct{}

logger *logr.Logger
logger *logr.Logger
txnLogger *logr.Logger
}

// database is everything needed to map between go types and an ovsdb Database
Expand Down Expand Up @@ -181,6 +182,25 @@ func newOVSDBClient(clientDBModel model.ClientDBModel, opts ...Option) (*ovsdbCl
)
ovs.logger = &l
}

// Specific logger for transactions. Set verbosity explicitly.
if ovs.options.txnLogger == nil {
if ovs.options.logger == nil {
// create a new logger to log to stdout
l := stdr.NewWithOptions(log.New(os.Stderr, "", log.LstdFlags), stdr.Options{LogCaller: stdr.All}).WithName("libovsdb").WithValues(
"database", ovs.primaryDBName,
).V(4)
ovs.txnLogger = &l
} else {
l := ovs.options.logger.WithValues(
"database", ovs.primaryDBName,
).V(4)
ovs.txnLogger = &l
}
} else {
l := ovs.options.txnLogger.V(4)
ovs.txnLogger = &l
}
ovs.metrics.init(clientDBModel.Name(), ovs.options.metricNamespace, ovs.options.metricSubsystem)
ovs.registerMetrics()

Expand Down Expand Up @@ -822,9 +842,9 @@ func (o *ovsdbClient) transact(ctx context.Context, dbName string, skipChWrite b
if o.rpcClient == nil {
return nil, ErrNotConnected
}
dbgLogger := o.logger.WithValues("database", dbName).V(4)
if dbgLogger.Enabled() {
dbgLogger.Info("transacting operations", "operations", fmt.Sprintf("%+v", operation))
// Use dedicated logger for providing transactions.
if o.txnLogger.Enabled() {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about injecting the logger from context? If so we can share a client and let different Transact calls use different log contexts.

something like log.FrromContext

o.txnLogger.Info(fmt.Sprintf("%+v", operation))
}
err := o.rpcClient.CallWithContext(ctx, "transact", args, &reply)
if err != nil {
Expand Down
10 changes: 10 additions & 0 deletions client/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type options struct {
timeout time.Duration
backoff backoff.BackOff
logger *logr.Logger
txnLogger *logr.Logger
registry prometheus.Registerer
shouldRegisterMetrics bool // in case metrics are changed after-the-fact
metricNamespace string // prometheus metric namespace
Expand Down Expand Up @@ -137,6 +138,15 @@ func WithLogger(l *logr.Logger) Option {
}
}

// WithTransactionLogger allows setting a specific log sink for transactions.
// Otherwise, the default go log package is used.
func WithTransactionLogger(l *logr.Logger) Option {
return func(o *options) error {
o.txnLogger = l
return nil
}
}

// WithMetricsRegistry allows the user to specify a Prometheus metrics registry.
// If supplied, the metrics as defined in metrics.go will be registered.
func WithMetricsRegistry(r prometheus.Registerer) Option {
Expand Down
Loading