Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Add detection of plugin child process log level #1736

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cce
Copy link
Contributor

@cce cce commented Sep 21, 2017

This attempts to guess if a plugin is reporting an Error, Warning, or Info log message so that when snapteld reads the captured output, it can log the message at the same level.

@intelsdi-x/snap-maintainers

func logCapturedLine(entry *log.Entry, text string) {
lc := strings.ToLower(text)
switch {
case strings.Contains(lc, "error"):
Copy link
Contributor

Choose a reason for hiding this comment

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

@cce I think it is not a good idea to detect log level in this way.
For example, the message 'warn: cannot collect metric /xx/xx/xx/error_rate from xxx platform' will be an error instead of warning in your case.

Copy link
Contributor Author

@cce cce Oct 2, 2017

Choose a reason for hiding this comment

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

You're right that your example line would be considered an error — I was looking for a quick solution to ensure that a plugin's error logs are not logged at "DEBUG" level. What do you think is the best approach? Could we assume logrus format and look for level=error?

Choose a reason for hiding this comment

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

@cce There's a similar issue already opened.
#1186

Choose a reason for hiding this comment

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

@cce I think parsing "level=error" like strings approach is acceptable. Though it will not be perfect (may be confused if this phrase occurred in log message), but the probability is rather low and the cost is not too high.

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.

3 participants