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 Generic Log functions with level via argument #863

Merged
merged 3 commits into from Jan 2, 2019

Conversation

lugray
Copy link
Contributor

@lugray lugray commented Nov 16, 2018

I wanted to be able to use Log with a variable level, and it seemed like I should add Logf and Logln if I was going to add Log.

Copy link
Collaborator

@richpoirier richpoirier left a comment

Choose a reason for hiding this comment

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

Hit Approve too fast.

Copy link
Collaborator

@richpoirier richpoirier left a comment

Choose a reason for hiding this comment

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

@lugray Do you only need Log() on Entry? I would think you'd want it on Logger?

Maybe some more context in the description would help -- some examples.

@lugray
Copy link
Contributor Author

lugray commented Dec 20, 2018

@lugray Do you only need Log() on Entry? I would think you'd want it on Logger?

In my case I did only need it on Entry, but you're right, it makes sense to make the change on Logger too, so I did that.

An example of why you'd want this, and where I use it:

const ignoreError logrus.Level = math.MaxInt32

func runAndLogGitCmd(ctx context.Context, cmd shell.Supervisor, defaultLevel logrus.Level, levelOverrides map[string]logrus.Level) (stdout []byte, err error) {
	stdout, stderr, err := cmd.RunAndGetOutput()
	if err == nil {
		return stdout, nil
	}

	gitErr := parseGitCmdError(string(stderr), err)

	level := defaultLevel
	if levelOverrides != nil {
		if l, ok := levelOverrides[gitErr.ID()]; ok {
			level = l
		}
	}

	if level == ignoreError {
		return stdout, nil
	}

	log(ctx, gitErr).WithFields(gitErr.LogFields()).Log(level, "error while running git command")
	return stdout, gitErr
}

Some git errors in our app should be fully ignored, others should be downgraded to WarnLevel, most should be ErrorLevel, but some commands are generally allowed to fail and so called with a defaultLevel of WarnLevel.

@richpoirier
Copy link
Collaborator

@lugray Maybe add a TestLog() test case to logrus_test.go and then we'll be good to merge.

@lugray
Copy link
Contributor Author

lugray commented Jan 2, 2019

Added test - thanks for the review on this.

@richpoirier richpoirier merged commit e1e72e9 into sirupsen:master Jan 2, 2019
@lugray lugray deleted the generic_log branch January 3, 2019 13:23
arcana261 pushed a commit to arcana261/logrus_sentry that referenced this pull request Jan 5, 2019
…/logrus

Apparently due to this [merge request](sirupsen/logrus#863)
another level to stacktrace was introduced. So if we intend to skip it
by default we have to increase default number of skips in our
configuration.
@tmshn tmshn mentioned this pull request Sep 1, 2019
cgxxv pushed a commit to cgxxv/logrus that referenced this pull request Mar 25, 2022
Add Generic Log functions with level via argument
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.

None yet

2 participants