cloudwatch_lambda.js uses old, deprecated context object

[blimmer]

The Problem

In the current cloudwatch_lambda.js (pinned version link), the lambda is being marked as complete by calling context.succeed:

sumologic-aws-lambda/cloudwatchlogs/cloudwatchlogs_lambda.js

Line 96 in 22a1dcb

context.succeed();

However, this is very old syntax. It was deprecated with the 0.11.x runtime and backfilled (poorly) into the new runtimes.

Since the docs now recommend running Node 8.10, this function should be rewritten using the more understandable async/await syntax. Here are the docs stating that it's available in the 8.10 runtime.

Rewriting the function to use async await will transition the runtime to use the explicit callback parameter, marked by when the function resolves the promise returned by the exports.handler method.

How I Discovered This

Because we're defining this lambda in our Infrastructure-as-Code repository (using Terraform), we encrypt the SUMO_ENDPOINT URL since we don't want anyone other than the lambda to be able to see it (a malicious actor could POST logs to the endpoint and run up our bill).

To do this, I made the exports.handler asynchronous and decrypt the KMS key before parsing the message and sending it to sumo.

const AWS = require('aws-sdk');
async function getSumoUrl() {
  var kms = new AWS.KMS();
  var kmsDecryptParams = {
    CiphertextBlob: Buffer.from(process.env.ENCRYPTED_SUMO_ENDPOINT, 'base64')
  };
  var data = await kms.decrypt(kmsDecryptParams).promise();
  var decryptedSumoEndpoint = data.Plaintext.toString('utf-8');
 
  return decryptedSumoEndpoint;
}

// existing code unchanged

// NOTE that this is async now
exports.handler = async function (event, context) {
  SumoURL = await getSumoUrl();

  // existing code unchanged
}

However, this did not work because of the reliance on the old, unsupported context.succeed call. This would have been a whole lot easier if the rest of the function used the newer Node 8 async/await syntax to start with.

Conclusion / Suggestions

It seems like this lambda was written a long time ago, based on the APIs it's using. Node has come quite a long way and rewriting with Node 8 standards would make it more readable and extensible for users.

[himanshu219]

Yes agree and we use callback() in our new version of cloudwatch lambda function (dlq_processor). @duchatran do we want to maintain backward compatibility with old nodejs versions? Otherwise I can migrate this one to async await which will save us from callback hell.

[blimmer]

The setup docs state that you should select the Node 8 runtime when you set up the lambda:

https://github.com/SumoLogic/sumologic-aws-lambda/blob/22a1dcb518ee53b2cbf66e4d17ba010e76a22081/cloudwatchlogs/README.md#create-lambda-function

So I think it'd probably be fair to drop the backwards compatibility. Additionally, Node 4 has been deprecated on the lambda side so I'd assume folks will upgrade to the latest runtime when updating their lambdas.