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

feat(agent): add ignore_error_inputs option for inputs #11313

Closed
wants to merge 8 commits into from

Conversation

observeralone
Copy link

@observeralone observeralone commented Jun 16, 2022

Required for all PRs:

resolves #11289, resolves #10694, resolves #3167

revival of #11304

Summary

Add two options: agent.ignore_error_inputs and plugin.IgnoreInitError
If (plugin.ignore_init_error || ignore_error_inputs) is true, discard the input plugins that produce the error during initialization. Otherwise, the program will exit when an input plugin has an error occurred during the initialization.

observeralone and others added 5 commits June 14, 2022 17:26
* add ignore_init_fail_input option for ignore initialization failed Input influxdata#11289 influxdata#10694

* rename option ignore_init_fail_input to ignore_error_inputs
* docs: add ignore_error_inputs docs

* test: TestAgent_IgnoreErrorInputs

* Update etc/telegraf.conf

Co-authored-by: Kun Zhao <zhao-kun@users.noreply.github.com>

* Update etc/telegraf_windows.conf

Co-authored-by: Kun Zhao <zhao-kun@users.noreply.github.com>

* Update docs/CONFIGURATION.md

Co-authored-by: Kun Zhao <zhao-kun@users.noreply.github.com>

* Update config/config.go

Co-authored-by: Kun Zhao <zhao-kun@users.noreply.github.com>

* Update docs/CONFIGURATION.md

Co-authored-by: Kun Zhao <zhao-kun@users.noreply.github.com>

* modify config/config.go ignore_error_inputs Default

* Update agent/agent_test.go

Co-authored-by: Hao Chen <chenhaox@gmail.com>

* Update agent/agent_test.go

Co-authored-by: Hao Chen <chenhaox@gmail.com>

* Update agent/agent_test.go

Co-authored-by: Hao Chen <chenhaox@gmail.com>

* modify agent/agent_test.go

* Remove the empty line for config.go

* modify agent/agent_test.go

* docs: modify telegraf.conf and telegraf_windows.conf

Co-authored-by: Kun Zhao <zhao-kun@users.noreply.github.com>
Co-authored-by: Hao Chen <chenhaox@gmail.com>
* feat(option): add ignore_init_error option for input

* Update agent/agent.go

* Update docs/CONFIGURATION.md

Co-authored-by: Hao Chen <chenhaox@gmail.com>
@telegraf-tiger telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Jun 16, 2022
@Hipska Hipska requested review from Hipska and srebhan June 16, 2022 11:04
Copy link
Contributor

@Hipska Hipska left a comment

Choose a reason for hiding this comment

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

For me same remark as in #11304 (review): This ignore errors check should happen against the result of Start() instead of Init(). We really want telegraf to halt when the plugin has wrong configuration or it could not be correctly initialised.

@observeralone
Copy link
Author

For me same remark as in #11304 (review): This ignore errors check should happen against the result of Start() instead of Init(). We really want telegraf to halt when the plugin has wrong configuration or it could not be correctly initialised.

The major purpose is let the telegraf can be normally run even if some of the plugins have problem whatever it’s failed in Init or start.
If we think one of plugin failed in Init(), then the whole telegraf should be exited, I respect it, but can we give an option to let the user select what behavior they like?
This requirement is reasonable, and the code changes has no risk. but it really help us a lot - we cannot go through all of plugins to correct the init behavior! So, many thanks if we can accept this PR as soon as possible !

plugins/inputs/statsd/statsd_test.go Outdated Show resolved Hide resolved
@telegraf-tiger
Copy link
Contributor

@observeralone
Copy link
Author

@Hipska
Sorry about the slow reply.
The problem with spaces you mentioned has been rolled back.
This PR is to fix the init error, we hope to deal with this problem, not directly crash.
If we want to do it on the start side, it will be implemented through another PR after.

@srebhan
Copy link
Member

srebhan commented Jun 22, 2022

@observeralone after discussing this matter with the team, we came to the conclusion that a global option to ignore all errors is not the way we want to go. There have been similar proposals (#10111) and requests (#3723, #9778) for the output side.

However, we think it's better to fix the individual plugins to avoid connecting to services in Init but use Start (or maybe Connect in the future) instead. Furthermore, we do see the demand for a more dynamic configuration of telegraf and other features such as life-cycle and connection management. We will start a discussion on how to integrate such features in future versions of telegraf.

For now, I'm closing this PR as a correct implementation requires a rework of Telegraf internals which is out-of-scope for now.

@srebhan srebhan closed this Jun 22, 2022
@default-student
Copy link

While I can completely understand that the general ignoring of errors is not the way to go, I have to say that it would be a great hotfix until the error handling is actually implemented in the various plugins.

In my case weve got a large production floor with many machines serving data via opcua, but not all of them are running all the time.
Telegraf cannot be used in this case as it just exits if it cant reach a server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
4 participants