fix(agent): Skip output buffer initialization in test mode#18918
Conversation
WZH8898
force-pushed
the
fix-test-disk-buffer-init
branch
2 times, most recently
from
e9464ee to
4bc0ec7
Compare
srebhan
left a comment
srebhan
left a comment
There was a problem hiding this comment.
Thanks @WZH8898 for tackling this issue! I do have some comments in the code... Furthermore, I really like the idea of creating a "discard buffer" for test-runs of Telegraf! Can we please
- Add a function to
buffer.goto just test the buffer settings and do nothing else? - Simply override the
buffer_strategyinconfigand avoid adding all those flags?
With this implemented, the config could (in test-mode) first check the buffer settings using the function of item 1 and then override the buffer_strategy with "discard"...
| [agent] | ||
| buffer_strategy = "disk" | ||
| buffer_directory = ` + strconv.Quote(bufferDirectory) + ` | ||
|
|
||
| [[inputs.cpu]] | ||
|
|
||
| [[outputs.influxdb]] | ||
| urls = ["http://localhost:8086"] | ||
| database = "telegraf" |
There was a problem hiding this comment.
How about moving this to a file in the testdata directory instead of writing it somewhere in every test?
| agent = &Telegraf{ | ||
| GlobalFlags: GlobalFlags{ | ||
| config: []string{configFile}, | ||
| test: true, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Can we reuse the agent here and just set test?
| bufferDirectory := filepath.Join(root, "buffer") | ||
| require.NoError(t, os.WriteFile(bufferDirectory, []byte("not a directory"), 0600)) | ||
|
|
||
| cfg := []byte(fmt.Sprintf(` |
There was a problem hiding this comment.
Same as above. Please create a real file in testdata instead of writing it everytime.
|
|
||
| type discardBuffer struct { | ||
| BufferStats | ||
| } | ||
|
|
||
| func newDiscardBuffer(stats BufferStats) *discardBuffer { | ||
| return &discardBuffer{BufferStats: stats} | ||
| } | ||
|
|
||
| func (*discardBuffer) Len() int { | ||
| return 0 | ||
| } | ||
|
|
||
| func (b *discardBuffer) Add(metrics ...telegraf.Metric) int { | ||
| for _, m := range metrics { | ||
| b.metricDropped(m) | ||
| } | ||
| return len(metrics) | ||
| } | ||
|
|
||
| func (*discardBuffer) BeginTransaction(int) *Transaction { | ||
| return &Transaction{} | ||
| } | ||
|
|
||
| func (*discardBuffer) EndTransaction(*Transaction) { | ||
| } | ||
|
|
||
| func (b *discardBuffer) Stats() BufferStats { | ||
| return b.BufferStats | ||
| } | ||
|
|
||
| func (*discardBuffer) Close() error { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Please move this to its own buffer_discard.go file to follow the pattern of the other implementations!
srebhan
added
area/agent
plugin/output
WZH8898
force-pushed
the
fix-test-disk-buffer-init
branch
from
4bc0ec7 to
38af8fe
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 |
srebhan
added
the
ready for final review
3 participants
Summary
This PR updates
telegraf --testand--test-waitto avoid initializing runtime output buffers during config loading, addressing issue #18917.In test mode, outputs are not used. However, Telegraf still initialized output buffers while loading the configuration. When
buffer_strategy = "disk"was configured, this attempted to create/open the disk-backed WAL underbuffer_directory, causingtelegraf --testto fail if the current user did not have write access to that directory.This PR marks config loading as test mode for
--testand--test-wait. Output configs still parse normally, including serializers, but runtime output buffer initialization is skipped for parsed-but-unused outputs in test mode.This PR also adds an internal discard buffer for outputs parsed in test mode.
Tests added in this PR cover:
Checklist
Related issues
resolves #18917