fix(agent): Allow overriding collection_jitter with 0s on a per-input basis#18519
fix(agent): Allow overriding collection_jitter with 0s on a per-input basis#18519WZH8898 wants to merge 1 commit intoinfluxdata:masterfrom
Conversation
WZH8898
force-pushed
the
fix-collection-jitter-zero
branch
from
a4cf9f0 to
381d1be
Compare
WZH8898
changed the title
WZH8898
force-pushed
the
fix-collection-jitter-zero
branch
from
381d1be to
75b3ba3
Compare
agent/agent.go
Outdated
| if input.Config.CollectionJitter != 0 { | ||
| jitter = input.Config.CollectionJitter | ||
| } | ||
| jitter := getCollectionJitter(time.Duration(a.Config.Agent.CollectionJitter), input.Config) |
There was a problem hiding this comment.
Please don't move this to a function as it makes code harder to read as we need to follow all the nested redirections! Furthermore, the minimal change would be
jitter := time.Duration(a.Config.Agent.CollectionJitter)
if input.Config.CollectionJitterSet {
jitter = input.Config.CollectionJitter
}which is also much more readable!
agent/agent_test.go
Outdated
| } | ||
|
|
||
| func TestGetCollectionJitter(t *testing.T) { | ||
| t.Parallel() |
There was a problem hiding this comment.
No strong reason beyond a table-driven helper test pattern. I removed this test to avoid low-value indirection.
agent/agent_test.go
Outdated
| tests := []struct { | ||
| name string | ||
| config *models.InputConfig | ||
| want time.Duration |
There was a problem hiding this comment.
Please call this expected as we do in other places.
agent/agent_test.go
Outdated
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() |
There was a problem hiding this comment.
You are right, it wasn't needed here. Removed together with that helper test.
agent/agent_test.go
Outdated
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| got := getCollectionJitter(agentJitter, tt.config) |
There was a problem hiding this comment.
No. Either really test the whole processing path, i.e. set the agent config, then initialize a model and check the jitter or skip the test altogether. This tests really doesn't gain anything as-is.
srebhan
changed the title
WZH8898
force-pushed
the
fix-collection-jitter-zero
branch
from
75b3ba3 to
0245d69
Compare
|
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
|
Hello! I am closing this issue due to inactivity. I hope you were able to resolve your problem, if not please try posting this question in our Community Slack or Community Forums or provide additional details in this issue and reqeust that it be re-opened. Thank you! |
2 participants
Summary
Fix per-input
collection_jitter = "0s"not overriding the agent-level jitter.The input configuration parsing currently discards whether
collection_jitterwas explicitly set. As a result, both an unset value and an explicit0sare represented as the zero-value duration, and the agent logic treats both cases as "use the global collection_jitter".This change preserves whether
collection_jitterwas explicitly configured on the input and updates the agent-side selection logic so that:collection_jitter = "0s"disables jitter for that inputTests were added to verify:
0sis preserved during config parsing0soverrides the agent-level jitterChecklist
Related Issues
resolves #18517