Add winston-newrelic-agent-transport to transport documentation

[kimnetics]

Background

The New Relic agent typically automatically sends Winston logs to New Relic when using CommonJS. With CommonJS no additional transport should be needed. However, when using ECMAScript modules, the automatic sending of logs can with certain coding patterns not work.

The winston-newrelic-agent-transport transport was created to leverage the New Relic agent to send logs to New Relic for the times when automatic log sending is not working.

PR

This PR updates the transports documentation to include information about winston-newrelic-agent-transport.

Why create another New Relic transport?

The current Newrelic Transport only handles errors. Its use of newrelic.noticeError to transmit log messages constrains it to specific use cases. Also, the noticeError method was not intended for logging.

[DABH]

@kimnetics Do you feel that this transport subsumes all of the existing functionality in https://github.com/winstonjs/winston/blob/master/docs/transports.md#newrelic-transport ? That transport (https://github.com/namshi/newrelic-winston) hasn't been updated in 5 years, so I'm assuming it's not actively maintained, and I'd be fine just replacing that transport with this one -- if it's a strict superset of features. Let me know your thoughts!

[kimnetics]

@DABH It is not a strict superset, but I do not believe it should be. In regards to general logging, my transport is a superset as the old transport only logs errors. The old transport does have some characteristics which are not brought forward into my transport. My rationale for not bringing them forward is explained below:

'dev' or 'test' exclusion from logging

The old transport has hardcoded logic to exclude 'dev' and 'test' environment from logging. That seems pretty opinionated for a logging transport. It could surprise people. My opinion is that a transport should just transport. That kind of logic is probably better left to the app's Winston logger code where any other special behavior configuration would likely be.

Personally, I want logging to happen in dev and test, so I do not want that logic in the first place.

Use of newrelic.noticeError

The old transport is using newrelic.noticeError to log. I believe this may be throwing off New Relic (NR) error counts, but there are rare cases where you would want to use it and with my transport you need to call it separately.

The purpose of that method is not to log but rather to let NR know that an error has occurred. This allows NR to properly class it as an error in the UI and counts. Normally errors are automatically noticed by NR so that method is typically only for times when you want something to be counted as an error, but for some reason don't want to throw an error. In other words, the method used by the old transport is for very special use cases. I believe I have only used it once in years of using NR.

My transport does not use newrelic.noticeError so in the special cases when you need to let NR know an error has occurred you need to call it separately from the logging statement. This matches to the NR API which separates logging from noticing an error. There is a reason they are separate. You sometimes don't want to both log an error and count an error.

I am concerned that the old transport may be producing potentially confusing output on the NR error page and counts. I believe both the NR automatically caught errors and the old transport newrelic.noticeError log messages are showing there. Users have to be careful when considering error counts as they may be doubled if the old transport is used to log errors already being caught by NR.

[kimnetics]

One more thing about the newrelic.noticeError behavior.

Since the old transport is using newrelic.noticeError, its logging messages are showing on the Error page in the NR UI.

My transport has all logging messages showing on the Logs page in the NR UI.

[kimnetics]

@DABH I do believe we should replace the existing newrelic-winston transport with this one. Its use of newrelic.noticeError to log is problematical. I was planning to make a video to show what I mean about the double of entry of error messages, when I found that the transport is based on a deprecated New Relic agent version.

Doing an npm install produces the following:

 ✘ npm install newrelic-winston
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'newrelic@5.13.1',
npm WARN EBADENGINE   required: { node: '>=6.0.0 <13.0.0', npm: '>=3.0.0' },
npm WARN EBADENGINE   current: { node: 'v18.18.2', npm: '9.8.1' }
npm WARN EBADENGINE }
npm WARN deprecated newrelic@5.13.1: This version of the New Relic Node Agent has reached the end of life.

If you're agreeable, I could remove the old transport as part of this PR. Please let me know if I should add another commit that removes the old transport.

[DABH]

Gotcha. Yeah, I figured as much due to the lack of activity on that repo. I'm okay with adding yours and removing the other one (feel free to make another commit or PR for that). Thanks again for your contribution to the winston ecosystem!

[kimnetics]

@DABH As I thought about it, a separate PR to remove newrelic-winston was cleaner than using a another commit on this PR. I created a separate PR to handle the removal: #2405.