Skip to content
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

Winston Transport File - zippedArchive option doesn't work correctly #1128

Closed
KingRial opened this issue Nov 3, 2017 · 33 comments
Closed

Winston Transport File - zippedArchive option doesn't work correctly #1128

KingRial opened this issue Nov 3, 2017 · 33 comments
Assignees
Labels
winston-file Issues to move to `winston-file` when we create it

Comments

@KingRial
Copy link

KingRial commented Nov 3, 2017

If you use the following transport file:

new (winston.transports.File)({
          level: 'silly',
          filename: './test.log',
          maxsize: 500000,
          maxFiles: 4,
          format: format.combine(
            format.splat(),
            format.label({ label: 'test' }),
            format.timestamp(),
            format.prettyPrint(),
            format.printf( log => {
              return '[ ' + log.timestamp + ' ][ ' + log.level + ' ][ '+ log.label + ' ] ' + log.message;
            })
          ), tailable: true,
          zippedArchive: true,
          exitOnError: false
        }) );

The resulting "test.log" is zipped.

This is wrong since the documentation tells that:

zippedArchive: If true, all log files but the current one will be zipped.

@ChristiaanWillemsen
Copy link

I have the exact same issue with 3.0.0-rc4. and even the zipped file is useless, it is not appended when you log, and I cannot read it with gzcat.

@wjford
Copy link

wjford commented Jul 10, 2018

I have the same issue with 3.0.0.

    new winston.transports.File({
      level: 'info',
      filename: LOGS_PATH,
      handleExceptions: true,
      maxsize: MAX_LOG_SIZE,
      timestamp: true,
      maxFiles: 5,
      eol: '\r\n',
      prettyPrint: true,
      zippedArchive: true
    })

This results in the latest file being gzipped. In 2.4.2, all but the latest file were zipped.

@sanderboom
Copy link

In my case the logfiles are not zipped at all, although .gz has been added.

λ file somelog.log.gz
somelog.log.gz: ASCII text

Gzipped would look like:

λ gzip package.json
λ file package.json.gz
package.json.gz: gzip compressed data, was "package.json", last modified: Thu Aug 23 16:41:38 2018, from Unix, original size 2354

@paolotremadio
Copy link

I use this package in all my projects: https://github.com/paolotremadio/homeautomation-winston-logger

All the files emitted are .gz but they are still in plain text (so they are not actually compressed). Can someone tell me what I'm doing wrong?

@masoudltf
Copy link

i have same issue, only ".gz" appends to the filename and not correctly gzipped.

@masoudltf
Copy link

can anyone help us?

@KingRial
Copy link
Author

KingRial commented Dec 8, 2018

@masoudltf: it's odd, it should zip all the logs (even if the first one should be not zipped).
Could you share the code ?

@paolotremadio
Copy link

@masoudltf
Copy link

masoudltf commented Dec 9, 2018

@KingRial
thanks for reply.
this is my code:

const defaultLogFormat = [
    winston.format.timestamp(),
    winston.format.align(),
    winston.format.printf(info => {
        return '[${info.level}] [${info.timestamp}] ${info.message}';
    })
];

new winston.transports.File({
        format: winston.format.combine(
            ...defaultLogFormat
        ),
        filename: 'path-to-filename',
        maxsize: 100000000,
        zippedArchive: true
})

@masoudltf
Copy link

Here’s an example: https://github.com/paolotremadio/homeautomation-winston-logger/blob/master/index.js

@paolotremadio :
According to above code in my previous comment, i don't think this code is wrong.

@KingRial
Copy link
Author

KingRial commented Dec 10, 2018

@masoudltf: you are right, the latest winston (3.1.0) doesn't zip any file but only adds the .gz extension.
Infact, it seems there is no method which inflates the logs, only the Zlib instance

@masoudltf
Copy link

bug report needed?

@DABH
Copy link
Contributor

DABH commented Dec 28, 2018

Sorry this issue went unattended. Looking at https://github.com/winstonjs/winston/blob/master/lib/winston/transports/file.js#L543 it looks like the File transport returns either a gzip stream or a regular stream. So if zippedArchive is true then your log should be getting zipped. There is also some code https://github.com/winstonjs/winston/blob/master/lib/winston/transports/file.js#L663 that looks related to adding .gz extensions to old files when rotation happens. But indeed I don't see something that indicates all but the latest log will be zipped -- looks more like all or none get zipped depending on zippedArchive. Also, there is no method to inflate logs, that would presumably be the user's job if they cared to view those logs later.

If you can reproduce issues using master, please let me know what code example you're using, and what your desired behavior is. We'll try to make everyone happy on this issue :)

@indexzero indexzero added the winston-file Issues to move to `winston-file` when we create it label Jan 29, 2019
@glemco
Copy link

glemco commented Feb 4, 2019

What it seems is happening [in _createStream method in the File class] is that it tries to open a gzip stream but it's not actually piping anything into it, instead the source readable stream is directly piped into the file (as plain text).
While rotating (tailable option) the file is simply renamed as <logname>.gz but without any compression in it.
It looks like this feature is an hybrid between how it used to work in version 2.x (the file is compressed on rotation) and a new implementation in which all the log files are created as already compressed (hence they all should have the gz extension, also the first one).
This behaviour (if that's what the developer wants) can be achieved doing something like source.pipe(gzip).pipe(dest) once the destination file is opened, instead of just source.pipe(dest), leaving the gzipped stream alone [still in _createStream method].

@shayantabatabaee
Copy link

@DABH
I have the same issue too, I also upgraded my Winston version to latest version (3.2.1) but still no success. when the zippedArchive is true , the logger only rename the extension of file so the zip file is corrupted.

@paolotremadio
Copy link

Is anyone working on a patch? Othwrwise I can give it a try

@glemco
Copy link

glemco commented Feb 6, 2019

I tried by just using the gzip stream, but it was giving me problems on the most recent log (the one that's currently written to, with tailable option). It was taking too much time so we rather decided to stay on 2.x . If you can make a pull request fixing it, that would be great

@smitalm
Copy link

smitalm commented Jul 4, 2019

+1
zippedArchive option on winston.transports.File is broken - resulting files have .gz suffix, but are corrupted.

winston version 3.2.1

@raapperez
Copy link

Any update on this? Anyone knows an older version that it works?

@paolotremadio
Copy link

2.4.4 does zip the rotated files.

@smitalm
Copy link

smitalm commented Jul 17, 2019

In our case, gzip functionality was crucial, so we decided to migrate our logging to https://log4js-node.github.io/log4js-node/ and everything worked flawlessly

pixtron added a commit to pixtron/winston that referenced this issue Dec 3, 2019
* Zip archives during rotation if zippedArchive is true
* Remove gzip stream which was never written to
  (see winstonjs#1128 (comment))
* ads tests checking the contents of zipped archives
* ads tests for rolling files with tailable: false
* Fixes previously failing test tests/tail.file.test.js
@mattvasc
Copy link

@DABH
I have the same issue too, I also upgraded my Winston version to latest version (3.2.1) but still no success. when the zippedArchive is true , the logger only rename the extension of file so the zip file is corrupted.

Still happening on 3.3.3 😔

@max0x7ba
Copy link

The problem persists in version 3.8.1.

@Lilleri
Copy link

Lilleri commented Apr 28, 2023

still persist in 3.8.2

@Pulkit0729
Copy link
Contributor

Hi @DABH , I would like to give it a try. Please assign it to me.

@DABH DABH self-assigned this Jul 13, 2023
@DABH DABH assigned Pulkit0729 and unassigned DABH Jul 13, 2023
@DABH
Copy link
Contributor

DABH commented Jul 13, 2023

Done — thank you so much for being willing to take this on!!

@Pulkit0729
Copy link
Contributor

Hey @DABH , can you specify what is the expeected ideal behaviour for zipped archive option? Also was there a previous version, when it was working as expected?

@Pulkit0729
Copy link
Contributor

Hi @DABH , I have a question that when should the log file be zipped? Is it after the file size is equal to maxsize and new file is created. If yes, in that case, we will start logging initially with no zip files?

@asnawisaharuddin
Copy link

Is there a workaround for this?

the logger only rename the extension of the file to .gz

Im using v3.10

@DABH
Copy link
Contributor

DABH commented Jul 26, 2023

@Pulkit0729 Yeah I think you rotate when the uncompressed file size reaches the max size, and then you compress the log file that you just finished writing to. And initially you would have no zip files. But if you have file rotation on, then with zipping set to true, you would end up with multiple zip files

@DABH
Copy link
Contributor

DABH commented Jul 26, 2023

@Pulkit0729 I think the last time this worked was probably Winston 2.x, it’s one of the features we sadly didn’t have test cases for and wasn’t carefully accounted for or tested when writing 3.x

@Pulkit0729
Copy link
Contributor

Hi @DABH , I have added a PR #2337 for this. Please review it.

@DABH
Copy link
Contributor

DABH commented Jan 3, 2024

Closed via #2337

@DABH DABH closed this as completed Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
winston-file Issues to move to `winston-file` when we create it
Projects
None yet
Development

Successfully merging a pull request may close this issue.