feat: Datadog type monitor #4616

seluard · 2024-03-24T19:27:41Z

⚠️⚠️⚠️ 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

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:

Tests
Translation in place
BUG: The creation works, but I missed something cause updating the monitor fields that I added is not working (Again bear with me 😞 )
Documentation about this new monitor type
Go through the checklist properly
datadog-api-cli dependency or use just http to keep uptime-kuma dependencies lean and clean.

Type of change

New feature (non-breaking change which adds functionality)
This change requires a documentation update

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

CommanderStorm · 2024-05-23T12:28:00Z

server/monitor-types/datadog.js

+     * 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.


can be simplified

Suggested change

* 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

CommanderStorm · 2024-05-23T12:28:32Z

server/monitor-types/datadog.js

+/**
+ * A DataDog class extends the MonitorType.
+ * It will query DataDog api to get datadog monitor state and sync the monitor status.
+ */


does not add context => lets not add this

Suggested change

/**

* A DataDog class extends the MonitorType.

* It will query DataDog api to get datadog monitor state and sync the monitor status.

*/

CommanderStorm · 2024-05-23T12:28:52Z

server/monitor-types/datadog.js

+
+    name = "datadog";


nit: formatting

Suggested change

name = "datadog";

name = "datadog";

CommanderStorm · 2024-05-23T12:29:17Z

server/monitor-types/datadog.js

+     * @returns {Promise<void>}
+     * @throws Will throw an error if the API call found any.
+     */
+    async check(monitor, heartbeat) {


Suggested change

async check(monitor, heartbeat) {

async check(monitor, heartbeat, _server) {

CommanderStorm · 2024-05-23T12:30:57Z

server/monitor-types/datadog.js

+                    apiKeyAuth: monitor.datadog_api_key,
+                    appKeyAuth: monitor.datadog_app_key


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 ^^

Suggested change

apiKeyAuth: monitor.datadog_api_key,

appKeyAuth: monitor.datadog_app_key

apiKeyAuth: monitor.datadogApiKey,

appKeyAuth: monitor.datadogAppKey,

CommanderStorm · 2024-05-23T12:40:17Z

src/pages/EditMonitor.vue

+                            <!-- Datadog Only-->
+                            <template v-if="monitor.type === 'datadog'">
+                                <div class="my-3">
+                                    <label for="datadog_site" class="form-label">DataDog Site (Site parameter)</label>


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)

CommanderStorm · 2024-05-23T12:51:39Z

server/model/monitor.js

@@ -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,
+


Suggested change

CommanderStorm · 2024-05-23T12:55:26Z

server/monitor-types/datadog.js

+                    return DOWN;
+                }
+            });
+            heartbeat.status = status;


is not having a heartbeat.msg intentional?
does datadog allow us to have a heartbeat.ping value?

CommanderStorm · 2024-05-23T13:00:37Z

db/knex_migrations/2024-03-24-0000-add-datadog-monitor-type.js

+    // Add new columns monitor.datadog*
+    return knex.schema
+        .alterTable("monitor", function (table) {
+            table.string("datadog_site", 255);


please use the existing hostname field, as this acts similar enough.

CommanderStorm · 2024-05-23T13:02:16Z

db/knex_migrations/2024-03-24-0000-add-datadog-monitor-type.js

+            table.string("datadog_api_key", 255);
+            table.string("datadog_app_key", 255);


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

CommanderStorm · 2024-05-23T13:58:05Z

@chakflying @louislam @Saibamen @Zaid-maker

Currently, we offer pretty basic monitors (grpc, http, dns, docker, databases).
I think this monitor does not fit into the line of moniotors we currently offer.

Similar as in #4619, I think that limiting our scope a bit is nessesary in this area.
I think including such monitors via #1117 instead might be a better call..

What are other peoples oppinions?

Zaid-maker · 2024-05-23T14:03:11Z

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

seluard added 4 commits March 24, 2024 19:50

feat: DataDog monitor type

9e01917

feat: DataDog monitor - base for proposal

1666659

fix: lint

59511dd

fix: lint 2

aaa54b8

CommanderStorm added area:monitor Everything related to monitors type:new proposing to add a new monitor labels Apr 3, 2024

CommanderStorm added the needs:review this PR needs a review by maintainers or other community members label May 19, 2024

github-actions bot added needs:resolve-merge-conflict A merge-conflict needs to be addressed before reviewing makes sense again and removed needs:resolve-merge-conflict A merge-conflict needs to be addressed before reviewing makes sense again labels May 19, 2024

github-actions bot removed the needs:resolve-merge-conflict A merge-conflict needs to be addressed before reviewing makes sense again label May 23, 2024

Merge branch 'master' into datadog-type-monitor

2109b9b

CommanderStorm requested changes May 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Datadog type monitor #4616

feat: Datadog type monitor #4616

seluard commented Mar 24, 2024 •

edited

CommanderStorm May 23, 2024

CommanderStorm May 23, 2024

CommanderStorm May 23, 2024

CommanderStorm May 23, 2024

CommanderStorm May 23, 2024

CommanderStorm May 23, 2024

CommanderStorm May 23, 2024

CommanderStorm May 23, 2024

CommanderStorm May 23, 2024

CommanderStorm May 23, 2024

CommanderStorm commented May 23, 2024

Zaid-maker commented May 23, 2024 •

edited

	async check(monitor, heartbeat) {
	async check(monitor, heartbeat, _server) {

		apiKeyAuth: monitor.datadog_api_key,
		appKeyAuth: monitor.datadog_app_key

		table.string("datadog_api_key", 255);
		table.string("datadog_app_key", 255);

feat: Datadog type monitor #4616

Are you sure you want to change the base?

feat: Datadog type monitor #4616

Conversation

seluard commented Mar 24, 2024 • edited

Description

New DataDog type monitor

Type of change

Checklist

Screenshots

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CommanderStorm commented May 23, 2024

Zaid-maker commented May 23, 2024 • edited

seluard commented Mar 24, 2024 •

edited

Zaid-maker commented May 23, 2024 •

edited