fix(agent): Reject negative collection_offset by majiayu000 · Pull Request #18574 · influxdata/telegraf

majiayu000 · 2026-03-20T03:13:02Z

Summary

Based on the review feedback from @srebhan on #18178, this PR takes the simpler approach of rejecting invalid offset values at startup rather than normalizing them:

Negative agent-level collection_offset → error at startup in runAgent
Negative per-input collection_offset → error during config load in buildInput
Negative per-input interval → error during config load in buildInput

This avoids the ambiguity of silently normalizing negative offsets, which could mean different things to different users.

Changes

Add agent-wide negative collection_offset check in runAgent (cmd/telegraf/telegraf.go), alongside existing interval/flush_interval checks
Add per-input negative interval and collection_offset checks in buildInput (config/config.go)
No changes to NewTicker signature or behavior

Checklist

No AI generated code was used in this PR
AI generated code used in this PR follows the InfluxData Policy on AI-Generated Code Contributions

Related issues

supersedes #18178
resolves #18175

Validate interval and offset parameters in clock.NewTicker to prevent misconfiguration. Negative offsets, offsets >= interval, and non-positive intervals now return an error with a helpful suggestion instead of causing unexpected behavior like rapid-fire ticks. Fixes influxdata#18175 Signed-off-by: majiayu000 <1835304752@qq.com>

Hipska

I'm not in favour of changing the NewTicker footprint, while the checks could be done in other places where it has more context to provide more relevant log messages of where the misconfiguration actually exists.

Hipska · 2026-03-20T08:24:33Z

+	if interval <= 0 {
+		return nil, fmt.Errorf("interval must be positive, got %s", interval)
+	}
+	if offset < 0 {
+		return nil, fmt.Errorf("negative offset %s is not allowed; use a positive offset instead, e.g. interval-offset = %s", offset, interval+offset)
+	}


These checks should be moved to func (c *Config) addInput or func (c *Config) buildInput.

And agent-wide check for interval already exists at func (t *Telegraf) runAgent, so the agent-wide check for negative offset should also be there.

Done — validation is now in buildInput (per-input) and runAgent (agent-wide), not in NewTicker. The NewTicker signature remains *Ticker with no changes.

Hipska · 2026-03-20T08:28:56Z

+	if offset >= interval {
+		return nil, fmt.Errorf("offset %s must be less than interval %s", offset, interval)
+	}


I don't feel like this is something to be checked in NewTicker, but already before calling this in func (a *Agent) runInputs where you can know the input ID/name and its logger.

Agreed — moved the checks out of NewTicker in the previous commit. No validation logic remains in NewTicker.

Agreed — moved the checks out of NewTicker. Per-input validation is in buildInput where the input name is available for error messages.

…xdata#18574 Move offset/interval checks from NewTicker to where they have more context: agent-wide collection_offset check in runAgent (alongside existing interval check), per-input check in buildInput. Revert NewTicker to its original signature returning *Ticker. Signed-off-by: majiayu000 <1835304752@qq.com>

Hipska

Just a suggestion to keep error messages short and clean.

Hipska · 2026-03-20T11:23:39Z

+		return fmt.Errorf(
+			"agent collection_offset must not be negative, found %v; use a positive offset instead (e.g. interval - offset)",
+			c.Agent.CollectionOffset,
+		)


This should be enough

Suggested change

return fmt.Errorf(

"agent collection_offset must not be negative, found %v; use a positive offset instead (e.g. interval - offset)",

c.Agent.CollectionOffset,

)

return fmt.Errorf("agent collection_offset must not be negative, found %v", c.Agent.CollectionOffset)

Done — simplified to return fmt.Errorf("agent collection_offset must not be negative, found %v", c.Agent.CollectionOffset) as suggested.

Hipska · 2026-03-20T11:32:48Z

 	cp.CollectionJitter, _ = c.getFieldDuration(tbl, "collection_jitter")
 	cp.CollectionOffset, _ = c.getFieldDuration(tbl, "collection_offset")
+	if cp.CollectionOffset < 0 {
+		return nil, fmt.Errorf("negative collection_offset %s is not allowed for input %s; use a positive offset instead (e.g. interval - offset)", cp.CollectionOffset, name)


Suggested change

return nil, fmt.Errorf("negative collection_offset %s is not allowed for input %s; use a positive offset instead (e.g. interval - offset)", cp.CollectionOffset, name)

return nil, fmt.Errorf("negative collection_offset %q is not allowed for input %s", cp.CollectionOffset, name)

Done — updated to use %q and shortened the message as suggested.

Signed-off-by: majiayu000 <1835304752@qq.com>

srebhan · 2026-03-20T14:10:12Z

@majiayu000 please restore the PR description template, especially the AI checkboxes and the "Related issues" section! The template is there for a reason namely to query relevant information we need to process the PR and a common structure so we can machine-read things.

Thank you!

srebhan

Thanks for your contribution @majiayu000! I have two comments in the code...

- Move agent-level collection_offset validation from runAgent to LoadConfigData where other agent table checks belong - Remove per-input interval check (to be submitted as separate PR) Signed-off-by: majiayu000 <1835304752@qq.com>

Signed-off-by: majiayu000 <1835304752@qq.com>

The error from buildInput is already prefixed with the input name by addInput, so including it in the message is redundant. Signed-off-by: majiayu000 <1835304752@qq.com>

telegraf-tiger · 2026-03-25T16:42:01Z

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 doesn't change the Telegraf binary size

📦 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

srebhan

Thanks @majiayu000!

skartikey · 2026-03-26T10:46:20Z

@majiayu000 Thanks for the contribution!

Signed-off-by: majiayu000 <1835304752@qq.com> (cherry picked from commit ca72998)

telegraf-tiger Bot added the fix pr to fix corresponding bug label Mar 20, 2026

Hipska suggested changes Mar 20, 2026

View reviewed changes

Hipska reviewed Mar 20, 2026

View reviewed changes

fix: simplify error messages and add interval check per review feedback

f025108

Signed-off-by: majiayu000 <1835304752@qq.com>

Hipska approved these changes Mar 20, 2026

View reviewed changes

srebhan reviewed Mar 23, 2026

View reviewed changes

Comment thread cmd/telegraf/telegraf.go Outdated

Comment thread config/config.go Outdated

Hipska reviewed Mar 23, 2026

View reviewed changes

Comment thread config/config.go Outdated

Hipska approved these changes Mar 23, 2026

View reviewed changes

fix: remove unnecessary int64 conversion per review feedback

c40afeb

Signed-off-by: majiayu000 <1835304752@qq.com>

Hipska approved these changes Mar 25, 2026

View reviewed changes

Comment thread config/config.go Outdated

fix: remove redundant input name from collection_offset error

5006ed0

The error from buildInput is already prefixed with the input name by addInput, so including it in the message is redundant. Signed-off-by: majiayu000 <1835304752@qq.com>

Hipska approved these changes Mar 25, 2026

View reviewed changes

srebhan approved these changes Mar 26, 2026

View reviewed changes

srebhan changed the title ~~fix(agent): reject negative collection_offset with helpful error~~ fix(agent): Reject negative collection_offset Mar 26, 2026

srebhan added area/configuration area/agent ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. labels Mar 26, 2026

srebhan assigned skartikey and mstrandboge Mar 26, 2026

skartikey unassigned mstrandboge Mar 26, 2026

skartikey approved these changes Mar 26, 2026

View reviewed changes

skartikey merged commit ca72998 into influxdata:master Mar 26, 2026
29 checks passed

github-actions Bot added this to the v1.38.2 milestone Mar 26, 2026

srebhan pushed a commit that referenced this pull request Mar 30, 2026

fix(agent): Reject negative collection_offset (#18574)

143c403

Signed-off-by: majiayu000 <1835304752@qq.com> (cherry picked from commit ca72998)

BrewTestBot mentioned this pull request Mar 30, 2026

telegraf 1.38.2 Homebrew/homebrew-core#275143

Merged

	return nil, fmt.Errorf("negative collection_offset %s is not allowed for input %s; use a positive offset instead (e.g. interval - offset)", cp.CollectionOffset, name)
	return nil, fmt.Errorf("negative collection_offset %q is not allowed for input %s", cp.CollectionOffset, name)

Conversation

majiayu000 commented Mar 20, 2026 • edited by srebhan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Checklist

Related issues

Uh oh!

Hipska left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hipska left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

srebhan commented Mar 20, 2026

Uh oh!

srebhan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

telegraf-tiger Bot commented Mar 25, 2026

Artifact URLs

Uh oh!

srebhan left a comment

Choose a reason for hiding this comment

Uh oh!

skartikey commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

majiayu000 commented Mar 20, 2026 •

edited by srebhan

Loading