SAP / cf-nodejs-logging-support

Node.js Logging Support for Cloud Foundry provides the creation of structured log messages and the collection of request metrics
https://sap.github.io/cf-nodejs-logging-support/
Apache License 2.0
43 stars 22 forks source link

Winston Support in v7 #195

Closed MichaelFlucher closed 10 months ago

MichaelFlucher commented 1 year ago

How should the Transport for winston be created with v7? The old way with

import * as log from "cf-nodejs-logging-support";
log.createWinstonTransport()

does not work anymore, so i tried it with empty options:

import log, { Level, Framework } from "cf-nodejs-logging-support";
log.createWinstonTransport({})

This gives the following error on the BTP:

2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR /home/vcap/app/node_modules/cf-nodejs-logging-support/build/main/lib/winston/winstonTransport.js:22
2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR this.logger.logMessage(info.level, info.message);
2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR ^
2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR TypeError: Cannot read properties of undefined (reading 'logMessage')
2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR at CfNodejsLoggingSupportLogger.log (/home/vcap/app/node_modules/cf-nodejs-logging-support/build/main/lib/winston/winstonTransport.js:22:25)
2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR at CfNodejsLoggingSupportLogger._write (/home/vcap/app/node_modules/winston-transport/index.js:82:19)
2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR at doWrite (/home/vcap/app/node_modules/readable-stream/lib/_stream_writable.js:409:139)
2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR at writeOrBuffer (/home/vcap/app/node_modules/readable-stream/lib/_stream_writable.js:398:5)
2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR at Writable.write (/home/vcap/app/node_modules/readable-stream/lib/_stream_writable.js:307:11)
2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR at DerivedLogger.ondata (/home/vcap/app/node_modules/readable-stream/lib/_stream_readable.js:681:20)
2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR at DerivedLogger.emit (node:events:513:28)
2023-09-25T07:53:52.786+0000 [APP/PROC/WEB/0] STDERR at addChunk (/home/vcap/app/node_modules/readable-stream/lib/_stream_readable.js:298:12)
2023-09-25T07:53:52.787+0000 [APP/PROC/WEB/0] STDERR at readableAddChunk (/home/vcap/app/node_modules/readable-stream/lib/_stream_readable.js:280:11)
2023-09-25T07:53:52.787+0000 [APP/PROC/WEB/0] STDERR at Readable.push (/home/vcap/app/node_modules/readable-stream/lib/_stream_readable.js:241:10)
2023-09-25T07:53:52.787+0000 [APP/PROC/WEB/0] STDERR Node.js v18.16.1
2023-09-25T07:53:52.797+0000 [APP/PROC/WEB/0] STDOUT Exit status 1
2023-09-25T07:53:53.676+0000 [APP/PROC/WEB/0] STDOUT Exit status 143
christiand93 commented 12 months ago

Hi Michael, Please excuse my delayed response. The problem is caused by an unassigned rootLogger parameter when creating the winston transport. As you can see here, the rootLogger gets only assigned, when no options are given. The idea is to allow specifying other Logger instances, if needed. However, the logic does not ensure that the rootLogger gets set in all cases. This can and should be improved.

Furthermore, the options parameter is mandatory, which causes errors if it is not specified. Making it optional would also improve the situation. To mitigate the problem, you can provide a null value to the options parameter. This ensures that the rootLogger gets assigned properly:

import log, { Level, Framework } from "cf-nodejs-logging-support";
log.createWinstonTransport(null)

I will work on a fix asap.

MichaelFlucher commented 10 months ago

Thanks @christiand93 for the hint, I didn't even think of passing null. For now I got it to work with the following lines:

import log from "cf-nodejs-logging-support";
import * as winston from "winston";

// for the transports of winston, as there is a missing overlap reported by typescript
log.createWinstonTransport(null) as unknown as winston.transport
christiand93 commented 10 months ago

The fix is merged and will be shipped with version 7.2.1.