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
feat: Datadog type monitor #4616
base: master
Are you sure you want to change the base?
Conversation
* Run the monitoring check on the given monitor | ||
* @param {object} monitor - The monitor object associated with the check. | ||
* @param {object} heartbeat - The heartbeat object to update. | ||
* @returns {Promise<void>} | ||
* @throws Will throw an error if the API call found any. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be simplified
* Run the monitoring check on the given monitor | |
* @param {object} monitor - The monitor object associated with the check. | |
* @param {object} heartbeat - The heartbeat object to update. | |
* @returns {Promise<void>} | |
* @throws Will throw an error if the API call found any. | |
* @inheritdoc |
/** | ||
* A DataDog class extends the MonitorType. | ||
* It will query DataDog api to get datadog monitor state and sync the monitor status. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does not add context => lets not add this
/** | |
* A DataDog class extends the MonitorType. | |
* It will query DataDog api to get datadog monitor state and sync the monitor status. | |
*/ |
|
||
name = "datadog"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: formatting
name = "datadog"; | |
name = "datadog"; | |
* @returns {Promise<void>} | ||
* @throws Will throw an error if the API call found any. | ||
*/ | ||
async check(monitor, heartbeat) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
async check(monitor, heartbeat) { | |
async check(monitor, heartbeat, _server) { |
apiKeyAuth: monitor.datadog_api_key, | ||
appKeyAuth: monitor.datadog_app_key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use this casing consistently over the rest of the monitor. No need for this monitor to be different
I have not marked up the rest of the occurences to not bloat this review^, but please fix them independently ^^
apiKeyAuth: monitor.datadog_api_key, | |
appKeyAuth: monitor.datadog_app_key | |
apiKeyAuth: monitor.datadogApiKey, | |
appKeyAuth: monitor.datadogAppKey, |
<!-- Datadog Only--> | ||
<template v-if="monitor.type === 'datadog'"> | ||
<div class="my-3"> | ||
<label for="datadog_site" class="form-label">DataDog Site (Site parameter)</label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add labels in a translatable fashion.
Currently, our kickass team of translators would not be able to translate this string
- add a translation key via
{{ t("KEY_HERE") }}
- add the translation to
src/locales/en.json
(important, as otherwise this is not avaliable for translation)
@@ -161,6 +161,9 @@ class Monitor extends BeanModel { | |||
kafkaProducerMessage: this.kafkaProducerMessage, | |||
screenshot, | |||
remote_browser: this.remote_browser, | |||
datadog_site: this.datadog_site, | |||
datadog_monitor_id: this.datadog_monitor_id, | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return DOWN; | ||
} | ||
}); | ||
heartbeat.status = status; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is not having a heartbeat.msg
intentional?
does datadog allow us to have a heartbeat.ping
value?
// Add new columns monitor.datadog* | ||
return knex.schema | ||
.alterTable("monitor", function (table) { | ||
table.string("datadog_site", 255); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the existing hostname field, as this acts similar enough.
table.string("datadog_api_key", 255); | ||
table.string("datadog_app_key", 255); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is one of these a password? If yes this could use the existing Rename radiusPassword
(has to be renamed to monitor.password for general use) field
@chakflying @louislam @Saibamen @Zaid-maker Currently, we offer pretty basic monitors (grpc, http, dns, docker, databases). Similar as in #4619, I think that limiting our scope a bit is nessesary in this area. What are other peoples oppinions? |
Yes I agree with you! We need to limit our current scope with limit amount of monitors, also maybe in feature we can add these feature as a Premium feature or smh like that. Even tho it's a paid service I don't think it's good to add this |
These 3rd party monitors are hard to maintain, because only our users are checking changelogs for APIs etc. The best solution will be to have plugins system. |
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]:
Description
Hello @louislam !
Some context:
We use DataDog to monitor our stack/services, and it does not offer status page product, we take a look at the open sources projects that could fit. We found uptime-kuma 💘 the one we like the most and could fit better.
We even work already into a POC, but we need to work around with a way to get DataDog monitor states into uptime-kuma, creating push monitors and an extra service that should be the one that gather datadog and make the request to the push monitor endpoint.
Instead of having this kind of workaround, I thought that could be fun and worth to contribute to the project! (Alert! : I'm not a developer, so please bear with me and my small code changes).
With this feature, anyone that is using DataDog will be able to relay on uptime-kuma out of the box to create first class status pages 😎 .
New DataDog type monitor
This feature will allow to sync DataDog monitor state with uptime-kuma monitor.
As this is just initial base code changes, there are some things still need to be added:
Type of change
Checklist
Screenshots