Skip to content

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

Merged
Al2Klimov merged 1 commit intomasterfrom
no-data-received-on-new-api-connection
Feb 3, 2025
Merged

API: also log error behind "No data received on new API connection"#9928
Al2Klimov merged 1 commit intomasterfrom
no-data-received-on-new-api-connection

Conversation

@Al2Klimov
Copy link
Copy Markdown
Member

@Al2Klimov Al2Klimov commented Dec 11, 2023

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
Copy Markdown
Member Author

Al2Klimov commented Dec 11, 2023

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
Copy Markdown
Member Author

Al2Klimov commented Dec 11, 2023

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
Copy Markdown
Member Author

Al2Klimov commented Dec 22, 2023

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
Copy Markdown
Member

@yhabteab yhabteab Nov 12, 2024

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
Copy Markdown
Member Author

@Al2Klimov Al2Klimov Nov 12, 2024

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
Copy Markdown
Member

@oxzi oxzi Jan 21, 2025

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
Copy Markdown
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
@Al2Klimov Al2Klimov merged commit 065dfe4 into master Feb 3, 2025
@Al2Klimov Al2Klimov deleted the no-data-received-on-new-api-connection branch February 3, 2025 14:39
@Al2Klimov Al2Klimov added this to the 2.15.0 milestone Feb 3, 2025
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 approved these changes

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

2.15.0

Development

Successfully merging this pull request may close these issues.

3 participants

@Al2Klimov @oxzi @yhabteab