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

Introduce Icinga DB check (like the IDO one) #9417

Merged
merged 24 commits into from
Jun 28, 2022

Conversation

julianbrost
Copy link
Contributor

backport of #9346

@cla-bot cla-bot bot added the cla/signed label Jun 28, 2022
@icinga-probot icinga-probot bot added area/icingadb New backend enhancement New feature or request labels Jun 28, 2022
@icinga-probot icinga-probot bot added this to the 2.13.4 milestone Jun 28, 2022
Al2Klimov and others added 24 commits June 28, 2022 15:35
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.
@julianbrost julianbrost marked this pull request as ready for review June 28, 2022 14:55
@Al2Klimov Al2Klimov enabled auto-merge June 28, 2022 14:56
@Al2Klimov Al2Klimov merged commit d1ccbdb into support/2.13 Jun 28, 2022
@icinga-probot icinga-probot bot deleted the feature/icingadb-check-2.13 branch June 28, 2022 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/icingadb New backend cla/signed enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants