feat: add 'captureBody' support for Hapi #2618
Conversation
The body of incoming requests to an HTTP server using the Hapi framework will be recorded in relevant APM transaction and error objects depending on the `captureBody` config var setting. Closes: #1905
github-actions
bot
added
the
agent-nodejs
|
@mshustov You know Hapi much better than I. Is |
I have a test hang with node v16 and v17 to solve first. |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
…ated legacy hapi) and node >=16 *hangs*. No need to support this.
|
run module tests for hapi,@hapi/hapi |
| @@ -60,13 +61,16 @@ module.exports = function (hapi, agent, { version, enabled }) { | |||
| if (typeof server.ext === 'function') { | |||
| server.ext('onPreAuth', onPreAuth) | |||
| server.ext('onPreResponse', onPreResponse) | |||
| if (agent._conf.captureBody !== 'off') { | |||
| server.ext('onPostAuth', onPostAuth) | |||
There was a problem hiding this comment.
Is onPostAuth the best place in the Hapi Request lifecycle (https://hapi.dev/api/?v=20.2.1#request-lifecycle) to capture the parsed request.payload?
Yeah, it seems that payload parsing is called before https://github.com/hapijs/hapi/blob/master/lib/route.js#L252-L270
Also, according to Hapi.js in action book, 6. Read/Parse payload step happens before onPostAuth callback.
|
@trentm LGTM. Tested with Kibana, it works fine. 👌 |
@mshustov Good to hear. |
…hen not meeting those criteria
|
The TAV run failed https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-nodejs%2Fapm-agent-nodejs-mbp/detail/PR-2618/3/pipeline This failure happens even if I disable the added I.e. to use Handled in 3a787b4 |
|
run module tests for hapi,@hapi/hapi |
There was a problem hiding this comment.
Code looks like it does the thing. I also spun up a quick HAPI test app with the following route
server.route({
method: 'POST',
path: '/test-post',
handler: (request, h) => {
console.log(request.payload)
return 'Hello World!' + "\n" + JSON.stringify(request.payload);
}
});
and posted via curl localhost:3000/test-post -d 'foo=bar', and the request body made it through to the http.request.body.original field in the UI
Approving
| function onPostAuth (request, reply) { | ||
| if (request.payload && request.raw && request.raw.req) { | ||
| // Save the parsed req body to be picked up by getContextFromRequest(). | ||
| request.raw.req.payload = request.payload |
There was a problem hiding this comment.
Looks like our parse treats .payload, .json, and .body the same so .payload is an appropriate place for this
apm-agent-nodejs/lib/parsers.js
Line 40 in 301744a
| @@ -15,10 +15,11 @@ module.exports = function (hapi, agent, { version, enabled }) { | |||
| agent.logger.debug('hapi version %s not supported - aborting...', version) | |||
| return hapi | |||
| } | |||
| const isHapiGte17 = semver.satisfies(version, '>=17') | |||
There was a problem hiding this comment.
👍 for caching/memoizing/whateveritscalledthesedays the value of the call to semver.satisfies
@astorm Whoa what is going on with the Oh there is a trailing single-quote on the "bar" as well. So I'm guessing that curl call included the literal single-quotes for the form-encoded POST body? |
|
Good question. This looks like a weird side effect of shell escaping in a bash script I used for testing and a type 1 thinking copy-pasta. I call the script like this and does not produce those extra quotes. TL;DR; -- all clear :) |
The body of incoming requests to an HTTP server using the Hapi
framework will be recorded in relevant APM transaction and error
objects depending on the
captureBodyconfig var setting.Closes: #1905
Checklist