Skip to content

Add read timeout support for fluentd logger#50249

Merged
thaJeztah merged 4 commits intomoby:masterfrom
aaithal:fluent-read-timeout
Aug 28, 2025
Merged

Add read timeout support for fluentd logger#50249
thaJeztah merged 4 commits intomoby:masterfrom
aaithal:fluent-read-timeout

Conversation

@aaithal
Copy link
Contributor

@aaithal aaithal commented Jun 20, 2025

vendor: github.com/philhofer/fwd v1.2.0

full diff: philhofer/fwd@v1.1.2...v1.2.0

vendor: github.com/tinylib/msgp v1.3.0

full diff: tinylib/msgp@v1.1.8...v1.3.0

vendor: github.com/fluent/fluent-logger-golang v1.10.1

full diff: fluent/fluent-logger-golang@v1.9.0...v1.10.1

Update fluent-logger-golang to its latest release. This brings in a
number of changes including the ability to configure a read timeout
and some thread safety improvements.

- What I did

Similar to #49911, these changes add a new configuration option (fluentd-read-timeout), which lets clients specify a read timeout for reading acks from server.

- How I did it

  • Updated fluent-logger-golang to v1.10.0
  • Added readTimeoutKey constant in fluentd.go

- How to verify it

  • Refactored TestWriteTimeoutIsEffective into TestReadWriteTimeoutsAreEffective to expand the coverage by adding a test case for read timeout functionality with ACK-based scenarios

- Human readable description for the release notes

Add a new log option for fluentd log driver (`fluentd-read-timeout`), which enables specifying read timeouts for reading acks from fluentd connections.

@aaithal aaithal force-pushed the fluent-read-timeout branch from efda038 to ee7c7cc Compare June 20, 2025 21:43
@thaJeztah thaJeztah added area/logging status/2-code-review kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. impact/changelog docs/revisit labels Jun 25, 2025
@thaJeztah thaJeztah added this to the 29.0.0 milestone Jun 25, 2025
vendor.mod Outdated
Comment on lines +196 to +197
github.com/philhofer/fwd v1.1.2 // indirect
github.com/philhofer/fwd v1.1.3-0.20240916144458-20a13a1f6b7c // indirect
Copy link
Member

Choose a reason for hiding this comment

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

@thaJeztah
Copy link
Member

Did a quick rebase, and split the dependency updates, also updated to the tagged version of github.com/philhofer/fwd, and removed the toolchain directive

@thaJeztah thaJeztah requested a review from austinvazquez August 8, 2025 17:52
@thaJeztah thaJeztah force-pushed the fluent-read-timeout branch from 7a147ae to bb02fa6 Compare August 8, 2025 18:47
thaJeztah and others added 4 commits August 25, 2025 12:37
full diff: philhofer/fwd@v1.1.2...v1.2.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: tinylib/msgp@v1.1.8...v1.3.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: fluent/fluent-logger-golang@v1.9.0...v1.10.1

Update fluent-logger-golang to its latest release. This brings in a
number of changes including the ability to configure a read timeout
and some thread safety improvements.

Signed-off-by: Anirudh Aithal <aithal@amazon.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Adds a new configuration to specify a read timeout for acks. Fluentd can
be configured to expect acks from the server. In such scenarios, this
configuration enables clients to timeout the read operation if the
server or the connection is unresponsive. The default behavior of
waiting forever remains unchanged.

Signed-off-by: Anirudh Aithal <aithal@amazon.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the fluent-read-timeout branch from bb02fa6 to dbe19a5 Compare August 25, 2025 10:47
@thaJeztah
Copy link
Member

Rebased and updated the dependency to v1.10.1, which downgraded the go version to go1.23.0; #50249 (comment)

Copy link
Contributor

@austinvazquez austinvazquez left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the work on the tests and patience sorting through the fluentd go sdk stuff.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

"LGTM" (but I pushed the last update 😅)

@thaJeztah thaJeztah merged commit 620b931 into moby:master Aug 28, 2025
247 of 248 checks passed
@austinvazquez austinvazquez moved this from New to Complete in 🔦 Maintainer spotlight Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/logging docs/revisit impact/changelog kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. status/2-code-review

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants