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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 8 commits into from
Closed

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

wants to merge 8 commits into from

Conversation

observeralone
Copy link

@observeralone observeralone commented Jun 15, 2022 •
edited
Loading

Required for all PRs:

resolves #11289, resolves #10694, resolves #3167

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 2 commits June 14, 2022 17:26
@observeralone
ignore init fail input option (#1)
ffd32b8 
* 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
@observeralone @zhao-kun @haoel
docs and test: ignore_error_inputs (#2)
d2b4398 
* 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>
@telegraf-tiger
Copy link
Contributor

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

@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 15, 2022
@observeralone observeralone changed the title feat(option)!: add ignore_error_inputs option for inputs feat(option): add ignore_error_inputs option for inputs Jun 15, 2022
@telegraf-tiger
Copy link
Contributor

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

@observeralone
Copy link
Author

!signed-cla

@observeralone
Copy link
Author

Thanks so much for the pull request! 🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

!signed-cla

@Hipska
Copy link
Contributor

Hipska commented Jun 15, 2022

Good idea, but I would like to have this also be configurable per plugin.

For example I accept errors for mqtt_consumer plugin, but not for cpu and disk plugin.

Other agent config options like interval also have this, so should be doable.

@Hipska
Copy link
Contributor

Hipska commented Jun 15, 2022

On the other hand, see #10086 where they fix the input in question to let it retry at each interval, that could be done for the plugins in question as well.

@observeralone
Copy link
Author

observeralone commented Jun 15, 2022 •
edited
Loading

Good idea, but I would like to have this also be configurable per plugin.

For example I accept errors for mqtt_consumer plugin, but not for cpu and disk plugin.

Other agent config options like interval also have this, so should be doable.

Your ideas belong to individual plugin-level switches, which require more granular control.
At the same time it does not conflict with this PR, because it is a master switch
You never know what other inputs will be coming tomorrow because the Init method crashes Telegraf, a master switch is already effective for most needs

@observeralone observeralone reopened this Jun 15, 2022
@observeralone
Copy link
Author

On the other hand, see #10086 where they fix the input in question to let it retry at each interval, that could be done for the plugins in question as well.

Good idea, but I don't just want to fix mongodb's init method.
Of course, I'm also following the mongodb init fix, hope you guys can fix it soon.
At the same time, we are also trying to figure out how to solve the problem of mongodb, thank you for your PR

Hipska
Hipska suggested changes Jun 15, 2022
View reviewed changes
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.

Actually, we don't want to allow ignoring Init() errors. However, we could accept this PR if this check is done against the Start() return value.

Init is for checking actual configuration errors, Start is for actually connecting to endpoints.

Al the same checks could actually also be done for outputs and processors, so only addressing inputs is doing half of the job.

agent/agent.go Outdated Show resolved Hide resolved
@observeralone @haoel
feat(option): add ignore_init_error option for input (#3)
4cdc53c 
* 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>
@Hipska Hipska changed the title feat(option): add ignore_error_inputs option for inputs feat(agent): add ignore_error_inputs option for inputs Jun 16, 2022
Hipska
Hipska suggested changes Jun 16, 2022
View reviewed changes
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.

Simplify the documentation a bit.

docs/CONFIGURATION.md Outdated Show resolved Hide resolved
docs/CONFIGURATION.md Outdated Show resolved Hide resolved
docs/CONFIGURATION.md Outdated Show resolved Hide resolved
@telegraf-tiger
Copy link
Contributor

Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1%)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB RPM TAR GZ ZIP
amd64.deb aarch64.rpm darwin_amd64.tar.gz windows_amd64.zip
arm64.deb armel.rpm darwin_arm64.tar.gz windows_i386.zip
armel.deb armv6hl.rpm freebsd_amd64.tar.gz
armhf.deb i386.rpm freebsd_armv7.tar.gz
i386.deb ppc64le.rpm freebsd_i386.tar.gz
mips.deb riscv64.rpm linux_amd64.tar.gz
mipsel.deb s390x.rpm linux_arm64.tar.gz
ppc64el.deb x86_64.rpm linux_armel.tar.gz
riscv64.deb linux_armhf.tar.gz
s390x.deb linux_i386.tar.gz
linux_mips.tar.gz
linux_mipsel.tar.gz
linux_ppc64le.tar.gz
linux_riscv64.tar.gz
linux_s390x.tar.gz
static_linux_amd64.tar.gz

@observeralone
Copy link
Author

I will close this PR and the new PR address is: #11313

@Hipska Hipska mentioned this pull request Jun 16, 2022
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hipska Hipska Hipska requested changes

Assignees
No one assigned
Labels
area/agent feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin needs design review
Projects
None yet
Milestone
No milestone
2 participants
@observeralone @Hipska