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

Missing trace output on syslog level writer #579

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cringha
Copy link

@cringha cringha commented Aug 16, 2023

Just like the title. When i want to use the syslog level writer + syslog output send messages. All others , e.g. "debug", "info" .. worked well. But the trace func , i got empty from syslog service( using journald )

I found there has a missing in syslog.go WriteLevel .

I also created a issues #578 ,

@cringha
Copy link
Author

cringha commented Aug 16, 2023

The code like this

        logwriter, _ := syslog.New(syslog.LOG_DEBUG, "demo")
	ll := zerolog.New(zerolog.SyslogLevelWriter(logwriter))
	level := "trace" 
	zl, err := zerolog.ParseLevel(level)
	if err != nil {
		fmt.Printf("Error parse level %v\n", err)
		return
	}
	fmt.Printf("test %s \n", level)
	ll.WithLevel(zl).Msgf("Message: level %s %v", level, time.Now())

@cringha
Copy link
Author

cringha commented Aug 16, 2023

Fixed, some error in ut

@mitar
Copy link
Contributor

mitar commented Aug 18, 2023

I think this was done on purpose. Mapping zerolog trace to syslog debug might be surprising to people. Syslog does not have trace equivalent.

I think this could be instead better done with some custom LevelWriter which would map trace level to debug level and then you pass that on to syslog writer. Then it is explicit what you want to do. It is then just a question of your configuration. I think existing zerolog syslog implementation is fine as is.

@mitar
Copy link
Contributor

mitar commented Aug 22, 2023

I realized that it is not really possible to map level inside a writer because you would also have to change level field inside the JSON payload so you would have to parse/marshal JSON again. Level mapping should be done at some other level it seems.

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

Successfully merging this pull request may close these issues.

2 participants