[log] logr.Logger and logr.LogSink Support for controller-runtime Compatibility by onelapahead · Pull Request #200 · hyperledger-firefly/common

onelapahead · 2025-11-19T20:37:39Z

Kubernetes controllers love logr as an interface for logging.

FireFly loves logrus (log levels are so much better than -vvvvv).

🍅 🥔

In our Kubernetes operators - we'd prefer to provide the controller-runtime / K8s libraries with an ff-common like logger, so that we can then use log.L(ctx) semantics w/ proper log levels to create a smoother logging experience for our developers vs our users.

These wrapper implementations would help achieve that.

…patibility Signed-off-by: hfuss <hayden.fuss@kaleido.io>

onelapahead · 2025-11-19T20:39:10Z

In a controller-runtime / kubebuilder base operator you can then easily do:

func main() {
    // ... flag parsing ...
    
    // Initialize ff-common logging instead of zapr
    ctx := context.Background()
    log.SetLevel(opts.LogLevel) // or read from flag
    
    // Set the controller-runtime logger to use our ff-common adapter
    ctrl.SetLogger(log.NewLogr(ctx, "firefly-operator"))
    
    // ... rest of setup ...
}

and use log.L(ctx) in the actual reconcilers - all w/ a consistent log output this way.

…for logging Signed-off-by: hfuss <hayden.fuss@kaleido.io>

…lementation Signed-off-by: hfuss <hayden.fuss@kaleido.io>

Signed-off-by: hfuss <hayden.fuss@kaleido.io>

EnriqueL8

Thanks @onelapahead - this is awesome 🙇🏼

Left a few comments/things I don't understand

EnriqueL8 · 2025-11-24T16:14:28Z

