-
Notifications
You must be signed in to change notification settings - Fork 571
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
Introduce Icinga DB check (like the IDO one) #9417
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
as the Icinga DB check already yields it.
Use the already existing format to pass performance data to Icinga 2 rather than some new JSON structure. Has the additional benefit of doing more things in Go than in C++.
They add no additional information compared to the *_1min values as it's always the same value divided by 60 anyways. Adding the actual value from the last second makes little sense for realistic values of check_interval.
In both C++ and Go, the keys are only used as constant strings, so namespacing them just adds clutter for the `general:*` keys, therefore remove it.
Not only XREAD queries are performed, so the previous error message was incorrect.
Improves readability, even more so after splitting it into separate lines.
Should make it easier to understand that this refers to Redis queries issued by Icinga 2.
Probably makes little difference for an end-user, but for support and development it's great to know which of the two is causing problems.
Sounds more natural in my opinion and I doubt that many users would get that due to the difference between takes/took, this refers to ongoing dumps.
- Add icinga2_ and icingadb_ prefixes to make clear which component is responsible for the value. - Rename heartbeat_lag to heartbeat_age, describes it better in my opinion and sound a bit less like something that should be as close to zero as possible. - Rename redis_dump/database_sync into full_dump/full_sync as this is how these operations are refered to in log messages as well. - Rename Redis backlog into Redis query backlog, makes it a bit clearer in my opinion. - Rename runtime_backlog into runtime_update_backlog, as the component in Icinga DB is called that way and this naming is also exposed in log messages. - Rename dump_config/state/history into config/state/history_dump, makes it sound more natural.
icingadb-web shows multiple lines from the check output collapsed into a single line. The lines containing just minuses make this look cluttered and making making it a heading provides little to no benefit. Even when rendering markdown in the check output at some point, having the lists labeled using normal paragraphs would look just fine.
IcingaDB::GetConnection() uses IcingaDB::m_Rcon which is only initialized in IcingaDB::Start(), therefore add a nullptr check to the check command. Additionally, as m_Rcon is potentially accessed concurrently, add a copy of the value that is safe for concurrent use.
The check makes no attempt to explicitly connect to Redis, it uses the connection of the IcingaDB feature, so this message better describes the state in this situation.
Scope all values using current/last instead of takes/took.
Also use the "current" and "full dump/sync" terminology in the other messages.
If some kind of query is not supposed to be processed at the moment, there is little point in checking it. During a full dump, state updates are suppressed (i.e. delayed), so when a dump takes very long, this would have resulted in a false Redis backlog warning.
d5b89be
to
ce81485
Compare
Al2Klimov
approved these changes
Jun 28, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
backport of #9346