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

Decide on a logging package #7

Closed
atc0005 opened this issue Sep 13, 2019 · 20 comments · Fixed by #12
Closed

Decide on a logging package #7

atc0005 opened this issue Sep 13, 2019 · 20 comments · Fixed by #12
Assignees
Labels
Milestone

Comments

@atc0005
Copy link
Owner

atc0005 commented Sep 13, 2019

I've been using the standard library log package and while it works, it seems to be lacking the basic leveled logging features I've come to rely upon over the years (e.g., logger from shell scripts, Python's logging module, ...).

There are a lot of options available, but I'm going to focus first on trying those which are most actively supported and used by some of the bigger teams/projects.

@atc0005 atc0005 added this to the v0.2 milestone Sep 13, 2019
@atc0005 atc0005 self-assigned this Sep 13, 2019
@atc0005
Copy link
Owner Author

atc0005 commented Sep 13, 2019

Some of the packages I hope to evaluate (and or look further into):


@atc0005
Copy link
Owner Author

atc0005 commented Sep 13, 2019

sirupsen/logrus and goinggo/tracelog are both very appealing, the former more so because it is actively maintained and likely has sufficient features to cover all of my use cases.

@atc0005
Copy link
Owner Author

atc0005 commented Sep 13, 2019

This one looked good, but based on issues and pull requests it looks to be abandoned:

https://github.com/op/go-logging

@atc0005
Copy link
Owner Author

atc0005 commented Sep 13, 2019

https://godoc.org/github.com/inconshreveable/log15#hdr-Library_Use

log15 is intended to be useful for library authors as a way to provide configurable logging to users of their library. Best practice for use in a library is to always disable all output for your logger by default and to provide a public Logger instance that consumers of your library can configure. Like so:

package yourlib

import "github.com/inconshreveable/log15"

var Log = log.New()

func init() {
    Log.SetHandler(log.DiscardHandler())
}

Users of your library may then enable it if they like:

import "github.com/inconshreveable/log15"
import "example.com/yourlib"

func main() {
    handler := // custom handler setup
    yourlib.Log.SetHandler(handler)
}

@atc0005
Copy link
Owner Author

atc0005 commented Sep 13, 2019

Worth noting: The inconshreveable/log15 package author used Sirupsen/logrus as an inspiration for their package design.

Unfortunately it doesn't look like the library is receiving active support (quite a few issues/pull requests languishing).

@atc0005
Copy link
Owner Author

atc0005 commented Sep 13, 2019

https://github.com/avelino/awesome-go#logging

Worth looking at later if sirupsen/logrus doesn't work out.

@atc0005
Copy link
Owner Author

atc0005 commented Sep 13, 2019

https://github.com/apsdehal/go-logger

The formatting options remind me of Python's logging module.

@atc0005
Copy link
Owner Author

atc0005 commented Sep 19, 2019

On a related note: does flaggy support limiting options for a flag parameter to a slice of options?

Ex: logging level

@atc0005
Copy link
Owner Author

atc0005 commented Sep 19, 2019

Had some initial stumbling blocks with getting logrus setup/working with the syslog hook, but this code seems to work as intended with v1.4.2 of logrus (the latest at this time of writing):

package main

import (
	"log/syslog"

	// Use `log` if we are going to override the default `log`, otherwise
	// import without an "override" if we want to use the `logrus` name.
	// https://godoc.org/github.com/sirupsen/logrus
	log "github.com/sirupsen/logrus"

	// Official examples use either `lSyslog` or `logrus_syslog`
	// https://godoc.org/github.com/sirupsen/logrus/hooks/syslog
	lSyslog "github.com/sirupsen/logrus/hooks/syslog"
  )

  func main() {
	// https://godoc.org/github.com/sirupsen/logrus#New
	// https://godoc.org/github.com/sirupsen/logrus#Logger
	//log       := logrus.New()
	hook, err := lSyslog.NewSyslogHook("", "", syslog.LOG_INFO, "")

	if err == nil {
		// https://github.com/sirupsen/logrus#hooks
		// https://github.com/sirupsen/logrus/blob/master/hooks/syslog/README.md
		// Seems to require `log.AddHook(hook)`` vs `log.Hooks.Add(hook)`
		log.AddHook(hook)
	}

	log.Info("A walrus appears")

	log.WithFields(log.Fields{
			"animal": "walrus",
			"size":   10,
	}).Info("A group of walrus emerges from the ocean")

	log.WithFields(log.Fields{
			"omg":    true,
			"number": 122,
	}).Warn("The group's number increased tremendously!")

	log.WithFields(log.Fields{
			"omg":    true,
			"number": 100,
	}).Fatal("The ice breaks!")
  }

I worked from the https://github.com/atc0005/go-logrustest repo to figure out the syntax. When I went back and fully read over the logrus README.md file, I found that most of my stumbling blocks had been covered (to some degree or another).

Yet another case where it is better to try and read the manual from front-to-back before beginning.

@atc0005
Copy link
Owner Author

atc0005 commented Sep 19, 2019

Some ideas regarding selection of log output based on current environment:

https://callistaenterprise.se/blogg/teknik/2017/08/02/go-blog-series-part10/

@atc0005
Copy link
Owner Author

atc0005 commented Sep 19, 2019

Scratch notes for validating whether a support logging format was chosen on the command-line:

https://github.com/jessevdk/go-flags/blob/c0795c8afcf41dd1d786bebce68636c199b3bb45/option.go#L251

	if len(option.Choices) != 0 {
		found := false

		for _, choice := range option.Choices {
			if choice == *value {
				found = true
				break
			}
		}

		if !found {
			allowed := strings.Join(option.Choices[0:len(option.Choices)-1], ", ")

			if len(option.Choices) > 1 {
				allowed += " or " + option.Choices[len(option.Choices)-1]
			}

			return newErrorf(ErrInvalidChoice,
				"Invalid value `%s' for option `%s'. Allowed values are: %s",
				*value, option, allowed)
		}
	}

This existing function is probably a better fit for the current state of the code however:

func inFileExtensionsPatterns(ext string, exts []string) bool {

@atc0005
Copy link
Owner Author

atc0005 commented Sep 19, 2019

package main

import (
	"flag"
	"fmt"
	"os"
)

func main() {
	required := []string{"b", "s"}
	os.Setenv("ENV_B", "test")
	flag.String("b", os.Getenv("ENV_B"), "b value")
	flag.Parse()

	seen := make(map[string]bool)
	flag.VisitAll(func(f *flag.Flag) {
		if f.Value.String() != "" {
			seen[f.Name] = true
		}
	})
	for _, req := range required {
		if !seen[req] {
			// or possibly use `log.Fatalf` instead of:
			fmt.Fprintf(os.Stderr, "missing required -%s argument/flag\n", req)
			os.Exit(2) // the same exit code flag.Parse uses
		}
	}
}

Idea for validating that non-empty values have been provided for ALL command-line flags (using the stdlib flag package).

@atc0005
Copy link
Owner Author

atc0005 commented Sep 20, 2019

On a related note: does flaggy support limiting options for a flag parameter to a slice of options?

Ex: logging level

I didn't spot the option when I looked. The only flag package which seemed to do so was jessevdk/go-flags:

https://github.com/jessevdk/go-flags#example

var opts struct {
	// Slice of bool will append 'true' each time the option
	// is encountered (can be set multiple times, like -vvv)
	Verbose []bool `short:"v" long:"verbose" description:"Show verbose debug information"`

	// Example of automatic marshalling to desired type (uint)
	Offset uint `long:"offset" description:"Offset"`

	// Example of a callback, called each time the option is found.
	Call func(string) `short:"c" description:"Call phone number"`

	// Example of a required flag
	Name string `short:"n" long:"name" description:"A name" required:"true"`

	// Example of a flag restricted to a pre-defined set of strings
	Animal string `long:"animal" choice:"cat" choice:"dog"`

	// Example of a value name
	File string `short:"f" long:"file" description:"A file" value-name:"FILE"`

	// Example of a pointer
	Ptr *int `short:"p" description:"A pointer to an integer"`

	// Example of a slice of strings
	StringSlice []string `short:"s" description:"A slice of strings"`

	// Example of a slice of pointers
	PtrSlice []*string `long:"ptrslice" description:"A slice of pointers to string"`

	// Example of a map
	IntMap map[string]int `long:"intmap" description:"A map from string to int"`
}

@atc0005
Copy link
Owner Author

atc0005 commented Sep 22, 2019

Need to pick back up converting the other files in the project to using logrus-based log calls.

@atc0005
Copy link
Owner Author

atc0005 commented Sep 23, 2019

TODO: Need to move file-based logging into main() ?

https://stackoverflow.com/questions/32619318/logging-to-a-file-in-golang

@atc0005
Copy link
Owner Author

atc0005 commented Sep 24, 2019

TODO: Need to move file-based logging into main() ?

https://stackoverflow.com/questions/32619318/logging-to-a-file-in-golang

Some good ideas here:

https://stackoverflow.com/a/43827612/903870

The use of the sync package, the clean wrapper are both good. I don't see a Close() call however.

@atc0005
Copy link
Owner Author

atc0005 commented Sep 24, 2019

For now, I'm bundling a copy of *os.File with the Config object and checking for nil value to determine whether I've set something. Feels crude, broken somehow, but I can revisit this later once I learn more.

@atc0005
Copy link
Owner Author

atc0005 commented Sep 25, 2019

Going to go with logrus for this project. I can pick another package for the next project in order to get some experience with several different ones.

Will use the use-go-flags-and-logrus branch to complete the work for this issue.

atc0005 added a commit that referenced this issue Sep 25, 2019
New packages tracked by go.mod:

- `sirupsen/logrus` v1.4.2
- `jessevdk/go-flags` v1.4.0

Packages removed:

- `integrii/flaggy` v1.2.2

Overall changes:

- Add sirupsen/logrus package to provide leveled, syslog
  and structured logging support
- Remove duplicate logging
- Apply leveled logging to better filter desired
  logging levels
- Add config validation (light)
- Add `String()` to meet `Stringer{}` interface
  requirements for `Config{}` struct
- Add OS-specific handling of syslog hook configuration
  by way of `*_windows.go` and `*_unix.go` files
- (Ab)use `Config{}` to carry a `*os.File` file handle
  for an optional log file (so that it can be freed
  from `main()` via `defer`)
- Add (optional on Linux, unavailable on Windows) syslog
  logging support
- Additional polish for "feedback" log statements; work
  towards having all required information set to INFO
  log level (which is the default)
- Short flags dropped.
  - There are some issues with `go-flags` misdetecting
    leading dashes in file patterns as short flags,
    so instead of dealing with that right now I've
    opted to only support long flag names
  - `go-flags` only supports single letter short flags,
    and with the number of flags that we're using I
    decided to keep things simple for now and only use
    long flag names

New optional configuration options:

- Ignore errors when removing files
- Log format (text or json, defaults to text)
- Log level (large list, mapped where possible to
  syslog logging levels)
- Console output toggle (stdout or stderr)
- Log file path (logging to a log file mutes
  console output)

refs #2, #7
atc0005 added a commit that referenced this issue Sep 25, 2019
New packages tracked by go.mod:

- `sirupsen/logrus` v1.4.2
- `jessevdk/go-flags` v1.4.0

Packages removed:

- `integrii/flaggy` v1.2.2

Overall changes:

- Add sirupsen/logrus package to provide leveled, syslog
  and structured logging support
- Remove duplicate logging
- Apply leveled logging to better filter desired
  logging levels
- Add config validation (light)
- Add `String()` to meet `Stringer{}` interface
  requirements for `Config{}` struct
- Add OS-specific handling of syslog hook configuration
  by way of `*_windows.go` and `*_unix.go` files
- (Ab)use `Config{}` to carry a `*os.File` file handle
  for an optional log file (so that it can be freed
  from `main()` via `defer`)
- Add (optional on Linux, unavailable on Windows) syslog
  logging support
- Additional polish for "feedback" log statements; work
  towards having all required information set to INFO
  log level (which is the default)
- Short flags dropped.
  - There are some issues with `go-flags` misdetecting
    leading dashes in file patterns as short flags,
    so instead of dealing with that right now I've
    opted to only support long flag names
  - `go-flags` only supports single letter short flags,
    and with the number of flags that we're using I
    decided to keep things simple for now and only use
    long flag names

Configuration options:

- (new, optional) Ignore errors when removing files
- (new, optional) Log format (text or json, defaults to text)
- (new, optional) Log level (large list, mapped where possible to
  syslog logging levels)
- (new, optional) Console output toggle (stdout or stderr)
- (new, optional) Log file path (logging to a log file mutes
  console output)
- (changed, required) Number of files to keep out of matches
  list is now a required flag

refs #2, #7
atc0005 added a commit that referenced this issue Sep 25, 2019
New packages tracked by go.mod:

- `sirupsen/logrus` v1.4.2
- `jessevdk/go-flags` v1.4.0

Packages removed:

- `integrii/flaggy` v1.2.2

Overall changes:

- Add sirupsen/logrus package to provide leveled, syslog
  and structured logging support
- Remove duplicate logging
- Apply leveled logging to better filter desired
  logging levels
- Add config validation (light)
- Add `String()` to meet `Stringer{}` interface
  requirements for `Config{}` struct
- Add OS-specific handling of syslog hook configuration
  by way of `*_windows.go` and `*_unix.go` files
- (Ab)use `Config{}` to carry a `*os.File` file handle
  for an optional log file (so that it can be freed
  from `main()` via `defer`)
- Add (optional on Linux, unavailable on Windows) syslog
  logging support
- Additional polish for "feedback" log statements; work
  towards having all required information set to INFO
  log level (which is the default)
- Short flags dropped.
  - There are some issues with `go-flags` misdetecting
    leading dashes in file patterns as short flags,
    so instead of dealing with that right now I've
    opted to only support long flag names
  - `go-flags` only supports single letter short flags,
    and with the number of flags that we're using I
    decided to keep things simple for now and only use
    long flag names

Configuration options:

- (new, optional) Ignore errors when removing files
- (new, optional) Log format (text or json, defaults to text)
- (new, optional) Log level (large list, mapped where possible to
  syslog logging levels)
- (new, optional) Console output toggle (stdout or stderr)
- (new, optional) Log file path (logging to a log file mutes
  console output)
- (changed, required) Number of files to keep out of matches
  list is now a required flag

refs #2, #7
atc0005 added a commit that referenced this issue Sep 25, 2019
New packages:

- `sirupsen/logrus` v1.4.2
- `jessevdk/go-flags` v1.4.0

Packages removed:

- `integrii/flaggy` v1.2.2
- `r3labs/diff` a71de73c46ad

Overall changes:

- Add sirupsen/logrus package to provide leveled, syslog
  and structured logging support
- Remove duplicate logging
- Apply leveled logging to better filter desired
  logging levels
- Add config validation (light)
- Add `String()` to meet `Stringer{}` interface
  requirements for `Config{}` struct
- Add OS-specific handling of syslog hook configuration
  by way of `*_windows.go` and `*_unix.go` files
- (Ab)use `Config{}` to carry a `*os.File` file handle
  for an optional log file (so that it can be freed
  from `main()` via `defer`)
- Add (optional on Linux, unavailable on Windows) syslog
  logging support
- Additional polish for "feedback" log statements; work
  towards having all required information set to INFO
  log level (which is the default)
- Short flags dropped.
  - There are some issues with `go-flags` misdetecting
    leading dashes in file patterns as short flags,
    so instead of dealing with that right now I've
    opted to only support long flag names
  - `go-flags` only supports single letter short flags,
    and with the number of flags that we're using I
    decided to keep things simple for now and only use
    long flag names

Configuration options:

- (new, optional) Ignore errors when removing files
- (new, optional) Log format (text or json, defaults to text)
- (new, optional) Log level (large list, mapped where possible to
  syslog logging levels)
- (new, optional) Console output toggle (stdout or stderr)
- (new, optional) Log file path (logging to a log file mutes
  console output)
- (changed, required) Number of files to keep out of matches
  list is now a required flag

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

Successfully merging a pull request may close this issue.

1 participant