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.

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

Cannot log with verbosity > 2 #20

Closed
byatesrae opened this issue Sep 12, 2022 · 13 comments
Closed

Cannot log with verbosity > 2 #20

byatesrae opened this issue Sep 12, 2022 · 13 comments

Comments

@byatesrae
Copy link

When logging at a verbosity > 2 no log messages appear, despite configuring zerolog's minimum accepted level.

I can see logging is short circuited here but I'm not sure why given logr's philosophy seems to imply we choose verbosity based on the applications needs.

package main

import (
	"os"

	"github.com/go-logr/zerologr"
	"github.com/rs/zerolog"
)

func main() {
	zLogger := zerolog.New(os.Stderr)
	zLogger = zLogger.Level(-10)
	zerolog.SetGlobalLevel(-10)

	zLogger.WithLevel(-1).Msg("zLogger -1") // trace
	zLogger.WithLevel(-2).Msg("zLogger -2") // less than trace

	logger := zerologr.New(&zLogger)
	logger.V(2).Info("logger v2")
	logger.V(3).Info("logger v3")
}

Go playground.

Expected output:

{"level":"trace","message":"zLogger -1"}
{"level":"-2","message":"zLogger -2"}
{"level":"trace","v":2,"message":"logger v2"}
{"level":"-2","v":3,"message":"logger v3"}

Actual output:

{"level":"trace","message":"zLogger -1"}
{"level":"-2","message":"zLogger -2"}
{"level":"trace","v":2,"message":"logger v2"}

@byatesrae byatesrae changed the title Cannot log verbosity > 2 Cannot log with verbosity > 2 Sep 12, 2022
@hn8
Copy link
Contributor

hn8 commented Sep 15, 2022

@byatesrae When using with logr, it is preferrable to avoid calling zerolog.SetGlobalLevel() and use logr.V() instead, so it is safe to use many packages depending on logr. Currently, logr.V() only supports level addition.
https://github.com/go-logr/logr/blob/master/logr.go#L286-L290

@byatesrae
Copy link
Author

Thanks @hn8, use of zerolog.SetGlobalLevel() aside how might we log with a verbosity > 2 using logr.V()? From my example I'm using logr.V(2) & logr.V(3) yet no log entry is created when using logr.V(3).

Here's a simplified example:

package main

import (
	"os"

	"github.com/go-logr/zerologr"
	"github.com/rs/zerolog"
)

func main() {
	zLogger := zerolog.New(os.Stderr)
	logger := zerologr.New(&zLogger)

	logger.V(2).Info("logger v2")
	logger.V(3).Info("logger v3")
	logger.V(4).Info("logger v4")
	logger.V(5).Info("logger v5")
}

Go Playground

Expected output:

{"level":"trace","v":2,"message":"logger v2"}
{"level":"-1","v":3,"message":"logger v3"}
{"level":"-2","v":4,"message":"logger v4"}
{"level":"-3","v":5,"message":"logger v5"}

Actual output:

{"level":"trace","v":2,"message":"logger v2"}

@hn8
Copy link
Contributor

hn8 commented Sep 21, 2022

@byatesrae zerologr's log level range is limited by underlying implementation of zerolog and logr. zerolog only supports these levels: https://github.com/rs/zerolog#leveled-logging. And logr's philosophy removes warn/fatal/panic. So there are only 3 available logr levels: 0 for info, 1 for debug, 2 for trace and separate error level.

@byatesrae
Copy link
Author

@hn8 I think I see where you're coming from. Although, it may just be me but it doesn't seem obvious from zerologr's README.md that only verbosity up to 2 is supported.

Out of curiosity, given the logr philosophy on logging is that only info (with verbosity) and error are needed... why not have zerologr only log at zerolog's info & error levels? Verbosity could still be a seperate structured log field when logging at info.

@ajanata
Copy link

ajanata commented Jan 24, 2023

@byatesrae When using with logr, it is preferrable to avoid calling zerolog.SetGlobalLevel() and use logr.V() instead, so it is safe to use many packages depending on logr. Currently, logr.V() only supports level addition. https://github.com/go-logr/logr/blob/master/logr.go#L286-L290

Our project uses zerolog as the primary logger, and a library we're using uses logr instead. We want to keep all configuration in zerolog, however we are unable to get debug logs out of the library because of this limitation. What is the suggested workaround in this case?

@tonglil
Copy link

tonglil commented Jan 27, 2023

@ajanata you can still get Zerolog to print debug level logs: https://go.dev/play/p/aoJrlyE494C

@tonglil
Copy link

tonglil commented Jan 27, 2023

I will make a PR to document that Zerolog is limited to printing up to V(2) verbosity.

@hn8
Copy link
Contributor

hn8 commented Jan 28, 2023

@ajanata In zerolog, using levels more verbose than TraceLevel is an undocumented feature. Would you please share what value you pass into zerolog.SetGlobalLevel() and logr.V()? Or do you pass in zerolog.Logger with different level in zelologr.New()?

@hn8
Copy link
Contributor

hn8 commented Jan 28, 2023

@tonglil undocumented zerolog levels do not support LevelSampler and LevelHook, and likely never will since they are struct. zapr may be the better choice than zerologr if higher verbosity is needed.

@thockin
Copy link
Collaborator

thockin commented Jan 29, 2023

The whole point of https://github.com/rs/zerolog/pull/350/files was to allow higher V levels. If zerologr can't make that work, it's not really a very useful logr implementation, IMO.

@hn8
Copy link
Contributor

hn8 commented Jan 30, 2023

It is possible but not enabled by default in new PR. Explained here #25 (comment)

@ajanata
Copy link

ajanata commented Feb 1, 2023

@ajanata In zerolog, using levels more verbose than TraceLevel is an undocumented feature. Would you please share what value you pass into zerolog.SetGlobalLevel() and logr.V()? Or do you pass in zerolog.Logger with different level in zelologr.New()?

Apologies, I suppose I should open a bug with opentelemetry-go, since they use V(5) for debug. We are using the standard debug level in zerolog.

@hn8
Copy link
Contributor

hn8 commented Feb 2, 2023

@ajanata You can now use SetMaxV to adjust global level of Zerolog in zerologr v1.2.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants