Skip to content
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

TLS Support #96

Closed
wants to merge 28 commits into from
Closed

TLS Support #96

wants to merge 28 commits into from

Conversation

zonito
Copy link
Contributor

@zonito zonito commented Jul 9, 2021

Guide me for Unit Test, if it is a must!

@tagomoris
Copy link
Member

I should have to say "must!", but I know it's not so easy.
@zonito Are you trying this code in your environment?

@tagomoris
Copy link
Member

Just note: #37

@gFazzari
Copy link

Is this PR still alive?

@zonito
Copy link
Contributor Author

zonito commented Nov 10, 2021

@gFazzari Yes!

@tagomoris Yes, I am running in our load test.

Copy link
Member

@tagomoris tagomoris left a comment

Choose a reason for hiding this comment

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

This change looks generally good to me.
I added two comments about details, and I want one more update on README about FluentNetwork to add "tls" as an option there.

@tagomoris
Copy link
Member

@zonito DCO check is now failing. Could you check the instruction?

@zonito
Copy link
Contributor Author

zonito commented Nov 10, 2021

@tagomoris Please confirm. I have also tried to address DCO check

Copy link
Member

@tagomoris tagomoris left a comment

Choose a reason for hiding this comment

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

@fluent/team-golang Do you have any idea, guys?

@tagomoris
Copy link
Member

We can't merge comments without --signoff option.

sparrc and others added 21 commits November 11, 2021 06:12
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: ivan-valkov <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: ivan-valkov <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
They cannot be retrieved for values so one needs to skip them. This aligns with the json.marshal behavior

Signed-off-by: dearoneesama <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: Satoshi Moris Tagomori <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: Satoshi Moris Tagomori <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: Satoshi Moris Tagomori <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: Satoshi Moris Tagomori <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: Satoshi Moris Tagomori <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
We deprecated running tests on Travis in bfce90d. Update the
build status badge accordingly.

Signed-off-by: Fujimoto Seiji <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
PR #77 introduced a new parameter named ForceStopAsyncSend. It can be
used to tell the logger to not try to send all the log messages in its
buffer before closing. Without this parameter, the logger hangs out
whenever it has logs to write and the target Fluentd server is down.

But this new parameter is not enough: the connection is currently lazily
initialized when the logger receive its first message. This blocks the
select reading messages from the queue (until the connection is ready).
Moreover, the connection dialing uses an exponential back-off retry.

Because of that, the logger won't look for messages on `stopRunning`
channel (the channel introduced by PR #77), either because it's
blocked by the Sleep used for the retry or because the connection
dialing is waiting until dialing timeout (eg. TCP timeout).

To fix these edge cases, the time.Sleep() used for back-off retry has
been transformed into a time.After(). Moreover, the dialer.Dial() call
used to connect to Fluentd has been replaced with dialer.DialContext()
and a cancellable context is now used to stop the call to that method.
The associated cancel function is stored in Fluent and got called by
Close() when ForceStopAsyncSend option is enabled.

Finally, the Fluent.run() method has been adapted to wait for both new
messages on f.pending and stop signal sent to f.stopRunning channel.
Previously, both channels were selected independently, in a procedural
fashion.

This fix is motivated by the issue described in: moby/moby#40063.

Signed-off-by: Albin Kerouanton <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Using defered calls to unlock muconn mutex ensures the mutex doesn't
stay locked if the lib user's have a panic recovering mechanism in place
and f.connect(), f.conn.Write() or f.close() panic.

Signed-off-by: Albin Kerouanton <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
Signed-off-by: Love Sharma <[email protected]>
ivan-valkov and others added 7 commits November 11, 2021 06:16
PR #77 introduced a new parameter named ForceStopAsyncSend. It can be
used to tell the logger to not try to send all the log messages in its
buffer before closing. Without this parameter, the logger hangs out
whenever it has logs to write and the target Fluentd server is down.

But this new parameter is not enough: the connection is currently lazily
initialized when the logger receive its first message. This blocks the
select reading messages from the queue (until the connection is ready).
Moreover, the connection dialing uses an exponential back-off retry.

Because of that, the logger won't look for messages on `stopRunning`
channel (the channel introduced by PR #77), either because it's
blocked by the Sleep used for the retry or because the connection
dialing is waiting until dialing timeout (eg. TCP timeout).

To fix these edge cases, the time.Sleep() used for back-off retry has
been transformed into a time.After(). Moreover, the dialer.Dial() call
used to connect to Fluentd has been replaced with dialer.DialContext()
and a cancellable context is now used to stop the call to that method.
The associated cancel function is stored in Fluent and got called by
Close() when ForceStopAsyncSend option is enabled.

Finally, the Fluent.run() method has been adapted to wait for both new
messages on f.pending and stop signal sent to f.stopRunning channel.
Previously, both channels were selected independently, in a procedural
fashion.

This fix is motivated by the issue described in: moby/moby#40063.

Signed-off-by: Albin Kerouanton <[email protected]>
Using defered calls to unlock muconn mutex ensures the mutex doesn't
stay locked if the lib user's have a panic recovering mechanism in place
and f.connect(), f.conn.Write() or f.close() panic.

Signed-off-by: Albin Kerouanton <[email protected]>
@zonito zonito mentioned this pull request Nov 10, 2021
@zonito
Copy link
Contributor Author

zonito commented Nov 20, 2021

Closing this duplicate PR because of #107

@zonito zonito closed this Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants