Monitors logic appears throws error when config.apiKey is present
tmadej opened this issue · 3 comments
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.");
+ }
}
}
}
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
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!