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

Req element twitce with hapi. #91

Closed
indreek opened this issue Nov 11, 2019 · 10 comments · Fixed by #157 or Salesflare/hapi-pino#3
Closed

Req element twitce with hapi. #91

indreek opened this issue Nov 11, 2019 · 10 comments · Fixed by #157 or Salesflare/hapi-pino#3

Comments

@indreek
Copy link

indreek commented Nov 11, 2019

Any ideas why im getting req twice in logged json?

	"level": 30,
	"time": 1573488495601,
	"pid": 6012,
	"hostname": "nodejs",
	"req": {
		"id": "1573488495579:nodejs:6012:k2umf1vz:10000",
		"method": "get",
		"url": "http://192.168.0.128:4010/hello/world/",
		"headers": {
			"content-type": "application/json",
			"user-agent": "PostmanRuntime/7.19.0",
			"accept": "*/*",
			"cache-control": "no-cache",
			"postman-token": "ae33203b-1dc5-4f93-998f-f450ba43279f",
			"host": "192.168.0.128:4010",
			"accept-encoding": "gzip, deflate",
			"connection": "keep-alive"
		},
		"remoteAddress": "192.168.0.1",
		"remotePort": 60046
	},
	"req": {
		"id": "1573488495579:nodejs:6012:k2umf1vz:10000",
		"method": "get",
		"url": "http://192.168.0.128:4010/hello/world/",
		"headers": {
			"content-type": "application/json",
			"user-agent": "PostmanRuntime/7.19.0",
			"accept": "*/*",
			"cache-control": "no-cache",
			"postman-token": "ae33203b-1dc5-4f93-998f-f450ba43279f",
			"host": "192.168.0.128:4010",
			"accept-encoding": "gzip, deflate",
			"connection": "keep-alive"
		},
		"remoteAddress": "192.168.0.1",
		"remotePort": 60046
	},
	"res": {
		"statusCode": 200,
		"headers": {
			"content-type": "application/json; charset=utf-8",
			"vary": "origin,accept-encoding",
			"access-control-expose-headers": "WWW-Authenticate,Server-Authorization",
			"cache-control": "no-cache",
			"content-encoding": "gzip"
		}
	},
	"responseTime": 22,
	"msg": "request completed",
	"v": 1
}
@indreek
Copy link
Author

indreek commented Nov 11, 2019

'use strict'

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

async function start () {
    // Create a server with a host and port
    const server = Hapi.server({
        // host: 0.0.0.0,
        port: 3000
    })

    // Add the route
    server.route({
        method: 'GET',
        path: '/',
        handler: async function (request, h) {
            return 'hello world'
        }
    })

    await server.register({
        plugin: require('hapi-pino'),
        options: {}
    })

    // also as a decorated API
    server.logger().info('another way for accessing it')

    // and through Hapi standard logging system
    server.log(['subsystem'], 'third way for accessing it')

    await server.start()

    return server
}

start().catch((err) => {
    console.log(err)
    process.exit(1)
})

@indreek
Copy link
Author

indreek commented Nov 12, 2019

Probably it comes from Pino https://getpino.io/#/docs/child-loggers?id=duplicate-keys-caveat

@indreek indreek closed this as completed Nov 12, 2019
@indreek
Copy link
Author

indreek commented Nov 13, 2019

Hi

It was still bothering me.

const getChildBindings = options.getChildBindings ? options.getChildBindings : (request) => ({ req: request })
//....
const childBindings = getChildBindings(request)
request.logger = logger.child(childBindings) // You are adding child req: here, 

// Although you are 
// 1) adding it here when logRequestStart === true or 
if (logRequestStart) {  
  request.logger.info({
    req: request
  }, 'request start')
}

tryAddEvent(server, options, 'on', 'response', function (request) {
//...
request.logger.info(
  {
    payload: options.logPayload ? request.payload : undefined,
    tags: options.logRouteTags ? request.route.settings.tags : undefined,
    // note: pino doesnt support unsetting a key, so this next line
    // has the effect of setting it or "leaving it as it was" if it was already added via child bindings
    req: logRequestStart ? undefined : request, // 2) adding it here when logRequestStart === false
    res: request.raw.res,
    responseTime: (info.completed !== undefined ? info.completed : info.responded) - info.received
  },
  'request completed'
)

quick fix for this in my opinion is to change options:

getChildBindings: () => ({})

@aorinevo
Copy link

aorinevo commented Feb 5, 2020

We are seeing the same error as well.

@indreek, have you been able to identify the root cause?

@aorinevo
Copy link

aorinevo commented Feb 5, 2020

Well, looks like the link right above says it all.

Thanks for sharing this :)

@antonsamper
Copy link

antonsamper commented Mar 19, 2020

I'm using version 6.5.0 and I'm seeing duplicate values in the req object in my logs:

x-appengine-country: "GBGB"     
x-appengine-default-namespace: "gmail.comgmail.com"      
x-appengine-https: "onon"     
x-appengine-region: "engeng" 

@mdenushev
Copy link

Hello it seems to be here and req: shouldLogRequestStart(request) || options.serializers.req ? undefined : request does the trick

@AdriVanHoudt
Copy link
Contributor

Setting getChildBindings: () => ({}) solves the issue for response logs but also gets rid of request data for request.log logs (being done here https://github.com/pinojs/hapi-pino/blob/master/index.js#L140).

Not sure what the right solution is to get both 🤔

  • getChildBindings: () => ({}) gets rid of doubles in response logs
  • getChildBindings: (request) => ({ req: request}) adds the request info of request.log but causes doubles in response logs
  • I don't think there is a way for the plugin to know whether req was added already
  • An option might be to pass event here https://github.com/pinojs/hapi-pino/blob/master/index.js#L140 which would allow the implementer to add the request only when event is passed?

@AdriVanHoudt
Copy link
Contributor

I'm using version 6.5.0 and I'm seeing duplicate values in the req object in my logs:

x-appengine-country: "GBGB"     
x-appengine-default-namespace: "gmail.comgmail.com"      
x-appengine-https: "onon"     
x-appengine-region: "engeng" 

I'm actually seeing the same thing.
Due to the double req props, Google Cloud logging seems to resolve this by adding the value twice 😅

@AdriVanHoudt
Copy link
Contributor

I think I've found a way to better handle this.
Please take a look at #157

mcollina pushed a commit that referenced this issue Mar 16, 2022
…rt log (#157)

There is no way to know in the response log event handling if the request was already added through the child bindings in `onRequest`.
And there was no way as a user to fix this behaviour properly.

So now `req` will be added once but you need to make sure that you pass it in `childBindings` for the response.
Request start event will have it regardless but also only once.

This is a breaking change as some people will have removed the `req` from the `childBindings` to get around this issue.

Fixes #91
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants