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.

Sign up for GitHub

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

Jump to bottom

API: also log error behind "No data received on new API connection" #9928

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

API: also log error behind "No data received on new API connection" #9928

wants to merge 1 commit into from

Conversation

Al2Klimov
Copy link
Member

No description provided.

@Al2Klimov Al2Klimov added area/distributed Distributed monitoring (master, satellites, clients) area/log Logging related core/quality Improve code, libraries, algorithms, inline docs labels Dec 11, 2023
@cla-bot cla-bot bot added the cla/signed label Dec 11, 2023
@Al2Klimov
Copy link
Member Author

This simplifies debugging, just like here:

[2023-12-11 17:14:27 +0100] information/ApiListener: No data received on new API connection from [::ffff:127.0.0.1]:50906. Ensure that the remote endpoints are properly configured in a cluster setup. Error: unexpected record

@Al2Klimov Al2Klimov mentioned this pull request Dec 11, 2023
Merged
@Al2Klimov
Copy link
Member Author

Btw. the API as such still works, even for small HTTP 1.0 requests which close the connection:

[2023-12-11 17:25:19 +0100] information/ApiListener: New client connection from [::ffff:127.0.0.1]:50949 (no client certificate)
[2023-12-11 17:25:19 +0100] warning/HttpServerConnection: Unauthorized request: GET /v1/

As expected. async_fill() requires only "some" data after all.

@Al2Klimov Al2Klimov self-assigned this Dec 22, 2023
@Al2Klimov
Copy link
Member Author

Even a clean shutdown works as expected: [2023-12-22 16:32:34 +0100] information/ApiListener: No data received on new API connection from [::ffff:127.0.0.1]:51452. Ensure that the remote endpoints are properly configured in a cluster setup. Error: End of file

@Al2Klimov Al2Klimov removed their assignment Dec 22, 2023
@Al2Klimov Al2Klimov requested a review from yhabteab November 8, 2024 15:06
yhabteab

This comment was marked as resolved.

Sign in to view

@Al2Klimov Al2Klimov force-pushed the no-data-received-on-new-api-connection branch from 82110a9 to a365079 Compare November 8, 2024 15:42
yhabteab
yhabteab reviewed Nov 12, 2024
View reviewed changes
lib/remote/apilistener.cpp
Copy link
Member

Choose a reason for hiding this comment

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

Prefixing these with Error doesn't seem appropriate because their corresponding log level isn't marked as such. I suggest appending the extra hints within parentheses to the log entries. Additionally, I was only able to trigger those two error cases, which are quite trivial, so I am unsure if there are other scenarios that would trigger that error condition.

[2024-11-12 09:30:53 +0100] warning/ApiListener: No data received on new API connection from [::1]:54558 for identity 'satellite'. Ensure that the remote endpoints are properly configured in a cluster setup. Error: Operation canceled
[2024-11-12 09:32:06 +0100] warning/ApiListener: No data received on new API connection from [::1]:54763 for identity 'satellite'. Ensure that the remote endpoints are properly configured in a cluster setup. Error: End of file

Copy link
Member Author

Choose a reason for hiding this comment

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

(openssl s_client -connect 127.0.0.1:5665 & sleep 2 && kill -9 $!)

[2024-11-12 11:11:32 +0100] information/ApiListener: No data received on new API connection from [::ffff:127.0.0.1]:55668: short read. Ensure that the remote endpoints are properly configured in a cluster setup.

Copy link
Member

Choose a reason for hiding this comment

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

This was addressed in https://github.com/Icinga/icinga2/compare/a365079e03770d04e65a796dd5a6044283b2db59..80a4567349f03393fb43140477dd43af74035b8d, if I am not mistaken.

@Al2Klimov Al2Klimov force-pushed the no-data-received-on-new-api-connection branch from a365079 to 80a4567 Compare November 12, 2024 10:00
@Al2Klimov Al2Klimov requested a review from oxzi January 21, 2025 11:11
oxzi
oxzi reviewed Jan 21, 2025
View reviewed changes
Copy link
Member

@oxzi oxzi left a comment

Choose a reason for hiding this comment

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

LGTM. Please rebase to make the CI happy.

@Al2Klimov Al2Klimov force-pushed the no-data-received-on-new-api-connection branch from 80a4567 to 411c57a Compare January 24, 2025 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yhabteab yhabteab yhabteab left review comments

@oxzi oxzi oxzi left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area/distributed Distributed monitoring (master, satellites, clients) area/log Logging related cla/signed core/quality Improve code, libraries, algorithms, inline docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Al2Klimov @oxzi @yhabteab