-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Log reconnect attempts #6404
Log reconnect attempts #6404
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
jenkins test it |
libbeat/publisher/pipeline/output.go
Outdated
@@ -69,12 +71,18 @@ func (w *netClientWorker) run() { | |||
return | |||
} | |||
|
|||
if attemptReconnect { | |||
logp.Info("Attempting 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.
+1 for logging connection and reconnect attempts
We have two loops in this methods. First loop attempts to connect. Second loop processes events.
The first loop becomes active after beats startup, once the first event is to be published. That is connecting the outputs is done lazily. In this case the log message should indicate this being the initial connect.
Once the send loop fails, we're about to reconnect.
Having a counter on failed connect attempts would be nice to have as well.
E.g. for the lifetime of an output it would be nice to have log messages like (assuming we would have an 'identifier' per output):
Startup output to <identifier>
Connect to <identifier>
Failed to connect to <identifier>: <reason>
Connect to <identifier>
Connection to <identifier> established
Failed to publish events to <identifier>: %v
Closed connection to <identifier>
Attempting to reconnect to <identifier> with 0 reconnect attempt(s).
Failed to connect to <identifier>: <reason>
Attempting to reconnect to <identifier> with 1 reconnect attempt(s).
Connection to <identifier> established
Closed connection to <identifier>
Shutdown output to <identifier>
That is, we have 4 phases/states to distinguish:
- init worker
- create initial connection
- reconnect
- shutdown
Having an enum to differentiate on state and logging should help here.
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.
I wanted to use hostname as an identifier. However, we don't have it here.
I'll be adding additional logging lines for other events.
@urso We counts for reconnect attempts now. |
@urso I need some pointers for how to create the identifier. The state enum should probably go into the |
@agathver The team is at Elastic{ON} / All hands this and next week so it will take some time to respond. |
Any updates on this? |
The counting looks fine. It would be nice to have an identifier in general. It's the client knowing about the actual endpoint/ID in use. Let's add the identifier to the output.Client.
I used the The console output will return |
28e98fa
to
f9a0852
Compare
@urso, Added identifiers for connections. I have used a composition-like approach for naming (borrowed from the convention in ReactJS), in tune with how most clients are being used. Example, a redis client wrapped with a failover will be |
f9a0852
to
79fb5b9
Compare
b455c64
to
178cac5
Compare
178cac5
to
1b93588
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.
Great improvement. Sorry for having you wait. Thank you for taking the time to add these!
* Log reconnect attempts (elastic#5715) * Add identifiers to connections (cherry picked from commit 06dea8b)
* Log reconnect attempts (elastic#5715) * Add identifiers to connections (cherry picked from commit 06dea8b)
Cherry-pick of PR elastic#6404 to 6.4 branch. Original message: Closes elastic#5175 Logs an info line "Attempting to reconnect" at every reconnect attempt.
Closes #5175
Logs an info line "Attempting to reconnect" at every reconnect attempt.