-
Notifications
You must be signed in to change notification settings - Fork 105
Async send #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Async send #56
Conversation
Signed-off-by: alexlry <alexandre@keymantics.com>
…n asynchronous mode - Configuration AsyncConnect is now named Async. - Errors during reconnect no longer cause panic. - Synchronous mode is now always synchronous (reconnect synchronously). Signed-off-by: alexlry <alexandre@keymantics.com>
e2bd9fe
to
3c0ef01
Compare
…8 and fixext 8. Signed-off-by: alexlry <alexandre@keymantics.com>
27ac125
to
bea58e6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally, this change looks very good to me. The diff is reasonably small, without compatibility issues.
fluent/fluent.go
Outdated
} | ||
} | ||
return err | ||
|
||
return errors.New("fluent#write: failed to reconnect") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about to show that it's after retries, with MaxRetry
value?
@@ -89,11 +93,21 @@ func New(config Config) (f *Fluent, err error) { | |||
if config.MaxRetry == 0 { | |||
config.MaxRetry = defaultMaxRetry | |||
} | |||
if config.MaxRetryWait == 0 { | |||
config.MaxRetryWait = defaultMaxRetryWait |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it common thing to show messages about deprecation on os.Stderr
here?
I'm very positive to merge this change. |
Signed-off-by: alexlry <alexandre@keymantics.com>
291216a
to
7d967ba
Compare
Thank you for your time reviewing these changes! I integrated your comments in last commit. |
I'll merge this change 12hours later unless any objections come here. |
Merged. Thank you! |
This option was deperecated in the upstream fluentd logging driver v1.4.0, and while we documented it as deprecated in the API changelog, there was no mention yet in the deprecated docs. relates to: - fluent/fluent-logger-golang#56 - moby/moby#39086 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This option was deperecated in the upstream fluentd logging driver v1.4.0, and while we documented it as deprecated in the API changelog, there was no mention yet in the deprecated docs. relates to: - fluent/fluent-logger-golang#56 - moby/moby#39086 Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit 95b0c43) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This PR is basically #52 without breaking changes.
We are using fluent logging in performance critical services and want to ensure fluent request can't impact service response time.
Noticeable changes in behavior are:
Should fix #33, #54