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

ApiListener#ListenerCoroutineProc(): get remote endpoint ASAP for logging #9997

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

Al2Klimov
Copy link
Member

On incoming connection timeout we log the remote endpoint which isn't available if it was already disconnected - an exception is thrown. Get it as long as we're still connected not to lose it, nor to get an exception.

ref/IP/48306

This is my alternative implementation for:

closes #9996

…ging

On incoming connection timeout we log the remote endpoint which isn't
available if it was already disconnected - an exception is thrown.  Get it
as long as we're still connected not to lose it, nor to get an exception.
@Al2Klimov Al2Klimov self-assigned this Feb 9, 2024
@cla-bot cla-bot bot added the cla/signed label Feb 9, 2024
@icinga-probot icinga-probot bot added area/distributed Distributed monitoring (master, satellites, clients) bug Something isn't working ref/IP labels Feb 9, 2024
@Al2Klimov
Copy link
Member Author

[2024-02-09 12:48:01 +0100] warning/ApiListener: Timeout while processing incoming connection from [::ffff:127.0.0.1]:52311

Still works. 👍

@Al2Klimov Al2Klimov removed their assignment Feb 9, 2024
@Al2Klimov Al2Klimov marked this pull request as ready for review February 9, 2024 11:48
@yhabteab
Copy link
Member

yhabteab commented Feb 9, 2024

[2024-02-09 12:48:01 +0100] warning/ApiListener: Timeout while processing incoming connection from [::ffff:127.0.0.1]:52311

Still works. 👍

Have you actually reproduced the problem and it's now working with this PR or does that just mean "still works as before"?

Nonetheless, you are still using the same method, which theoretically can still raise an exception, though was not able to trigger this error, with and without your PR. See also here!

@Al2Klimov
Copy link
Member Author

  1. Still works as before.
  2. No, it can't happen even theoretically. I've just accepted the TCP connection, so it is "still" connected.

@julianbrost julianbrost merged commit abea2f2 into master Feb 20, 2024
49 checks passed
@julianbrost julianbrost deleted the ListenerCoroutineProc-remote_endpoint branch February 20, 2024 12:46
@julianbrost julianbrost added this to the 2.15.0 milestone Feb 20, 2024
@julianbrost julianbrost added the consider backporting Should be considered for inclusion in a bugfix release label Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/distributed Distributed monitoring (master, satellites, clients) bug Something isn't working cla/signed consider backporting Should be considered for inclusion in a bugfix release ref/IP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants