docs(specs): Add `probe` as value to `startup_error_behavior` #16052

LandonTClipp · 2024-10-21T16:29:08Z

Summary

This PR submits a spec for a new probe value to startup_error_behavior.

Checklist

No AI generated code was used in this PR

Related issues

resolves #16028

LandonTClipp · 2024-10-24T18:44:10Z

@srebhan sorry for the ping, just curious if you have time to give thoughts on this spec?

DStrand1

Thanks for making this PR! I have one question below, but additionally I think it may make more sense to include this as an edit to the existing spec (possibly dependent on the outcome from my other question)

Additionally, could you format the long lines with line breaks to reduce the line length a bit? Thanks!

docs/specs/tsd-008-probe-on-startup.md

srebhan

@LandonTClipp sorry for the late response and thanks for the spec! That already reads quite nicely. I think you need to more describe the intended behavior here than implementations. Use must and should for mandatory and optional features respectively. I would also love to see the Configuration part to be added to TSD-006 to keep all option related aspects there...

docs/specs/tsd-008-probe-on-startup.md

Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>

LandonTClipp · 2024-10-29T16:31:47Z

@srebhan @DStrand1 I addressed all of your comments, including the line width comment. Thank you for your thoughtful insight 🙏🏻

srebhan

Thanks @LandonTClipp for working with us on this spec! Some more comments from my side, but I feel we are very close...

docs/specs/tsd-006-startup-error-behavior.md

docs/specs/tsd-008-probe-on-startup.md

Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>

LandonTClipp · 2024-11-04T18:16:55Z

@srebhan addressed comments and made some typo fixes.

Also related, what do you think about reverting #15916 in favor of this spec?

srebhan

Thanks @LandonTClipp! The PR looks great! Could you please rename the file to tsd-009-probing.md as we already merged TSD-008 in the meantime. Please also change the reference in TSD-006 accordingly!

Regarding reverting the code in nvidia-smi, yes I think you can revert or change the implementation to reflect this spec as we did not release the mentioned feature yet...

LandonTClipp · 2024-11-07T16:15:54Z

Regarding reverting the code in nvidia-smi, yes I think you can revert or change the implementation to reflect this spec as we did not release the mentioned feature yet...

Excellent, I'll probably just revert for now, in case I cannot implement this spec in time for the release.

srebhan

Thanks a lot @LandonTClipp!

LandonTClipp · 2024-11-20T19:11:11Z

@DStrand1 sorry for the ping but I wanted to check what you think of this. I'm motivated to implement it for the plugins I care about :D

LandonTClipp · 2024-12-02T21:02:11Z

@srebhan @DStrand1 pinging again on this

srebhan · 2024-12-05T10:27:27Z

@LandonTClipp could you please fix the linter issues? Seems like you do have a lot of trailing spaces in markdown....

telegraf-tiger · 2024-12-05T17:51:49Z

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

⚠️ This pull request increases the Telegraf binary size by 2.03 % for linux amd64 (new size: 261.9 MB, nightly size 256.7 MB)

📦 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_arm64.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz	windows_i386.zip
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

LandonTClipp · 2024-12-05T20:08:28Z

@srebhan done!

DStrand1

Thanks for the great work!

…data#16052)

Hipska · 2025-01-06T12:26:09Z

docs/specs/tsd-006-startup-error-behavior.md

+If probing is available *and* returns an error Telegraf must *ignore* the
+plugin as-if it was not configured at all.


It is not clear what happens if the probing is not returning an error. As it reads that this probing only needs to be done when startup returns errors.

@Hipska perhaps one more paragraph added here: https://github.com/influxdata/telegraf/pull/16052/files#diff-2d519c82d1022b0befbd7601817fd1b2073ad5e9b607cd9c2205f0529b7d0fffR47 more clearly outlining what Telegraf does on Probe success is what you're looking for? I'm happy to add more clarification.

Yeah indeed.

Also, is probing really only on startup? Meaning if conditions change and recourses come available, that will only take effect when telegraf gets restarted?

That's correct. Sorry for the delayed response on the update to the spec, I will get to it soon.

This spec is backwards compatible so if you don't want this behavior, you don't need to do anything.

As said in other comment, maybe the spec should clarify a use case? When would it be handy to only be able to recover by restarting telegraf completely?

(cherry picked from commit 795b8a9)

Hipska · 2025-01-17T10:51:20Z

docs/specs/tsd-009-probe-on-startup.md

+When plugins are first instantiated, Telegraf will call the plugin's `Start()`
+method (for inputs) or `Connect()` (for outputs) which will initialize its
+configuration based off of config options and the running environment. It is
+sometimes the case that while the initialization step succeeds, the upstream
+service in which the plugin relies on is not actually running, or is not capable
+of being communicated with due to incorrect configuration or environmental
+problems. In situations like this, Telegraf does not detect that the plugin's


Initialisation happens in Init(), connection attempts in Start() (for service plugins) / Connect() (for outputs).

@LandonTClipp

docs: Add probe as value to startup_error_behavior

c19a5a4

telegraf-tiger bot added the docs Issues related to Telegraf documentation and configuration descriptions label Oct 21, 2024

LandonTClipp changed the title ~~docs: Add probe as value to startup_error_behavior~~ docs(specs): Add probe as value to startup_error_behavior Oct 23, 2024

DStrand1 reviewed Oct 28, 2024

View reviewed changes

docs/specs/tsd-008-probe-on-startup.md Outdated Show resolved Hide resolved

srebhan requested changes Oct 29, 2024

View reviewed changes

srebhan self-assigned this Oct 29, 2024

srebhan added the specification label Oct 29, 2024

LandonTClipp and others added 6 commits October 29, 2024 10:35

