level-change event fired when child is created

[laSinteZ]

When I create a child instance of a logger, level-change event is fired. This, for example, causes 2 events per request in pino-http (can provide simple repro). I am not sure if it's desired behaviour, since it isn't described in the documentation and can be a bit confusing. If it is a desired behaviour, I can help expanding the documentation.

Example code:

const pino = require("pino");
const logger = pino();

logger.on("level-change", () => {
  console.log("level-change event had happened");
});

logger.info("Hello, world. No events.")
logger.child({});
logger.child({});
logger.child({});
logger.child({});

Output:

{"level":30,"time":1618034735938,"pid":16304,"hostname":"MacBook-Air.local","msg":"Hello, world. No events."}
level-change event had happened
level-change event had happened
level-change event had happened
level-change event had happened

[jsumners]

It happens because we need to set the level on the child instance:

pino/lib/proto.js

Line 124 in 84fa44b

instance[setLevelSym](childLevel)

Which in turn invokes:

pino/lib/levels.js

Lines 84 to 111 in 84fa44b

	function setLevel (level) {
	const { labels, values } = this.levels
	if (typeof level === 'number') {
	if (labels[level] === undefined) throw Error('unknown level value' + level)
	level = labels[level]
	}
	if (values[level] === undefined) throw Error('unknown level ' + level)
	const preLevelVal = this[levelValSym]
	const levelVal = this[levelValSym] = values[level]
	const useOnlyCustomLevelsVal = this[useOnlyCustomLevelsSym]
	const hook = this[hooksSym].logMethod
	for (const key in values) {
	if (levelVal > values[key]) {
	this[key] = noop
	continue
	}
	this[key] = isStandardLevel(key, useOnlyCustomLevelsVal) ? levelMethods[key](hook) : genLog(values[key], hook)
	}
	this.emit(
	'level-change',
	level,
	levelVal,
	labels[preLevelVal],
	preLevelVal
	)
	}

I suppose an optional second parameter can be passed to signal omitting the event.

[mcollina]

I think this is a bug, as we should call reinitialize the EventEmitter state when creating a child. However, I'm not sure this is worth the overhead!

[laSinteZ]

Well, I guess at least it's worth to document it, since I was quite surprised to see those events polluting logs (even though I've solved it with simple if newVal !== prevVal in my handler)

[mcollina]

If you would like to try fixing it, I'd welcome a PR. The problem is caused by child loggers "inheriting" their parent event listeners.

[mcollina]

A PR updating the doc is also fine.

[laSinteZ]

@mcollina wouldn't it be effectively a breaking change if child loggers WON'T inherit event listeners though? I believe someone could rely on this functionality.

[mcollina]

@mcollina wouldn't it be effectively a breaking change if child loggers WON'T inherit event listeners though? I believe someone could rely on this functionality.

version numbers are there to be bumped. We have the next branch that we accumulate all our breaking changes and then ship them.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.