elastic / apm-agent-nodejs

https://www.elastic.co/guide/en/apm/agent/nodejs/current/index.html
BSD 2-Clause "Simplified" License
582 stars 225 forks source link

Automatically get body from koa-bodyparser #409

Open watson opened 6 years ago

watson commented 6 years ago

If using the popular koa-bodyparser module, our captureBody config option currently doesn't have any effect. It would be nice if the agent natively supported extracting the body from this module. Either by getting it from ctx.request.body where it's stored by the koa-bodyparser, or by patching koa-bodyparser directly.

Fixes: elastic/apm-agent-nodejs#407

watson commented 6 years ago

We could instead consider getting the body directly from the request stream using https://github.com/watson/stream-observer

Qard commented 6 years ago

If we try to collect the request body with stream-observer we should make sure we have an escape condition decided for if the body gets too big.

watson commented 6 years ago

Yes. We already have an upper limit on 2048 bytes after which the body is truncated. We should respect that and just stop listening.

The stream-observer module returns a function which you can call at any time to stop listening for data.

Qard commented 6 years ago

Ah, cool. Sounds like it might be a good option then. :)

ilionic commented 5 years ago

Not sure it's related - if using koa-bodyparser getting GET unknown route

Qard commented 5 years ago

koa-bodyparser wouldn't cause that, but you'll currently only get routing information if you use koa-router. Using koa directly, or using other routing modules will not record a different transaction name. You can use agent.setTransactionName('custom name') if you want to provide a name yourself.

imchao9 commented 5 years ago

@watson does this problem fix? If not fix, how to make the koa-bodyparser work? I try the

  captureBody: 'all',

I can not get the body information now.

watson commented 5 years ago

No, given the way koa-bodyparser work, we need to either patch it somehow or we need to hook in at a lower level. The way captureBody works currently is by expecting that the body is set as a property on the request object, either with the name req.body, req.json or req.payload. The koa-bodyparser module does not do that.

imchao9 commented 5 years ago

No, given the way koa-bodyparser work, we need to either patch it somehow or we need to hook in at a lower level. The way captureBody works currently is by expecting that the body is set as a property on the request object, either with the name req.body, req.json or req.payload. The koa-bodyparser module does not do that.

And do we have any plan to make it work?

watson commented 5 years ago

Yes, that's why I created this issue. But it's not yet scheduled when it will be implemented.

To help us prioritize, could you share a little information on what your use-case for capturing the body is? Is it during the development phase of the application, or in a production environment? How large are the avg. bodies you want to capture, what's their content-type etc.

imchao9 commented 5 years ago

Here is a demo

require('elastic-apm-node').start({
  // Override service name from package.json
  // Allowed characters: a-z, A-Z, 0-9, -, _, and space
  serviceName: 'helloworld',

  // Use if APM Server requires a token
  secretToken: '',

  // Set custom APM Server URL (default: http://localhost:8200)
  serverUrl: 'http://localhost:8200',
  logLevel: 'trace',
  asyncHooks: false,
  captureHeaders: true,
  captureBody: 'all',
  errorOnAbortedRequests: true,
  abortedErrorThreshold: true,
  transactionSampleRate: '1.0',

});

const Koa = require('koa');
const Router = require('koa-router');
const request = require('superagent');

const app = new Koa();
const router = new Router();

router.get('/tt', async (ctx, next) => {
  const response = await request.get('https://petstore.swagger.io/v2/store/inventory');
  ctx.body = response.body;
})

app.use(router.routes())
  .use(router.allowedMethods());

app.use(async ctx => {
  ctx.req.body = ctx.request.body
})

app.listen(3000);
Qard commented 5 years ago

@watson We should probably just capture the body from the raw-body module. As far as I know, every body parser uses it.

sibelius commented 3 years ago

so no request body neither response body are recorded into apm?

sibelius commented 1 year ago

where is the best place to add a test to this ?