elastic / apm-agent-nodejs

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

captureBody do not work #407

Closed cuiweiqiang closed 6 years ago

cuiweiqiang commented 6 years ago

This is my package.json dependencies:

"dependencies": {
    "elastic-apm-node": "^1.7.1",
    "kcors": "^2.2.1",
    "koa": "^2.5.1",
    "koa-body": "^4.0.3",
    "koa-bodyparser": "^4.2.1",
    "koa-convert": "^1.2.0",
    "koa-generic-session": "^2.0.1",
    "koa-json": "^2.0.2",
    "koa-logger": "^3.2.0",
    "koa-router": "^7.4.0",
    "mongoose": "^5.1.6",
    "require-directory": "^2.1.1"
  }

and this is my start params:

var apm = require('elastic-apm-node').start({
    // Set required service name (allowed characters: a-z, A-Z, 0-9, -, _, and space)
    serviceName: 'apm-test-node',
    // Use if APM Server requires a token
    // secretToken: '',
    // Set custom APM Server URL (default: http://localhost:8200)
    serverUrl: 'http://localhost:8200',
    filterHttpHeaders:false,
    instrument: 'true',
    captureBody: 'all',
    flushInterval: 0
});

But when do a request there is no body information in APM UI:

nobody

even spans:

nospan
watson commented 6 years ago

Oh, I can see our docs are not clear about how this works at all. Sorry about that.

We currently don't capture the body unless it's available on the http.IncomingMessage object. This is normally made available by frameworks like Express, but there's no official standard for this and Node.js doesn't do this natively.

Currently we look for the body on either req.json, req.body or req.payload.

If you put your body onto one of these 3 properties, the Node.js agent will pick it up. Would that be an acceptable solution for you?

I've opened an issue to make sure we update the docs to be more clear about this: #408

cuiweiqiang commented 6 years ago

Hmmmm, I use ugly code inject to the http-shared.js. The package raw-body provide a method which to parse raw body from IncomingMessage. Because of my project provide restful api, so it pass params by json, the hacked code work well. But this is very ungraced solution. Is there any way to delay execute hook function or hook ctx in koa ?

watson commented 6 years ago

If you use raw-body as a middleware in Koa, do you store the body on a property on the request object and in that case which one? Is it req.text as shown in their example code?

I'm not sure what you mean by "Is there any way to delay execute hook function or hook ctx in koa ?" - which hook are you referring to?

cuiweiqiang commented 6 years ago

Thanks for reply. I hacked this file : /node_modules/elastic-apm-node/lib/instrumentation/http-shared.js, and add this code in line 22:

1529654923129

When apm agent start, the instrumentation will start too, I find a piece of code in Instrumentation.prototype.start method. In this method, the _patchModule method is executed. My project use koa-bodyparser to parse the request body, then ctx object is be injectd like ctx.request.body. Can we read body from here?

watson commented 6 years ago

Thanks for sharing the details 😃

Yes, I think it would make sense for us to look into koa-bodyparser and see if we can add support for ti directly.

That being said, I think there's a non-hacky way for you to expose the body to the Elastic APM agent. How about adding this middleware after the bodyParser:

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

If I'm not mistaken you'll always have access to the raw request via ctx.req, and the koa-bodyparser middelware will store the parsed body on ctx.request.body, so if you just copy it over like this, I think it should have the same effect.

This is still not ideal, but better than manually patching http-shared.js I think.

watson commented 6 years ago

FYI: I've created a new issue to look into native support of koa-bodyparser: #409

cuiweiqiang commented 6 years ago

@watson Good Job 👍

watson commented 6 years ago

I'll close this issue for now as the docs have been updated. Feel free to subscribe to #409 if you want to stay up to date on the specific support for koa-bodyparser 😃