Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

logging: Ensure errors are logged correctly. #27

Merged
merged 3 commits into from
Apr 11, 2018

Conversation

jodh-intel
Copy link
Contributor

Ensure all errors are logged using the logrus.WithError() API.
The current logrus.Error(err) calls will result in log messages
containing the following fields:

level=error msg="..."

Whereas using logrus.WithError() will result in the expected:

level=error error="..."

Fixes #26.

Signed-off-by: James O. D. Hunt james.o.hunt@intel.com

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

also some sort of loop deref cleanup, and a typo fix....
lgtm

Fix typo.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel
Copy link
Contributor Author

Not sure what you mean about the loop cleanup, but created a separate commit for the typo.

@grahamwhaley
Copy link
Contributor

just this little bit and its friends didn't feel directly needed by the Error work, that's all:

-				nextKnob := ksmThrottleIntervals[k.currentKnob].nextKnob
-				interval := ksmThrottleIntervals[k.currentKnob].interval
+				currentKnob := k.currentKnob
+				nextKnob := ksmThrottleIntervals[currentKnob].nextKnob
+				interval := ksmThrottleIntervals[currentKnob].interval

Ensure all errors are logged using the `logrus.WithError()` API.
The current `logrus.Error(err)` calls will result in log messages
containing the following fields:

```
level=error msg="..."
```

Whereas using `logrus.WithError()` will result in the expected:

```
level=error error="..."
```

Fixes kata-containers#26.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Supplement the existing logs when errors occur with further structured
log fields.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel
Copy link
Contributor Author

Better now?

Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

yep - thanks (I was only noting you know...)

@jodh-intel
Copy link
Contributor Author

ping @sboeuf, @bergwolf.

@sboeuf sboeuf merged commit e2f4445 into kata-containers:master Apr 11, 2018
@sboeuf sboeuf removed the review label Apr 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants