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

Hapi Instrumentation does not Capture Request Body #1905

Closed
astorm opened this issue Dec 3, 2020 · 2 comments · Fixed by #2618
Closed

Hapi Instrumentation does not Capture Request Body #1905

astorm opened this issue Dec 3, 2020 · 2 comments · Fixed by #2618
Assignees
Labels
8.3-candidate agent-nodejs Make available for APM Agents project planning. kibana

Comments

@astorm
Copy link
Contributor

astorm commented Dec 3, 2020

Unsure if this is an enhancement request or a bug report, but it looks like our HAPI instrumentation does not collect the request bodies as captured by the built-in HAPI payload handling. HAPI populates request.payload.name with these values, but we read our request bodies in from the native node IncomeingMessage/Request object (i.e. req.body)

You can observe this behavior by running the following small sample program

const Hapi = require('@hapi/hapi')

const init = async () => {
  const server = Hapi.server({
    port: 3000,
    host: '0.0.0.0'
  })

  server.route({
    method: 'POST',
    path: '/test',
    handler: (request, h) => {
      // request.response.header({foo:"bar"})
      console.log(request.payload)
      const response = h.response('Hello World!')
      for (const [header, value] of Object.entries({ 'X-foo': 'bar' })) {
        response.header(header, value)
      }
      return response
    }
  })
  await server.start()
  console.log(server.info)
  console.log('Server running on %s', server.info.uri)
}

and posting some form variables.

$ curl -i http://localhost:3000 -d foo=bar

HAPI captures the request body, but if you examine the generated transactions, no request body is captured.

Ideally our HAPI instrumentation should match our express instrumentation's functionality and capture these values.

@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Dec 3, 2020
@astorm astorm mentioned this issue Dec 4, 2020
9 tasks
@trentm trentm added the kibana label Mar 17, 2022
@mshustov
Copy link

we read our request bodies in from the native node IncomeingMessage/Request object (i.e. req.body)

Hapi doesn't seem to extend IncomingRequest object with a parsed payload object. The request body is parsed in a route lifecycle
and assigned to the Hapi request object https://github.com/hapijs/hapi/blob/master/lib/route.js#L430-L434
https://github.com/hapijs/hapi/blob/master/lib/route.js#L289-L291
Can we store request payload in Hapi instrumentation logic to access it later when finalizing a transaction?

payload.context.request = parsers.getContextFromRequest(this.req, conf, 'transactions')

@trentm trentm self-assigned this Mar 22, 2022
@trentm
Copy link
Member

trentm commented Mar 22, 2022

A slight digression on request body capture. When I started looking at this I noticed that the Hapi Request object provides a 'peek' event that enables on to "tap" into the raw chunks of request data. (There is a 'peek' event documented on the Hapi Response object, https://hapi.dev/api/#-responseevents, so I'm not sure if this request.events.on('peek', ...) counts as undocumented, an oversight or a bug in the Hapi reference docs.)

  function onPreAuth (request, reply) {
...
    request.events.on('peek', function (chunk, encoding) {
      console.warn('XXX request "peek" event', chunk, encoding)
    })
...

Theoretically this could be used to capture the raw HTTP request body. However, this isn't the general way we capture the body for other frameworks (e.g. Express).

Instead the other existing instrumentation lets the framework/app parse the request body, and the APM instrumentation looks for the parsed body set to req.json || req.body || req.payload and uses that, if available. To store it on the APM transaction context, that object is then re-serialized as JSON. There are pros and cons here, but this is what we are doing elsewhere now. An important "Pro" of using the parsed-body is that we can apply sanitizeFieldNames to body fields in some cases for privacy reasons.

trentm added a commit that referenced this issue Mar 22, 2022
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
trentm added a commit that referenced this issue Mar 23, 2022
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.

The testing of a POST to a hapi server also showed that older
hapi versions are broken (hangs or crashes) with node >= 16,
so this change adjusts the version combinations against which
we test.

Closes: #1905
@trentm trentm mentioned this issue Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.3-candidate agent-nodejs Make available for APM Agents project planning. kibana
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants