elastic / elastic/apm-agent-nodejs

instrumentation of a non-async Lambda handler that does not call `callback()` does not end the transaction

Open
#3,034 0 comments 0 reactions 0 assignees View on GitHub
agent-nodejs
Dominant language
JavaScript
Stars
594
Forks
244
Avg merge
1d 8h
Merged PRs (30d)
16

Description

If you have a [non-async Lambda handler function](https://docs.aws.amazon.com/lambda/latest/dg/nodejs-handler.html#nodejs-handler-sync), e.g.:

```
exports.handler = function (event, context, callback) {
// Handler code does *not* call callback().
setTimeout(function () {
console.log("some output from user code at the end of the handler");
}, 1000);
}
```

then the agent's current Lambda instrumentation does not notice the ending of the invocation. This means the transaction for the Lambda invocation is not ended and not sent.

The Lambda Runtime itself *does* notice the end of the invocation. For example, from the debug logs of a Lambda function where this was noticed (h/t @AlexanderWert):

```
...
2022-11-18T12:26:28.448Z d6ddb353-fabb-40b9-9b88-9a90ec4c299b INFO some output from user code at the end of the handler
... 100ms pause ...
{"log.level":"info","@timestamp":"2022-11-18T12:26:28.549Z",...,"message":"Received runtimeDone event for this function invocation",...}
...
```

While it is a bit of a code smell to have a non-async handler that doesn't call callback -- one is relying on the Lambda Runtime to wait for all unref'd async tasks to be complete -- our Lambda instrumentation should catch this.

### speculation

I haven't dug into this, but here is some speculation. I think the Lambda Runtime is likely using the `beforeExit` node.js event to detect when the invocation is complete. We could do the same. The logic for ending the invocation then would be:

- `callback(err, result)` provides a result, that's all.
- if `context.callbackWaitsForEmptyEventLoop = false` is set, then `callback()` *does* mean the invocation is complete
- otherwise, use `beforeExit` to decide when the invocation is done

I think a `process.on('beforeExit', ...)` in "lib/lambda.js" could make a fix for this fairly localized.

Related issue: https://github.com/elastic/apm-agent-nodejs/issues/2643

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.