-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Feature/winston 3 migration #5275
Feature/winston 3 migration #5275
Conversation
* ⚡ Release 3.1.3 * Update CHANGELOG.md
Bumps [winston](https://github.com/winstonjs/winston) and [winston-daily-rotate-file](https://github.com/winstonjs/winston-daily-rotate-file). These dependencies needed to be updated together. Updates `winston` from 2.4.4 to 3.1.0 - [Release notes](https://github.com/winstonjs/winston/releases) - [Changelog](https://github.com/winstonjs/winston/blob/master/CHANGELOG.md) - [Commits](winstonjs/winston@2.4.4...3.1.0) Updates `winston-daily-rotate-file` from 1.7.2 to 3.5.1 - [Release notes](https://github.com/winstonjs/winston-daily-rotate-file/releases) - [Commits](winstonjs/winston-daily-rotate-file@v1.7.2...v3.5.1) Signed-off-by: dependabot[bot] <support@dependabot.com>
@pixel2 thanks! Those changes were expected. Can you have a look at the tests? Those seem to be failing on the CI. |
Codecov Report
@@ Coverage Diff @@
## master #5275 +/- ##
=========================================
- Coverage 93.91% 92.2% -1.71%
=========================================
Files 123 123
Lines 8972 8970 -2
=========================================
- Hits 8426 8271 -155
- Misses 546 699 +153
Continue to review full report at Codecov.
|
The problem seemed to be related to the streams not closing properly. Even though the documentation says that a reconfiguration should remove all previous transports. However I haven't looked at the log output, but there seem to be some changes in how Winston 3 handle formats. Not sure if that will affect logging in parse server. |
The failed tests in LoggerController are because of a bug in Winston 3.1 (winstonjs/winston#1130). There is an ugly workaround but a fix (winstonjs/winston#1434) has already been merged with master, so hopefully, it won't be that long until they release a new version. |
Just realized that https://github.com/winstonjs/winston/tree/master fixed the problem with not closing streams as well. Will revert those changes and wait for an updated Winston release. |
Yeah, there’s a pending Winston release that we need. Otherwise we’d need to keep the loggers on / change the way the server works completely around logging |
Bumps [winston](https://github.com/winstonjs/winston) and [winston-daily-rotate-file](https://github.com/winstonjs/winston-daily-rotate-file). These dependencies needed to be updated together. Updates `winston` from 2.4.4 to 3.2.0 - [Release notes](https://github.com/winstonjs/winston/releases) - [Changelog](https://github.com/winstonjs/winston/blob/master/CHANGELOG.md) - [Commits](winstonjs/winston@2.4.4...3.2.0) Updates `winston-daily-rotate-file` from 1.7.2 to 3.6.0 - [Release notes](https://github.com/winstonjs/winston-daily-rotate-file/releases) - [Commits](winstonjs/winston-daily-rotate-file@v1.7.2...v3.6.0) Signed-off-by: dependabot[bot] <support@dependabot.com>
I've merged with #5326. Unfortunately, we had to fix our logging with the old adapter (Winston 2), so this is not as urgent for us anymore. The Winston 3 was a bigger change than I first realized with changes to formatters and I haven't been able to analyze how this change affects the ParseServer logging. |
Hi @pixel2, I might be able to pick this up if you don't mind, I am making some progress with tests. See https://github.com/stage88/parse-server/tree/winston-3 |
A small change in how winston logs messages, I'll update any statements that I find that could duplicate the message in the log file, for example From their readme:
|
Migration to Winston 3.x. Had to make some major changes to the underlying structure since Winston 3 allows for multiple transports of the same type and does not track the transport name anymore.
winston.transports
has changed from a dictionary to a list.