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

fix(inputs.sysstat): Prevent default sadc_interval from increasing on reload #15212

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

nick-kentik
Copy link
Contributor

Summary

On reload, telegraf throws away the sysstat.Sysstat instance it has and creates a new one; however, this plugin also relies on package-global state (the firstTimestamp variable), which leads to sysstat running sadc with increasingly long intervals following a reload, if the interval to use is not explicitly set via sadc_interval. Fix this by moving the firstTimestamp state into the plugin struct.

Checklist

  • No AI generated code was used in this PR

Related issues

resolves #15102

@nick-kentik nick-kentik force-pushed the nt-fix-sadc-reload-issue branch from fddb887 to 588dfaf Compare April 23, 2024 12:13
@nick-kentik
Copy link
Contributor Author

I thought about adding a test for this - sysstat_interval_test.go is pretty close, it would just need extending with an s2 and a second sleep - but felt bad about doubling its runtime from (say) 3 to 6 seconds, so left it out.

@nick-kentik nick-kentik changed the title sysstat: Fix sadc interval reload issue fix: Sysstat default interval on reload Apr 23, 2024
@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Apr 23, 2024
@nick-kentik nick-kentik force-pushed the nt-fix-sadc-reload-issue branch from 588dfaf to 7d70635 Compare April 23, 2024 13:07
@telegraf-tiger
Copy link
Contributor

@nick-kentik nick-kentik changed the title fix: Sysstat default interval on reload fix(inputs.sysstat): Prevent default sadc_interval from increasing on reload Apr 23, 2024
@telegraf-tiger telegraf-tiger bot added area/sysstat plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Apr 23, 2024
Copy link
Contributor

@powersj powersj left a comment

Choose a reason for hiding this comment

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

Thank you for the PR!

@powersj powersj 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 Apr 23, 2024
@DStrand1 DStrand1 removed their assignment Apr 23, 2024
@srebhan srebhan merged commit e148d6c into influxdata:master Apr 23, 2024
29 checks passed
@github-actions github-actions bot added this to the v1.30.3 milestone Apr 23, 2024
powersj pushed a commit that referenced this pull request May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sysstat fix pr to fix corresponding bug plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inputs.sysstat setting a very long interval for sadc following a SIGHUP
4 participants