elastic / elastic/apm-agent-nodejs

extend `captureAttributes` option to `agent.captureError()` to support an array of attribute names to capture

Open
#2,115 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

### current state

Currently `agent.captureError(...)` will include all simple properties ("simple" being basic types: bools, strings, numbers, Dates; see [`attributesFromErr`](https://github.com/elastic/apm-agent-nodejs/blob/fc0a7608026f1cbe6393825a5802976f634774f3/lib/errors.js#L64-L97)) of the given Error object as `error.exception.attributes`. For example:

```
{
"error": {
...
"exception": {
"message": "null",
"type": "NotFound",
"handled": true,
"code": "NotFound",
"attributes": {
"time": "2021-06-16T19:50:09.694Z",
"requestId": "XZY89K3EJ0MJPBMR",
"extendedRequestId": "KWx44n6oO1N9Lfp7qr8SKvqfCFGirnWpiEcd60+mIl+sxUHGTbCjJACguZt+1raU/8ZfRkLbMjk=",
"statusCode": 404,
"retryable": false,
"retryDelay": 97.76669227033857
},
...
```

In #1912 a `captureAttributes` boolean option was added to turn off this capture. The specific use case was the `@elastic/elasticsearch` instrumentation where the Error object can include the (possibly large) response body.

### the problem

When setting `captureAttributes: false` the only alternative for adding *some* Error object properties is via `error.context.custom`. However that's unsatisfying because:

1. those properties are no longer in the same place in the data model (`context.custom` instead of `exception.attributes`); and
2. the serialization falls to the calling code, which means that one has to special case Date's oneself otherwise naively one gets `{}` for a passed in `Date` instance, e.g.:

```
{
"error": {
...
"context": {
...
"custom": {
"time": {}, // <--- HERE: passing in the same `err.time`
"requestId": "XZY89K3EJ0MJPBMR"
}
},
"exception": {
...
"attributes": {
"time": "2021-06-16T19:50:09.694Z",
"requestId": "XZY89K3EJ0MJPBMR",
...
},
```

### proposal

I propose extending `captureAttributes` to mean the following:

- `true` (the default) means capture all simple attributes (no change)
- `false` means capture no exception.attributes (no change)
- An array of attribute names, e.g. `["time", "requestId"]`, means capture each of the named attributes on the Error object if they are defined and are simple. (This is the new addition.)

This new facility would allow instrumentations in the agent to be explicit about what attributes to capture, without having to worry about random (possibly large, possibly private) properties being added by new versions of libraries.

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.