search

pinojs / pino-pretty

🌲Basic prettifier for Pino log lines
MIT License
1.27k stars 150 forks source link

Allow to provide custom levels and custom colors #257

Closed matteozambon89 closed 2 years ago

matteozambon89 commented 3 years ago

This should provide the possibility to handle custom levels and custom colors, especially through CLI.

matteozambon89 commented 3 years ago

@mcollina all done! Let me know if you can merge and deploy it please

matteozambon89 commented 2 years ago

@mcollina and @jsumners can this be merged to master?

matteozambon89 commented 2 years ago

@jsumners I've removed package.json from my commits as the changes shouldn't have been committed in the first place and rebased to pinojs:master once again.

mcollina commented 2 years ago

can you check CI? It was failing

dotcore commented 2 years ago

I am also very interested in this feature, so I looked into why the CI is failing. For some probably non-intentional reason in commit id c94f89c in file test/basic.test.js, following code gets changed to: 

    const expectedLines = [
      '    msg: {',
      '      "b": {',
      '        "c": "d"',
      '      },',
      '      "a": "[Circular ~]"',
      '    }'
    ]

instead of:

    const expectedLines = [
      '    msg: {',
      '      "a": "[Circular]",',
      '      "b": {',
      '        "c": "d"',
      '      }',
      '    }'
    ]

After that the tests are all green. I think this also could use a rebase to upstream again. I could also submit a pull request, if you want.

matteozambon89 commented 2 years ago

I am also very interested in this feature, so I looked into why the CI is failing. For some probably non-intentional reason in commit id c94f89c in file test/basic.test.js, following code gets changed to:

    const expectedLines = [
      '    msg: {',
      '      "b": {',
      '        "c": "d"',
      '      },',
      '      "a": "[Circular ~]"',
      '    }'
    ]

instead of:

    const expectedLines = [
      '    msg: {',
      '      "a": "[Circular]",',
      '      "b": {',
      '        "c": "d"',
      '      }',
      '    }'
    ]

After that the tests are all green. I think this also could use a rebase to upstream again. I could also submit a pull request, if you want.

Sorry if I've been AWOL, between Xmas holidays and New Year "back to work" I haven't had time to look into this issue.

Thanks @dotcore for your input. The commit was made somewhat intentionally as the local test was failing and wasn't allowing me to push. I'll revert that commit and push again to see if it fixes the issue.

I will also do a rebase to upstream, so we can hopefully close this PR.

matteozambon89 commented 2 years ago

@mcollina @jsumners @dotcore reverted the test changes and rebased to upstream

dotcore commented 2 years ago

@mcollina @jsumners @dotcore reverted the test changes and rebased to upstream

One typo is left.

' "a": "[Circular]",',

instead of

' "a": "[Circular],"',

This causes 1 test to fail.

matteozambon89 commented 2 years ago

@dotcore I've updated that, although my tests keep failing for the test/basic.test.js as the format is different on my machine.

I'm on MacOS with node 16, which environment are you running the tests from?

dotcore commented 2 years ago

@dotcore I've updated that, although my tests keep failing for the test/basic.test.js as the format is different on my machine.

I'm on MacOS with node 16, which environment are you running the tests from?

I'm on MacOS with node 17.4.0, but I also tested on node 16.13.2 and 12.22.9 (Switched with nvm).

matteozambon89 commented 2 years ago

@dotcore for some reason this is what I see when I run npm run test, can you confirm this doesn't happen to you and all tests are passing?

