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

Add --verbose flag, hide INFO log messages from stderr by default #109

Merged
merged 3 commits into from
Apr 11, 2024

Conversation

tstromberg
Copy link
Collaborator

Fixes output regression from #104

@tstromberg tstromberg requested a review from vaikas April 11, 2024 00:07
@tstromberg
Copy link
Collaborator Author

@vaikas - I feel like I'm doing this wrong, as calls to log.Fatal are now silently discarded. What's the idiomatic way to handle fatal messages?

@vaikas
Copy link
Member

vaikas commented Apr 11, 2024

@vaikas - I feel like I'm doing this wrong, as calls to log.Fatal are now silently discarded. What's the idiomatic way to handle fatal messages?

Hm, so when I do something like this, it seems to work just fine?

package main

import (
	"context"
	"flag"
	"log/slog"
	"os"

	"github.com/chainguard-dev/clog"
)

func main() {
	logSource := flag.Bool("fatal", false, "use fatal logging")
	addSource := flag.Bool("log-source", false, "include source")

	flag.Parse()

	log := clog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{AddSource: *addSource}))

	ctx := clog.WithLogger(context.Background(), log)
	clog.FromContext(ctx).Info("bincapz starting")
	if *logSource {
		clog.FromContext(ctx).Fatalf("fatal logging enabled")
	}
}
➜  clog go run . --fatal
time=2024-04-11T10:32:17.436+03:00 level=INFO msg="bincapz starting"
time=2024-04-11T10:32:17.436+03:00 level=ERROR msg="fatal logging enabled"
exit status 1
➜  clog go run .
time=2024-04-11T10:32:20.549+03:00 level=INFO msg="bincapz starting"

vaikas
vaikas previously approved these changes Apr 11, 2024
Copy link
Member

@vaikas vaikas left a comment

Choose a reason for hiding this comment

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

Is the regression that logs and output both go to stdout by default? I'm not sure mixing them is the right default behaviour. But I'm fine with if that's what we want to do 😂

@tstromberg
Copy link
Collaborator Author

@vaikas - Sorry about my lack of clarity there. The regression is:

  • Previous behavior: Only FATAL level errors were sent to stderr by default (unless --alsologtostderr or --logtostderr) were set.

  • New behavior: All INFO and above messages are sent to stderr by default.

The current behavior makes interactive usage of bincapz a mess, for example, go run . /bin/ls:

...
gs:[{Name:$getuid Base:0 Offset:41812 Data:[103 101 116 117 105 100]} {Name:$getuid Base:0 Offset:45465 Data:[103 101 116 117 105 100]} {Name:$getuid Base:0 Offset:131699 Data:[103 101 116 117 105 100]} {Name:$getuid Base:0 Offset:135007 Data:[103 101 116 117 105 100]}]} {Rule:usr_path Namespace:rules/ref/path/usr.yara Tags:[harmless] Metas:[{Identifier:description Value:References paths within /usr/}] Strings:[{Name:$ref Base:0 Offset:17908 Data:[47 117 115 114 47 108 105 98 47 100 121 108 100]} {Name:$ref Base:0 Offset:18048 Data:[47 117 115 114 47 108 105 98 47 108 105 98 117 116 105 108 46 100 121 108 105 98]} {Name:$ref Base:0 Offset:18096 Data:[47 117 115 114 47 108 105 98 47 108 105 98 110 99 117 114 115 101 115 46 53 46 52 46 100 121 108 105 98]} {Name:$ref Base:0 Offset:18152 Data:[47 117 115 114 47 108 105 98 47 108 105 98 83 121 115 116 101 109 46 66 46 100 121 108 105 98]} {Name:$ref Base:0 Offset:66972 Data:[47 117 115 114 47 108 105 98 47 100 121 108 100]} {Name:$ref Base:0 Offset:67112 Data:[47 117 115 114 47 108 105 98 47 108 105 98 117 116 105 108 46 100 121 108 105 98]} {Name:$ref Base:0 Offset:67160 Data:[47 117 115 114 47 108 105 98 47 108 105 98 110 99 117 114 115 101 115 46 53 46 52 46 100 121 108 105 98]} {Name:$ref Base:0 Offset:67216 Data:[47 117 115 114 47 108 105 98 47 108 105 98 83 121 115 116 101 109 46 66 46 100 121 108 105 98]}]} {Rule:libc Namespace:rules/signal/handle.yara Tags:[harmless] Metas:[] Strings:[{Name:$signal Base:0 Offset:42194 Data:[95 115 105 103 110 97 108]} {Name:$signal Base:0 Offset:45709 Data:[95 115 105 103 110 97 108]} {Name:$signal Base:0 Offset:131972 Data:[95 115 105 103 110 97 108]} {Name:$signal Base:0 Offset:135251 Data:[95 115 105 103 110 97 108]}]} {Rule:kill Namespace:rules/signal/send.yara Tags:[harmless] Metas:[{Identifier:syscall Value:kill} {Identifier:pledge Value:proc}] Strings:[{Name:$kill Base:0 Offset:41919 Data:[95 107 105 108 108]} {Name:$kill Base:0 Offset:45530 Data:[95 107 105 108 108]} {Name:$kill Base:0 Offset:131757 Data:[95 107 105 108 108]} {Name:$kill Base:0 Offset:135072 Data:[95 107 105 108 108]}]} {Rule:bsd_time_conversion Namespace:rules/time/clock-convert.yara Tags:[harmless] Metas:[] Strings:[{Name:$localtime Base:0 Offset:41951 Data:[108 111 99 97 108 116 105 109 101]} {Name:$localtime Base:0 Offset:45548 Data:[108 111 99 97 108 116 105 109 101]} {Name:$localtime Base:0 Offset:131779 Data:[108 111 99 97 108 116 105 109 101]} {Name:$localtime Base:0 Offset:135090 Data:[108 111 99 97 108 116 105 109 101]}]} {Rule:bsd_time Namespace:rules/time/clock-get.yara Tags:[harmless] Metas:[] Strings:[{Name:$_time Base:0 Offset:42467 Data:[95 116 105 109 101]} {Name:$_time Base:0 Offset:45863 Data:[95 116 105 109 101]} {Name:$_time Base:0 Offset:132129 Data:[95 116 105 109 101]} {Name:$_time Base:0 Offset:135405 Data:[95 116 105 109 101]}]} {Rule:isatty Namespace:rules/tty/isatty.yara Tags:[harmless] Metas:[{Identifier:description Value:checks if file handle refers to a terminal}] Strings:[{Name:$ref Base:0 Offset:41905 Data:[105 115 97 116 116 121]} {Name:$ref Base:0 Offset:45523 Data:[105 115 97 116 116 121]} {Name:$ref Base:0 Offset:131748 Data:[105 115 97 116 116 121]} {Name:$ref Base:0 Offset:135065 Data:[105 115 97 116 116 121]}]} {Rule:argparse Namespace:rules/ui/parses-arguments.yara Tags:[harmless] Metas:[{Identifier:description Value:Parses command-line arguments}] Strings:[{Name:$ref2 Base:0 Offset:41059 Data:[111 112 116 97 114 103]} {Name:$ref2 Base:0 Offset:45623 Data:[111 112 116 97 114 103]} {Name:$ref2 Base:0 Offset:131866 Data:[111 112 116 97 114 103]} {Name:$ref2 Base:0 Offset:135165 Data:[111 112 116 97 114 103]} {Name:$ref3 Base:0 Offset:41777 Data:[103 101 116 111 112 116]} {Name:$ref3 Base:0 Offset:45444 Data:[103 101 116 111 112 116]} {Name:$ref3 Base:0 Offset:131674 Data:[103 101 116 111 112 116]} {Name:$ref3 Base:0 Offset:134986 Data:[103 101 116 111 112 116]}]}]"
time=2024-04-11T08:12:50.063-04:00 level=INFO msg="terminal width" width=125 descWidth=91
Path: /bin/ls
Risk: ✅ 1/LOW
time=2024-04-11T08:12:50.063-04:00 level=INFO msg="table width: 69"
---------------------------------------------------------------------
meta   format                 macho
1/LOW  env/TERM               look up or override terminal settings
1/LOW  fs/directory/traverse  traverse filesystem hierarchy
1/LOW  fs/link/read           read value of a symbolic link

@tstromberg
Copy link
Collaborator Author

I learned me a bit about slog and think this PR now behaves close enough to the previous behavior. PTAL.

@tstromberg tstromberg enabled auto-merge (squash) April 11, 2024 12:25
@tstromberg tstromberg changed the title Hide log messages from stderr by default Add --verbose flag, hide INFO log messages from stderr by default Apr 11, 2024
@tstromberg tstromberg merged commit 351c2be into chainguard-dev:main Apr 11, 2024
8 checks passed
egibs pushed a commit to egibs/malcontent that referenced this pull request Aug 5, 2024
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