Monitors logic appears throws error when config.apiKey is present

[tmadej]

When trying to deploy with monitors enabled, config.apiKey present & test mode off, I found that this plugin was throwing the below error:

Environment: darwin, node 18.17.1, framework 3.34.0 (local) 3.33.0v (global), plugin 6.2.3, SDK 4.3.2
Docs:        docs.serverless.com
Support:     forum.serverless.com
Bugs:        github.com/serverless/serverless/issues

Error:
Error: When `monitors` is enabled, `DATADOG_API_KEY` and `DATADOG_APP_KEY` environment variables must be set.
    at validateConfiguration (node_modules/serverless-plugin-datadog/dist/src/index.js:505:19)
    at ServerlessPlugin.<anonymous> (node_modules/serverless-plugin-datadog/dist/src/index.js:114:13)
    at Generator.next (<anonymous>)
    at node_modules/serverless-plugin-datadog/dist/src/index.js:38:71
    at new Promise (<anonymous>)
    at __awaiter (node_modules/serverless-plugin-datadog/dist/src/index.js:34:12)
    at ServerlessPlugin.beforePackageFunction (node_modules/serverless-plugin-datadog/dist/src/index.js:105:16)
    at PluginManager.runHooks (node_modules/serverless/lib/classes/plugin-manager.js:530:15)
    at async PluginManager.invoke (node_modules/serverless/lib/classes/plugin-manager.js:563:9)
    at async PluginManager.spawn (node_modules/serverless/lib/classes/plugin-manager.js:585:5)
    at async before:deploy:deploy (node_modules/serverless/lib/plugins/deploy.js:40:11)
    at async PluginManager.runHooks (node_modules/serverless/lib/classes/plugin-manager.js:530:9)
    at async PluginManager.invoke (node_modules/serverless/lib/classes/plugin-manager.js:563:9)
    at async PluginManager.run (node_modules/serverless/lib/classes/plugin-manager.js:604:7)
    at async Serverless.run (node_modules/serverless/lib/serverless.js:179:5)
    at async node_modules/serverless/scripts/serverless.js:834:9

When reviewing the logic for this, I found that the logic is slightly off. I cleaned it up in the below patch and it seems to be working as expected now:

diff --git a/node_modules/serverless-plugin-datadog/dist/src/index.js b/node_modules/serverless-plugin-datadog/dist/src/index.js
index e795e71..3d29348 100644
--- a/node_modules/serverless-plugin-datadog/dist/src/index.js
+++ b/node_modules/serverless-plugin-datadog/dist/src/index.js
@@ -498,11 +498,15 @@ function validateConfiguration(config) {
         }
     }
     if (config.monitors) {
-        if ((process.env.DATADOG_API_KEY === undefined || process.env.DATADOG_APP_KEY === undefined) &&
-            // Support deprecated monitorsApiKey and monitorsAppKey
-            (config.apiKey === undefined || config.appKey === undefined) &&
-            (config.testingMode === false || config.integrationTesting === false)) {
-            throw new Error("When `monitors` is enabled, `DATADOG_API_KEY` and `DATADOG_APP_KEY` environment variables must be set.");
+        if (config.testingMode === false && config.integrationTesting === false) {
+            if (process.env.DATADOG_API_KEY === undefined && 
+                process.env.DATADOG_APP_KEY === undefined &&
+                // Support deprecated monitorsApiKey and monitorsAppKey
+                config.apiKey === undefined && 
+                config.appKey === undefined) {
+                    
+                throw new Error("When `monitors` is enabled, `DATADOG_API_KEY` and `DATADOG_APP_KEY` environment variables must be set.");
+            }
         }
     }
 }

[sfirrin]

Hi @tmadej, thanks a lot for opening this and testing out the patch

IIUC the config.testingMode === false && config.integrationTesting === false fixes the issue but I think the nested conditional needs a slight tweak to still error if only one of the API key or the app key is present. We would welcome a PR or we can work on one and update you here

[duncanista]

Hey @tmadej,

I'm revisiting this, but there has been a couple releases since this issue was created. Is the issue persisting to this day? I'm not sure I understand what might be the issue. Happy to raise a PR to fix the problem. Could you share a reproduction case? Thanks!

[duncanista]

Hey @tmadej,

We haven't heard from you in a while – I'm gonna close this as we haven't found a reproduction case for this! Please upgrade to the latest version!

Feel free to re-open if you have any questions, or issue persist.