> pino-pretty@7.3.0 test
> tap --100 --color

 FAIL  test/basic.test.js
 ✖ should be equal

  test/basic.test.js                                         
  701 |         lines.shift(); lines.pop()                   
  702 |         for (let i = 0; i < lines.length; i += 1) {  
> 703 |           t.equal(lines[i], expectedLines[i])        
      | ------------^                                        
  704 |         }                                            
  705 |         cb()                                         
  706 |       }                                              

  --- expected              
  +++ actual                
  @@ -1,1 +1,1 @@           
  -      "a": "[Circular]", 
  +      "b": {             

  test: test/basic.test.js basic prettifier tests prettifies msg object with
    circular references
  stack: |
    Writable.write [as _write] (test/basic.test.js:703:13)
    doWrite (node_modules/readable-stream/lib/_stream_writable.js:409:139)
    writeOrBuffer (node_modules/readable-stream/lib/_stream_writable.js:398:5)
    Writable.write (node_modules/readable-stream/lib/_stream_writable.js:307:11)
    Pino.write (node_modules/pino/lib/proto.js:190:10)
    Pino.LOG [as info] (node_modules/pino/lib/tools.js:58:21)
    Test.<anonymous> (test/basic.test.js:711:9)
    Test.<anonymous> (test/basic.test.js:685:5)
    Object.<anonymous> (test/basic.test.js:34:1)
    Module.replacementCompile (node_modules/append-transform/index.js:60:13)
    Object.<anonymous> (node_modules/append-transform/index.js:64:4)

 FAIL  test/basic.test.js
 ✖ should be equal

  test/basic.test.js                                         
  701 |         lines.shift(); lines.pop()                   
  702 |         for (let i = 0; i < lines.length; i += 1) {  
> 703 |           t.equal(lines[i], expectedLines[i])        
      | ------------^                                        
  704 |         }                                            
  705 |         cb()                                         
  706 |       }                                              

  --- expected      
  +++ actual        
  @@ -1,1 +1,1 @@   
  -      "b": {     
  +        "c": "d" 

  test: test/basic.test.js basic prettifier tests prettifies msg object with
    circular references
  stack: |
    Writable.write [as _write] (test/basic.test.js:703:13)
    doWrite (node_modules/readable-stream/lib/_stream_writable.js:409:139)
    writeOrBuffer (node_modules/readable-stream/lib/_stream_writable.js:398:5)
    Writable.write (node_modules/readable-stream/lib/_stream_writable.js:307:11)
    Pino.write (node_modules/pino/lib/proto.js:190:10)
    Pino.LOG [as info] (node_modules/pino/lib/tools.js:58:21)
    Test.<anonymous> (test/basic.test.js:711:9)
    Test.<anonymous> (test/basic.test.js:685:5)
    Object.<anonymous> (test/basic.test.js:34:1)
    Module.replacementCompile (node_modules/append-transform/index.js:60:13)
    Object.<anonymous> (node_modules/append-transform/index.js:64:4)

 FAIL  test/basic.test.js
 ✖ should be equal

  test/basic.test.js                                         
  701 |         lines.shift(); lines.pop()                   
  702 |         for (let i = 0; i < lines.length; i += 1) {  
> 703 |           t.equal(lines[i], expectedLines[i])        
      | ------------^                                        
  704 |         }                                            
  705 |         cb()                                         
  706 |       }                                              

  --- expected      
  +++ actual        
  @@ -1,1 +1,1 @@   
  -        "c": "d" 
  +      },         

  test: test/basic.test.js basic prettifier tests prettifies msg object with
    circular references
  stack: |
    Writable.write [as _write] (test/basic.test.js:703:13)
    doWrite (node_modules/readable-stream/lib/_stream_writable.js:409:139)
    writeOrBuffer (node_modules/readable-stream/lib/_stream_writable.js:398:5)
    Writable.write (node_modules/readable-stream/lib/_stream_writable.js:307:11)
    Pino.write (node_modules/pino/lib/proto.js:190:10)
    Pino.LOG [as info] (node_modules/pino/lib/tools.js:58:21)
    Test.<anonymous> (test/basic.test.js:711:9)
    Test.<anonymous> (test/basic.test.js:685:5)
    Object.<anonymous> (test/basic.test.js:34:1)
    Module.replacementCompile (node_modules/append-transform/index.js:60:13)
    Object.<anonymous> (node_modules/append-transform/index.js:64:4)

 FAIL  test/basic.test.js
 ✖ should be equal

  test/basic.test.js                                         
  701 |         lines.shift(); lines.pop()                   
  702 |         for (let i = 0; i < lines.length; i += 1) {  
> 703 |           t.equal(lines[i], expectedLines[i])        
      | ------------^                                        
  704 |         }                                            
  705 |         cb()                                         
  706 |       }                                              

  --- expected               
  +++ actual                 
  @@ -1,1 +1,1 @@            
  -      }                   
  +      "a": "[Circular ~]" 

  test: test/basic.test.js basic prettifier tests prettifies msg object with
    circular references
  stack: |
    Writable.write [as _write] (test/basic.test.js:703:13)
    doWrite (node_modules/readable-stream/lib/_stream_writable.js:409:139)
    writeOrBuffer (node_modules/readable-stream/lib/_stream_writable.js:398:5)
    Writable.write (node_modules/readable-stream/lib/_stream_writable.js:307:11)
    Pino.write (node_modules/pino/lib/proto.js:190:10)
    Pino.LOG [as info] (node_modules/pino/lib/tools.js:58:21)
    Test.<anonymous> (test/basic.test.js:711:9)
    Test.<anonymous> (test/basic.test.js:685:5)
    Object.<anonymous> (test/basic.test.js:34:1)
    Module.replacementCompile (node_modules/append-transform/index.js:60:13)
    Object.<anonymous> (node_modules/append-transform/index.js:64:4)

 FAIL  test/basic.test.js
 ✖ should be equal

  test/basic.test.js                                                             
  931 |     Object.assign(fs, { close, writeSync })                              
  932 |     t.match(chunks.join(''), /INFO .+: this message has been buffered/)  
> 933 |     t.equal(closeCalled, false)                                          
      | ------^                                                                  
  934 |   })                                                                     
  935 |                                                                          
  936 |   t.test('stream usage', async (t) => {                                  

  test: test/basic.test.js basic prettifier tests does not call fs.close on stdout
    stream
  found: true
  wanted: false
  compare: ===
  stack: |
    Test.<anonymous> (test/basic.test.js:933:7)
    Test.<anonymous> (test/basic.test.js:910:5)
    Object.<anonymous> (test/basic.test.js:34:1)
    Module.replacementCompile (node_modules/append-transform/index.js:60:13)
    Object.<anonymous> (node_modules/append-transform/index.js:64:4)
dotcore commented 2 years ago

@dotcore for some reason this is what I see when I run npm run test, can you confirm this doesn't happen to you and all tests are passing?

I can confirm that it works for me, also CI didn't fail. No idea why it does not work for you.

matteozambon89 commented 2 years ago

@dotcore for some reason this is what I see when I run npm run test, can you confirm this doesn't happen to you and all tests are passing?

I can confirm that it works for me, also CI didn't fail. No idea why it does not work for you.

Who knows 😂

dotcore commented 2 years ago

@dotcore for some reason this is what I see when I run npm run test, can you confirm this doesn't happen to you and all tests are passing?

I can confirm that it works for me, also CI didn't fail. No idea why it does not work for you.

Who knows 😂

Not sure if this may help, but maybe delete your node_modules and run npm i, in case you have some outdated dependencies.

matteozambon89 commented 2 years ago

@dotcore nope, that didn't solve the issue. Something between node and MacOS must be configured differently in this machine.

@mcollina can we have this PR merged to master?

mcollina commented 2 years ago

Done