+}
+
+// WithFields adds the specified, structured fields to the logger in the context
+func WithFields(ctx context.Context, fields map[string]string) context.Context {


Should we keep the interface consistent with above and instead of a map have keyValuePairs as well?

Well so this one is meant to feel more logrus native, but did struggle with the names to indicate that...

EnriqueL8 · 2025-11-24T16:15:14Z

+
+// Init initializes the sink
+func (l *Sink) Init(_ logr.RuntimeInfo) {
+	// Optional: store callDepth if needed


Yeah don't think we need to implement this for now, will leave a different comment

EnriqueL8 · 2025-11-24T16:15:35Z

+func (l *Sink) Enabled(level int) bool {
+	// Map logr V-levels to ff-common levels
+	// logr: V(0)=info, V(1)=debug, V(2+)=trace
+	switch level {


Check for negative level?

shouldn't be possible for logr but it is is int so will be defensive

EnriqueL8 · 2025-11-24T16:15:58Z

+}
+
+// Info logs an info message with the given keys and values. keysAndValues is not efficiently implemented, use WithValues instead
+func (l *Sink) Info(level int, msg string, keysAndValues ...interface{}) {


Confused how with Sink we still allow passing level int ?

I don't see Debug etc... so maybe this function is actually called Logf ?

aaah is this implementing just the interface ?? that makes sense then

yeah this is all the logr interface - which doesn't have log levels just ints lol - its quite terse.

This is just so k8s libs log through our logging, so we can use our logging in k8s controllers w/o having two different loggers going

EnriqueL8 · 2025-11-24T16:18:07Z

@@ -0,0 +1,37 @@
+// Copyright © 2022 - 2025 Kaleido, Inc.


Suggested change

// Copyright © 2022 - 2025 Kaleido, Inc.

// Copyright © 2025 Kaleido, Inc.

EnriqueL8 · 2025-11-24T16:18:18Z

@@ -0,0 +1,114 @@
+// Copyright © 2022 - 2025 Kaleido, Inc.


Suggested change

// Copyright © 2022 - 2025 Kaleido, Inc.

// Copyright © 2025 Kaleido, Inc.

EnriqueL8 · 2025-11-24T16:18:56Z

+func (l *Sink) buildContext(keysAndValues []interface{}) context.Context {
+	fields := make(map[string]string)
+	for i := 0; i < len(keysAndValues); i += 2 {
+		fields[keysAndValues[i].(string)] = keysAndValues[i+1].(string)


Do we want to be a bit safer with this cast keysAndValues[i].(string) and throw some error?

EnriqueL8 · 2025-11-24T16:22:31Z

+	return logr.New(&Sink{
+		name:   name,
+		ctx:    ctx,
+		logger: L(ctx),


This L( is so miss leading, it's actually getting the logger from context - it was already there so no worries just annoying

maybe I should be using the private impl that L is pointed to ? That would read better

Signed-off-by: hfuss <hayden.fuss@kaleido.io>

…amically via monitoring server; all dbsql logs to trace Signed-off-by: hfuss <hayden.fuss@kaleido.io>

Signed-off-by: hfuss <hayden.fuss@kaleido.io>

onelapahead · 2025-11-30T00:48:41Z

Likely need to break out the dynamic logging changes into a separate PR... will do that once I'm done testing

Signed-off-by: hfuss <hayden.fuss@kaleido.io>

onelapahead · 2025-11-30T23:57:28Z

After testing - this PR is working great for a few different use cases, but logr is yet another dependency for ff-common to support.

I think until ff-common migrates to a newer framework like zap instead of logrus - its best to avoid logr support. All the other logging enhancements will be pulled out to a smaller PR.

[log] logr.Logger and logr.LogSink Support for controller-runtime Com…

6b5ffb0

…patibility Signed-off-by: hfuss <hayden.fuss@kaleido.io>

onelapahead requested a review from a team November 19, 2025 20:37

logr stores an internal ctx for fields; withlogfields and withfields …

143bde6

…for logging Signed-off-by: hfuss <hayden.fuss@kaleido.io>

onelapahead mentioned this pull request Nov 19, 2025

[log] WithLogFields for Configuring Larger Logging Contexts #139

Closed

onelapahead added 2 commits November 19, 2025 16:23

use new withlogfields internally; document inefficiencies of logr imp…

4004679

…lementation Signed-off-by: hfuss <hayden.fuss@kaleido.io>

fixed linter

4629a14

Signed-off-by: hfuss <hayden.fuss@kaleido.io>

EnriqueL8 reviewed Nov 24, 2025

View reviewed changes

onelapahead added 4 commits November 28, 2025 18:36

gatherer for custom metrics exporting

f21e9a0

Signed-off-by: hfuss <hayden.fuss@kaleido.io>

eliminating monitoring server logs; supporting changing log level dyn…

139bc91

…amically via monitoring server; all dbsql logs to trace Signed-off-by: hfuss <hayden.fuss@kaleido.io>

fixed test; removed other noisy log from dbsql

864ecbb

Signed-off-by: hfuss <hayden.fuss@kaleido.io>

move http conn logs to trace

f656209

Signed-off-by: hfuss <hayden.fuss@kaleido.io>

onelapahead added 3 commits November 30, 2025 11:10

more flexible dynamic logging API for future settings

0e811ee

Signed-off-by: hfuss <hayden.fuss@kaleido.io>

fix panic for klog.Object structs in controller-runtime

d73a8c5

Signed-off-by: hfuss <hayden.fuss@kaleido.io>

removing logr implementation

e77cfe7

Signed-off-by: hfuss <hayden.fuss@kaleido.io>

onelapahead mentioned this pull request Nov 30, 2025

[log] [metric] Log Setting API, More TRACE Logs, and Gatherer Support #201

Merged

onelapahead closed this Nov 30, 2025

	// Copyright © 2022 - 2025 Kaleido, Inc.
	// Copyright © 2025 Kaleido, Inc.

Uh oh!

Conversation

onelapahead commented Nov 19, 2025

Uh oh!

onelapahead commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EnriqueL8 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

onelapahead commented Nov 30, 2025

Uh oh!

onelapahead commented Nov 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

onelapahead commented Nov 19, 2025 •

edited

Loading