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

Add support for custom mongodb commands #4445

Merged
merged 6 commits into from May 19, 2024

Conversation

SebastianLng
Copy link
Contributor

@SebastianLng SebastianLng commented Feb 1, 2024

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

Extends the MongoDB monitor to allow running custom MongoDB commands and optionally comparing the value against an expected value.

Besides being able to check database content, this is useful to further check the health of MongoDB, especially clusters. For example, the hello command offers various health information which can be queried and checked with this change.

From a UI standpoint, this adds a command input, a json query input and an expected value input. All are basically the same as comparable HTTP(s) Json query inputs and other DB monitors inputs.
All new fields are optional, and the default command is still the ping command that was used before. All commands are checked for the ok result, so the behaviour without using the new inputs is the same as before.

If the Json Query is set, it'll be applied to the query result. If json query result is undefined, the monitor will produce a failure. The Json query without expected value is basically checking if the value exists. If the expected value is set, it will be compared to the result.

Type of change

  • User interface (UI)
  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
  • My changes generates no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots

image

Testing

Test MongoDB
docker run --name test-mongodb -p 27017:27017 -e MONGO_INITDB_ROOT_USERNAME=admin -e MONGO_INITDB_ROOT_PASSWORD=admin mongo

Connection string: mongodb://admin:admin@localhost:27017
Test query: { "hello": 1 }

Here are the tests I ran:
mongodb-monitor-tests.json

Copy link
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

Foremost, I think this sort of conflicts with #3919
@chakflying I think this is your area ⇒ I would rather not interfere:
what do you think about this?
Can this be integrated into your plan, or what level of refactoring need to be done to accommodate it?

Given the level of change (basically a rewrite) that this PR adds to the mongodb monitor, I think some additional changes are necessary.

@chakflying
Copy link
Collaborator

We discussed this a bit in #3857. Personally, I think the code change here is relatively small, and is good preparation if we wanted to support this generically in the future. I'm not against merging this if people wanted this feature.

@SebastianLng
Copy link
Contributor Author

Thank you for the response, I migrated to the new monitor type and added some info for testing.

CommanderStorm

This comment was marked as resolved.

Copy link
Collaborator

@chakflying chakflying left a comment

Choose a reason for hiding this comment

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

Tested seems to work okay.

Copy link
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

From my side there are no roadblocks.
Works as intended.

src/lang/en.json Outdated Show resolved Hide resolved
src/pages/EditMonitor.vue Outdated Show resolved Hide resolved
@torstenbrink

This comment was marked as spam.

@CommanderStorm

This comment was marked as resolved.

@CommanderStorm CommanderStorm added the area:monitor Everything related to monitors label Apr 3, 2024
@CommanderStorm
Copy link
Collaborator

I don't know if this can be better/automatically tested via #4451

Honestly, such testcases should be build in #4451 first => we can expand this later

Thanks for the new feature!

All changes in this PR work as intended
⇒ merging with junior maintainer approval

@CommanderStorm CommanderStorm merged commit a3ac954 into louislam:master May 19, 2024
17 checks passed
@CommanderStorm CommanderStorm added this to the 2.0.0 milestone May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:monitor Everything related to monitors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants