uptime-kuma icon indicating copy to clipboard operation
uptime-kuma copied to clipboard

Add nut server monitor

Open johnelliott opened this issue 1 year ago • 7 comments

Description

Resolves #3932

Hello and thank you to @CommanderStorm for on-boarding me to the project in #3932

This is my first PR to the project. I read the contributions guide and I think I'm going about the style correctly.

Some questions I had were regarding the database schema,

Otherwise I added a separate file for the monitor as Frank suggested, used the new Knex migrations, and modeled the data after the way MQTT worked since the authentication of NUT isn't http basic auth.

Some shortcomings of this patch are that I wasn't sure how to deal with isImportant and the current patch creates some noise with the toasts popping up for each ping. I'd appreciate any help that can point me in towards solving that.

Type of change

New feature (non-breaking change which adds functionality)

Checklist

  • [x] My code follows the style guidelines of this project
  • [x] I ran ESLint and other linters for modified files
  • [x] I have performed a self-review of my own code and tested it
  • [x] I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)

Most of the added code is anonymous functions and doesn't ad library code for other code to use, so I didn't add extra documentation

  • [ ] My changes generate no new warnings

I do see a warning I would like help with:

WARN: The ping is not effective when the status is DOWN

Screenshots

Edit Monitor

image

johnelliott avatar Oct 30 '23 20:10 johnelliott

I recently realized that there are too many columns (almost 80 columns) and nearly reach to the limit of MariaDB unfortunately.

So I think it is not allow to add more columns for specific monitor type starting from now.

I don't have a general solution for this yet. However, I may plan to make mqtt_username and mqtt_password as for general username and password in the future.

Read more about the row size issue: https://mariadb.com/kb/en/troubleshooting-row-size-too-large-errors-with-innodb/

louislam avatar Oct 31 '23 05:10 louislam

Another (a bit more permananet) solution might be to store the monior config as a json string. Thoughts?

CommanderStorm avatar Oct 31 '23 08:10 CommanderStorm

Agreed. We can store all extra fields as a TEXT column in json, with the exception of fields that:

  • might need indexing,
  • or are very common, e.g. port, username, password

Then at database load and store time, we should be able to use destructuring to make those fields available in the regular monitor object.

This would slightly increase storage requirements, e.g. boolean would get encoded as "true", but the impact should be insignificant.

chakflying avatar Oct 31 '23 14:10 chakflying

make mqtt_username and mqtt_password as for general username and password

This is one option that makes sense to me as a new contributor. I considered but decided not to use the mqtt column as it might be confusing to future users. This is one of those tough naming choices :)

johnelliott avatar Oct 31 '23 18:10 johnelliott

@CommanderStorm @chakflying

Actually I did similar design for the notification table, because I expected there will be more and more notifications, but did not expect there could be so many monitor types.

I thought fields like url, hostname, port should be searchable (still not implemented yet though🤣) in the monitor table. If I made them as a json, the search performance will be very bad.

Anyway, will come back later, I put this in the 2.1.0 milestone.

image

louislam avatar Nov 01 '23 00:11 louislam

Interested in this :)

dn4hc avatar Jan 26 '24 02:01 dn4hc

@dn4hc

Please refrain from posting +1 / requests for update-things on PRs, as this makes reviewing it even harder. PRs are for noting problems and working on them, while for updates, there is a subscribe to this PR-button at the top. We use 👍🏻 on PRs to prioritise work.

CommanderStorm avatar Jan 26 '24 11:01 CommanderStorm