search

newrelic / node-newrelic

New Relic Node.js agent code base. Developers are welcome to create pull requests here, please see our contributing guidelines. For New Relic technical support, please go to http://support.newrelic.com.
Apache License 2.0
966 stars 394 forks source link

Refactor pino instrumentation #2424

Closed jsumners-nr closed 3 weeks ago

jsumners-nr commented 1 month ago

See the conversation in https://github.com/newrelic/node-newrelic/pull/2421. Basically, the following needs to be refactored in order to satisfy the SonarQube linter: https://github.com/newrelic/node-newrelic/blob/599072b43b77a8c11c9ef414b08dfe6e04bca9d2/lib/instrumentation/pino/pino.js#L18-L95

Prior to opening the PR for review, I attempted:

function instrument(shim, tools) {
    // snip
    shim.wrap(tools, 'asJson', function wrapJson(shim, asJson) {
        return wrappedAsJson.bind({ shim, agent, symbols, asJson, metrics, config})
    }
}

function wrappedAsJson() {
    const { shim, agent, symbols, asJson, metrics, config } = this
    // implementation
}

But that won't work because wrappedAsJson needs this to be bound to the instrumented instance of Pino.

workato-integration[bot] commented 1 month ago

https://new-relic.atlassian.net/browse/NR-298951