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

[BUG]: ristretto uses glog which breaks a lot of applications #1922

Open
mkevac opened this issue Apr 5, 2023 · 7 comments
Open

[BUG]: ristretto uses glog which breaks a lot of applications #1922

mkevac opened this issue Apr 5, 2023 · 7 comments
Labels
kind/bug Something is broken.

Comments

@mkevac
Copy link

mkevac commented Apr 5, 2023

What version of Badger are you using?

v4.0.1

What version of Go are you using?

Not relevant

Have you tried reproducing the issue with the latest release?

Yes

What is the hardware spec (RAM, CPU, OS)?

Not relevant

What steps will reproduce the bug?

Use badger in an app that has -v flag.

Expected behavior and actual result.

Expected: program does not panic.
Actual: program panics.

 marko@daemons3  ~/foobar   badger ±  ./foobar
./foobar flag redefined: v
panic: ./foobar flag redefined: v

goroutine 1 [running]:
flag.(*FlagSet).Var(0xc000168120, {0x1943150, 0x2396f01}, {0x1939880, 0x1}, {0x17ec0d9, 0xd})
	/home/marko/go/src/flag/flag.go:1005 +0x2c5
flag.BoolVar(...)
	/home/marko/go/src/flag/flag.go:732
foobar/service.initialize({0x17eeb64, 0x10}, {0x1774800, 0xc00013c660}, {0x0, 0x0}, 0x1850638)
	/home/marko/pkg/mod/[email protected]/service/service.go:326 +0x3a5
foobar/service.Initialize(...)
	/home/marko/pkg/mod/[email protected]/service/service.go:302
main.main()
	/home/marko/foobar/main.go:42 +0x331

This is because glog library pollutes flags and adds -v flag. This breaks every program that has -v flag.
Library should not pollute global flags.

Dependency graph

 go mod why github.com/golang/glog
# github.com/golang/glog
foobar/ukv/db
github.com/dgraph-io/badger/v4
github.com/dgraph-io/ristretto/z
github.com/golang/glog

See relevant issues:
DataDog/dd-trace-go#1153
dgraph-io/ristretto#293

Additional information

Ristretto is abandoned and they will not remove glog dependency. Consider moving to ristretto fork.

@mkevac mkevac added the kind/bug Something is broken. label Apr 5, 2023
@David-Mao
Copy link

David-Mao commented Jun 26, 2023

Gentle ping. This is quite annoying, and a fix seems not that complicated (as long as someone can commit dgraph-io/ristretto#293 )

@mholt
Copy link

mholt commented Aug 26, 2023

Less gentle ping. 😇

This package is a 4th-degree dependency of my application. I'm surprised to see CLI flags and panics that are out of my control.

This is bizarre to me as glog even states in its README that it's only for internal use at Google (why it is even public is beyond me):

The repository contains an open source version of the log package used inside Google. The master copy of the source lives inside Google, not here. The code in this repo is for export only and is not itself under development. Feature requests will be ignored.

Can we at least get some acknowledgement from @dgraph-io that this dependency will be removed?

@mangalaman93
Copy link
Contributor

Ristretto has a change merged that removes glog. As soon as we do a new release, we should upgrade ristretto in badger.

@Lercher
Copy link

Lercher commented Sep 17, 2023

Just for the sake of better "full-text findability". Besides -v glog also silently adds these command line flags to programs using badger v4

  -alsologtostderr
        log to standard error as well as files
  -log_backtrace_at value
        when logging hits line file:N, emit a stack trace
  -log_dir string
        If non-empty, write log files in this directory
  -logtostderr
        log to standard error instead of files
  -stderrthreshold value
        logs at or above this threshold go to stderr
  -v value
        log level for V logs
  -vmodule value
        comma-separated list of pattern=N settings for file-filtered logging

@pluja
Copy link

pluja commented Nov 25, 2023

Will this be addressed? I'd really like to get rid of the extra flags that are being added by using badger v4... Is there any known way to remove them?

EDIT: I found a way, you can declare your own flag set with flag.NewFlagSet("flagsetname", flag.ExitOnError) and use that instead of default...

Copy link

This issue has been stale for 60 days and will be closed automatically in 7 days. Comment to keep it open.

@github-actions github-actions bot added the Stale label Jul 19, 2024
@mholt
Copy link

mholt commented Jul 19, 2024

Please keep it open.

@github-actions github-actions bot removed the Stale label Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something is broken.
Development

No branches or pull requests

6 participants