Update docs/specs/tsd-008-probe-on-startup.md

4b4d955

Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>

Update docs/specs/tsd-008-probe-on-startup.md

d42b9d5

Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>

Address PR comments

096615a

Add probe to the startup-error-behavior spec.

938ff72

Add link to spec

6b45aba

Add whitespace to limit lines to 80 columns.

290aaf4

srebhan reviewed Oct 30, 2024

View reviewed changes

LandonTClipp and others added 7 commits October 31, 2024 12:18

Update docs/specs/tsd-006-startup-error-behavior.md

fb226eb

Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>

Update docs/specs/tsd-008-probe-on-startup.md

d245b17

Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>

Update docs/specs/tsd-008-probe-on-startup.md

50e34d2

Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>

Update docs/specs/tsd-008-probe-on-startup.md

7d5ce02

Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>

Update docs/specs/tsd-008-probe-on-startup.md

23090cc

Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>

Remove duplicated config section

d8b6c46

fix typos

3c50a74

srebhan reviewed Nov 6, 2024

View reviewed changes

Rename to tsd-009

3025e68

LandonTClipp mentioned this pull request Nov 7, 2024

feat(inputs.nvidia-smi): REVERT -- Add probe_on_startup option #16165

Merged

srebhan approved these changes Nov 11, 2024

View reviewed changes

srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Nov 11, 2024

srebhan assigned DStrand1 and unassigned srebhan Nov 11, 2024

LandonTClipp added 2 commits December 5, 2024 11:13

remove trailing whitespaces

65eb8ec

more linting fixes

019c480

DStrand1 approved these changes Dec 11, 2024

View reviewed changes

DStrand1 merged commit 795b8a9 into influxdata:master Dec 11, 2024
27 checks passed

github-actions bot added this to the v1.33.1 milestone Dec 11, 2024

justinwwhuang pushed a commit to justinwwhuang/telegraf_fork that referenced this pull request Dec 19, 2024

docs(specs): Add probe as value to startup_error_behavior (influx…

7a3eab9

…data#16052)

izekr pushed a commit to izekr/telegraf that referenced this pull request Dec 19, 2024

docs(specs): Add probe as value to startup_error_behavior (influx…

a3f304f

…data#16052)

Hipska reviewed Jan 6, 2025

View reviewed changes

DStrand1 pushed a commit that referenced this pull request Jan 10, 2025

docs(specs): Add probe as value to startup_error_behavior (#16052)

179be67

(cherry picked from commit 795b8a9)

Hipska mentioned this pull request Jan 16, 2025

feat(agent): Add support for input probing #16333

Merged

1 task

LandonTClipp deleted the LandonTClipp/probe_spec branch January 16, 2025 16:58

Hipska reviewed Jan 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(specs): Add `probe` as value to `startup_error_behavior` #16052

docs(specs): Add `probe` as value to `startup_error_behavior` #16052

LandonTClipp commented Oct 21, 2024 •

edited

Loading

LandonTClipp commented Oct 24, 2024

DStrand1 left a comment

srebhan left a comment

LandonTClipp commented Oct 29, 2024

srebhan left a comment

LandonTClipp commented Nov 4, 2024

srebhan left a comment

LandonTClipp commented Nov 7, 2024

srebhan left a comment

LandonTClipp commented Nov 20, 2024

LandonTClipp commented Dec 2, 2024

srebhan commented Dec 5, 2024

telegraf-tiger bot commented Dec 5, 2024

Artifact URLs

LandonTClipp commented Dec 5, 2024

DStrand1 left a comment

Hipska Jan 6, 2025

LandonTClipp Jan 16, 2025

Hipska Jan 17, 2025

LandonTClipp Jan 22, 2025

Hipska Jan 22, 2025

Hipska Jan 17, 2025 •

edited

Loading

Hipska Jan 29, 2025

		If probing is available and returns an error Telegraf must ignore the
		plugin as-if it was not configured at all.

docs(specs): Add probe as value to startup_error_behavior #16052

docs(specs): Add probe as value to startup_error_behavior #16052

Conversation

LandonTClipp commented Oct 21, 2024 • edited Loading

Summary

Checklist

Related issues

LandonTClipp commented Oct 24, 2024

DStrand1 left a comment

Choose a reason for hiding this comment

srebhan left a comment

Choose a reason for hiding this comment

LandonTClipp commented Oct 29, 2024

srebhan left a comment

Choose a reason for hiding this comment

LandonTClipp commented Nov 4, 2024

srebhan left a comment

Choose a reason for hiding this comment

LandonTClipp commented Nov 7, 2024

srebhan left a comment

Choose a reason for hiding this comment

LandonTClipp commented Nov 20, 2024

LandonTClipp commented Dec 2, 2024

srebhan commented Dec 5, 2024

telegraf-tiger bot commented Dec 5, 2024

Artifact URLs

LandonTClipp commented Dec 5, 2024

DStrand1 left a comment

Choose a reason for hiding this comment

Hipska Jan 6, 2025

Choose a reason for hiding this comment

LandonTClipp Jan 16, 2025

Choose a reason for hiding this comment

Hipska Jan 17, 2025

Choose a reason for hiding this comment

LandonTClipp Jan 22, 2025

Choose a reason for hiding this comment

Hipska Jan 22, 2025

Choose a reason for hiding this comment

Hipska Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

Hipska Jan 29, 2025

Choose a reason for hiding this comment

docs(specs): Add `probe` as value to `startup_error_behavior` #16052

docs(specs): Add `probe` as value to `startup_error_behavior` #16052

LandonTClipp commented Oct 21, 2024 •

edited

Loading

Hipska Jan 17, 2025 •

edited

Loading