Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filters are broken in some cases and crash app #152

Closed
bradennapier opened this issue Sep 24, 2019 · 1 comment
Closed

Filters are broken in some cases and crash app #152

bradennapier opened this issue Sep 24, 2019 · 1 comment

Comments

@bradennapier
Copy link

Describe the bug
A clear and concise description of what the bug is. Documentation indicates that if we do not return a payload a given request will not be sent to the apm server. This is true in some cases but not all.

The filter function is synchronous and should return the manipulated payload object. If a filter function doesn’t return any value or returns a falsy value, the remaining filter functions will not be called and the payload will not be sent to the APM Server.

There is also an issue with the sampled property not being the same everywhere which is confusing.

See #151 for more context

To Reproduce

In this case the app will crash any time that an error occurs within the request. Below is the general steps to make the error occur and details along the way

/**
 * Filter to only send sampled results to the APM Server
 */
apm.addFilter(payload => {
  console.log('\n--- Filter ---\n');
  console.log(JSON.stringify(payload, null, 2));

  if (typeof payload.sampled === 'boolean' && !payload.sampled) {
    /*
      CASE 1

      When this occurs the data result looks similar to below.  
      Returning here when not sampling works as expected and does
      not produce any errors.
    */
    return;
  }

  if (
    typeof payload.transaction?.sampled === 'boolean' &&
    !payload.transaction.sampled
  ) {
    /*
      CASE 2

      When an error occurs, the `transaction.sampled` is where
      the data is, but regardless, if this filter ever returns
      undefined it will cause the app to crash completely.
    */
    return;
  }

  return payload;
});

[CASE 1]: First Filter Works

It first provides the given payload. If I return undefined for this then it does not crash the application. However, this is a tiny data payload, my goal is to completely ignore anything that is not sampled.

{
  "id": "566594a22f516683",
  "trace_id": "fc9d048d55f21ffbebb1a2d44648b7d3",
  "name": "GET /example",
  "type": "request",
  "duration": 94.044,
  "timestamp": 1569358244043041,
  "result": "HTTP 2xx",
  "sampled": false,
  "sync": false,
  "span_count": {
    "started": 0
  }
}

[CASE 2] transaction.type of request Crashes

In this case, returning undefined crashes the app with the following stacktrace

{
  "exception": {
    "message": "Failed to validate request [1]",
    "type": "UnauthorizedError",
    "code": "Unauthorized",
    "attributes": {
      "jse_shortmsg": "Failed to validate request [1]",
      "message": "Failed to validate request [1]"
    },
    "handled": true
  },
  "id": "5b6003f09be05fc325400882513f357c",
  "parent_id": "d4106918fb0b26a6",
  "trace_id": "dbffee8786a51aace74e9efde9ab91ba",
  "timestamp": 1569358223470000,
    "custom": {},
  "transaction_id": "d4106918fb0b26a6",
  "transaction": {
    "type": "request",
    "sampled": false
  }
}
  TypeError: Cannot read property 'id' of undefined
  
  - agent.js:391 send
    [elastic-apm-node]/lib/agent.js:391:68
  
  - agent.js:383 prepareError
    [elastic-apm-node]/lib/agent.js:383:7
  
  - agent.js:299 
    [elastic-apm-node]/lib/agent.js:299:7
  
  - parsers.js:81 
    [elastic-apm-node]/lib/parsers.js:81:7
  
  - index.js:20 
    [after-all-results]/index.js:20:25
  
  - next_tick.js:61 process._tickCallback
    internal/process/next_tick.js:61:11
@bradennapier
Copy link
Author

Realized this is wrong repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant