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

Forward stderr from executed scripts #3851

Closed
hahnjo opened this issue Mar 2, 2018 · 4 comments
Closed

Forward stderr from executed scripts #3851

hahnjo opened this issue Mar 2, 2018 · 4 comments
Labels
area/exec feature request Requests for new plugin and for new features to existing plugins
Milestone

Comments

@hahnjo
Copy link
Contributor

hahnjo commented Mar 2, 2018

Proposal:

The exec plugin should forward any output of stderr, especially in the case of errors.

Current behavior:

[[inputs.exec]]
  commands = [ "./test.sh" ]
#!/bin/sh

echo "Error message" >&2
exit 1
$ ./telegraf -config test.conf
[...]
2018-03-02T15:57:20Z E! Error in plugin [inputs.exec]: exec: exit status 1 for command './test.sh'
2018-03-02T15:57:30Z E! Error in plugin [inputs.exec]: exec: exit status 1 for command './test.sh'

Desired behavior:

$ ./telegraf -config test.conf
[...]
2018-03-02T15:57:20Z E! Error in plugin [inputs.exec]: exec: exit status 1 for command './test.sh'
2018-03-02T15:57:20Z E! './test.sh': Error message
2018-03-02T15:57:30Z E! Error in plugin [inputs.exec]: exec: exit status 1 for command './test.sh'
2018-03-02T15:57:30Z E! './test.sh': Error message

Use case: [Why is this important (helps with prioritizing requests)]

Probably a lot. In my case, I have a small C executable that does some non-trivial initialization work that can fail for quite some different reasons. Getting error messages on the log would speed up finding the problem and even allow pathological analysis (why didn't we get data at that time?).

@hahnjo
Copy link
Contributor Author

hahnjo commented Mar 2, 2018

I might be able to contribute this feature but I first wanted to discuss if it's worth it. I'm not sure either if it's just as simple as adding the output in the exec plugin...

@danielnelson
Copy link
Contributor

I think this is a good idea, but we need to make sure that we gracefully handle the case where there is lots of output to stderr. Probably just need a limit of something like 512 chars and make sure the output is utf-8 encoded. I would also add it to the exiting error to it's one line:

E! Error in plugin [inputs.exec]: exec: exit status 1 for command './test.sh': error message

@danielnelson danielnelson added feature request Requests for new plugin and for new features to existing plugins area/exec labels Mar 5, 2018
@hahnjo
Copy link
Contributor Author

hahnjo commented Mar 5, 2018

Okay, so only in the case of a non-zero return value and not for warnings when the command did gather some data (but possibly not all)?

Your suggestion also implies that we should only display one line so I think this limit makes more sense. What do you think?

@danielnelson
Copy link
Contributor

danielnelson commented Mar 5, 2018

Yeah, I think we should only report on error exit status since I'm sure many are currently using stderr for debugging messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/exec feature request Requests for new plugin and for new features to existing plugins
Projects
None yet
Development

No branches or pull requests

2 participants