Skip to content

[20.10 backport] backport fluentd log driver async connect fix#43147

Merged
thaJeztah merged 3 commits into
moby:20.10from
PettitWesley:backport-fluentd-fix
Jan 20, 2022
Merged

[20.10 backport] backport fluentd log driver async connect fix#43147
thaJeztah merged 3 commits into
moby:20.10from
PettitWesley:backport-fluentd-fix

Conversation

@PettitWesley

@PettitWesley PettitWesley commented Jan 12, 2022

Copy link
Copy Markdown
Contributor

cherry picks of:

Currently, the fix for the issue is only merged into master. We want to get released sooner if possible.

This issue shows the impact for AWS customers: aws/amazon-ecs-agent#2787

And this comment in that issue shows how I tested both the original PR and this backport: aws/amazon-ecs-agent#2787 (comment)

@thaJeztah thaJeztah changed the title Backport fluentd log driver async connect fix to 20.10 [20.10 backport] backport fluentd log driver async connect fix Jan 12, 2022
@thaJeztah thaJeztah added this to the 20.10.13 milestone Jan 12, 2022
@thaJeztah

Copy link
Copy Markdown
Member

Thanks! Wondering if we should

  • cherry-pick the commit from vendor: github.com/fluent/fluent-logger-golang 1.6.1 #42418 as an intermediate commit in this PR as well (end result would be the same, but for a slightly cleaner history) looks like that one was not yet in the 20.10 branch, and there's some tickets linked to that that were resolved.
  • minor nit: we tend to cherry-pick commits with -X option (which adds a reference to the commit that was cherry-picked) (and the -s (sign-off) option, otherwise the DCO check may fail if the last line of the commit message is not a signed-off-by line)

let me also ping @samuelkarp for the above, and for review 👍

Cam and others added 3 commits January 13, 2022 01:05
Updates the fluent logger library. Namely this fixes a couple places
where the library could panic when closing and writing to channels.

see fluent/fluent-logger-golang#93 and
fluent/fluent-logger-golang#95

closes moby#40829
closes moby#32567

Signed-off-by: Cam <[email protected]>
(cherry picked from commit a6a98d6)
Signed-off-by: Wesley <[email protected]>
Updates the fluent logger library to v1.8.0. Following PRs/commits were
merged since last bump:

* [Add callback for error handling when using
  async](fluent/fluent-logger-golang#97)
* [Fix panic when accessing unexported struct
  field](fluent/fluent-logger-golang#99)
* [Properly stop logger during (re)connect
  failure](fluent/fluent-logger-golang#82)
* [Support a TLS-enabled connection](fluent/fluent-logger-golang@e5d6aa1)

See https://github.com/fluent/fluent-logger-golang/compare/v1.6.1..v1.8.0

Signed-off-by: Albin Kerouanton <[email protected]>
(cherry picked from commit e24d61b)
Signed-off-by: Wesley <[email protected]>
The flag ForceStopAsyncSend was added to fluent logger lib in v1.5.0 (at
this time named AsyncStop) to tell fluentd to abort sending logs
asynchronously as soon as possible, when its Close() method is called.
However this flag was broken because of the way the lib was handling it
(basically, the lib could be stucked in retry-connect loop without
checking this flag).

Since fluent logger lib v1.7.0, calling Close() (when ForceStopAsyncSend
is true) will really stop all ongoing send/connect procedure,
wherever it's stucked.

Signed-off-by: Albin Kerouanton <[email protected]>
(cherry picked from commit bd61629)
Signed-off-by: Wesley <[email protected]>
@PettitWesley

Copy link
Copy Markdown
Contributor Author

@thaJeztah Fixed the commits as requested.

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants