villadora / express-bunyan-logger Goto Github PK

The current module forces to create a different bunyan logger instance for express which I think is not very helpful if you have a bunyan logger instance in your app.

I created a pull request for fixing it: #9

Hey I was wondering if it is possible to specify specific excluding for streams.

Since the req and res object can get pretty big I dont love to exclude them in the console. But I would like to keep them stored in a file or database.

Thnx.

instead of having a fixed version...
Otherwise I get warnings and can't do npm shrinkwrap

hi there,

your lib makes it possible to pass in a pre-existing bunyan instance (which will be used in preference of creating a new bunyan instance if given), see here:

https://github.com/villadora/express-bunyan-logger/blob/master/index.js#L28

... but this does not seem to be documented.

PS: you might want to check that opts.logger is, if swet, actually an instance of bunyan before using it.

Describe the bug:
We are getting vulnerability error in express-bunyan-logger-1.3.3 version.

Below is the dependency tree:
express-bunyan-logger-1.3.3.tgz (Root Library)
❌ useragent-2.3.0.tgz (Vulnerable Library)

Expected behavior:
The express-bunyan-logger-1.3.3 should not be having any dependency on vulnerable components.

Actual behavior:
The dependent package useragent-2.3.0.tgz refers to be vulnerable components, as per the author the "Fix Resolution: NorDroN.AngularTemplate - 0.1.6;dotnetng.template - 1.0.0.4;JetBrains.Rider.Frontend5 - 213.0.20211008.154703-eap03;MIDIator.WebClient - 1.0.105".

Thanks & Regards,
Praveen Kumar D.

The passed in options are mutated by express-bunyan-logger. That doesn't work well when you want to share config settings between the standard and error logger.

https://github.com/villadora/express-bunyan-logger/blob/master/index.js#L30

Mutation breaks the following use-case:

var options = { format: ':res[statusCode] :method' };
app.use(logger(options));
  ...
app.use(logger.errorLogger(options));

Logging the entire metadata object can be quite noisy and excluding every single field in it is inefficient and wouldn't work anyway. I think it would be handy to have a way to say "print the format string but not meta".

It could be something like this:

// app.js
  app.use(require('express-bunyan-logger')({
    logger: logger,
    format: ':remote-address - - :method :url HTTP/:http-version :status-code :res-headers[content-length] :referer :user-agent[family] :user-agent[major].:user-agent[minor] :user-agent[os] :response-time ms',
    excludes: ['*']
  }));

// express-bunyan-logger/index.js
            if (~excludes.indexOf('*')) {
              logFn.call(logger, format(meta));
            } else {
              logFn.call(logger, meta, format(meta));
            }

or it could be another option, something like includeMeta that defaults to true. I'd like to submit a pull request for something like that. Thoughts?

I was incorrectly doing

var bunyanLogger = require("express-bunyan-logger");
var express = require("express");
var app = express();
app.use(bunyanLogger);

when I really wanted:

app.use(bunyanLogger());

It's hard to tell what goes wrong in the first case. The app just hangs and never hits the callback. It would be nice if this library detected if opts was actually an http.IncomingMessage and returned an appropriate error.

express-bunyan-logger incorrectly logs a 200 response code when connections are aborted. This happens when a connection is closed due to timeout (server.timeout value is reached), for example.

Other libraries correctly report undefined instead of 200 in this case. For example, see this morgan issue and the corresponding source code here.

Well I think the title is pretty much explanatory.

Is there any particular reason about that?

If you add an obfuscate option like obfuscate: ['body.password'] and attempt to access req.body.password after the req has been logged (immediate: true for example), you'll receive [HIDDEN] as the value for req.body.password. The logger should deep copy the req/res data (body, headers), rather than assigning, to prevent mutation.

When using this with express I get the following deprecation warnings

connections property is deprecated. Use getConnections() method

It seems that express-bunyan-logger uses this for serializing requests.
Would it be possible to have it fixed ?

The relevant deprecation warning is here https://github.com/joyent/node/blob/master/lib/net.js#L1004

It would be really great if it was possible to define some field to be added to all log jsons. This is similar to Bunyan's log.child({key:value}) method which creates a log that appends fields to all log items from that logger.

https://github.com/trentm/node-bunyan#logchild

Hello,

I use express-bunyan-logger as child logger of bunyan. My config is:

var logExpressConfig = {
  type: 'app-express'
};

var logExpressExcludes = [
  'user-agent',
  'body',
  'short-body',
  'req-headers',
  'res-headers',
  'req',
  'res',
  'incoming',
  'response-hrtime'
]

var logger = bunyan.createLogger(logConfig);

var expressLogger = logger.child(logExpressConfig);

module.exports = logger;

module.exports.expressLogger = {
  logger: expressLogger,
  excludes: logExpressExcludes,
}

var expressLogger = require('../logger.js').expressLogger;
var logger = require('express-bunyan-logger')(expressLogger);
app.use(logger);

Is there way to get rid of brackets from console? Output is formated with bunyan -o short. I'm sorry if this is not problem of this package.

13:07:57.038Z  INFO app: ::1 <-- GET /login HTTP/1.1 304 - http://localhost:3000/ Chrome 41.0 Mac OS X 10.10.3 253.729234 ms (req_id=180869c9-c85d-4082-99fb-08efa77685ee, type=app-express, remote-address=::1, ip=::1, method=GET, url=/login, referer=http://localhost:3000/, http-version=1.1, response-time=253.729234, status-code=304)

Is it possible to exclude some routes from being logged?
If so, how can this be done?

when I try to get e.g a non existent file the request is logged as successful with status code 200 although the client correctly gets a 404...
I tried to debug it and at the time where the hook is the status code is still 200...

So I looked at the connect logger source:

  return function logger(req, res, next) {
    var sock = req.socket;
    req._startTime = new Date;
    req._remoteAddress = sock.socket ? sock.socket.remoteAddress : sock.remoteAddress;

    function logRequest(){
      res.removeListener('finish', logRequest);
      res.removeListener('close', logRequest);
      var line = fmt(exports, req, res);
      if (null == line) return;
      stream.write(line + '\n');
    };

    // immediate
    if (immediate) {
      logRequest();
    // proxy end to output logging
    } else {
      res.on('finish', logRequest);
      res.on('close', logRequest);
    }


    next();
  };
};

looks like this is the right way to do it (immediate=false)...

Any chance to get this in?
Thanks
Ognian

/Users/mbenin/projects/copilot/node_modules/express-bunyan-logger/index.js:87
fmt = fmt.replace(/"/g, '"');
^
TypeError: Cannot call method 'replace' of undefined
at compile (/Users/mbenin/projects/copilot/node_modules/express-bunyan-logger/index.js:87:15)
at Function.module.exports.errorLogger (/Users/mbenin/projects/copilot/node_modules/express-bunyan-logger/index.js:18:58)
at module.exports (/Users/mbenin/projects/copilot/node_modules/express-bunyan-logger/index.js:6:33)
at appInit (/Users/mbenin/projects/copilot/express/config.js:55:47)
at configureByEnvironment (/Users/mbenin/projects/copilot/express/config.js:98:5)
at Function. (/Users/mbenin/projects/copilot/express/config.js:130:5)
at Function.app.configure (/Users/mbenin/projects/copilot/node_modules/express/lib/application.js:391:61)
at module.exports (/Users/mbenin/projects/copilot/express/config.js:129:7)
at Object. (/Users/mbenin/projects/copilot/express/init.js:10:20)
at Module._compile (module.js:456:26)

Hi!

You made a typo error on npm:
https://npmjs.org/package/express-bunyan-logger

s/exporess-buyan-logger/express-bunyan-logger/

Thanks for the module

Regards,
Federico

I am interested in this feature for very high performance APIs. Any thoughts appreciated.

This library will generate deprecation warnings with node v12.

(node:8) [DEP0066] DeprecationWarning: OutgoingMessage.prototype._headers is deprecated
    at ServerResponse.logging (/app/node_modules/express-bunyan-logger/index.js:130:36)
    at ServerResponse.emit (events.js:201:15)
    at ServerResponse.EventEmitter.emit (domain.js:471:20)

The fix is fairly straightforward.
I can create a PR for it if one is required.

Attempting to installing with iojs 3.3 results in the following error:

$ npm install express-bunyan-logger
> [email protected] install /Users/mike/code/proj/node_modules/express-bunyan-logger/node_modules/bunyan/node_modules/dtrace-provider
> node scripts/install.js

---------------
Building dtrace-provider failed with exit code 1 and signal 0
re-run install with environment variable V set to see the build output
---------------
[email protected] node_modules/express-bunyan-logger
├── [email protected] ([email protected])
└── [email protected] ([email protected], [email protected], [email protected])

The root cause seems to be that node-bunyan (aka bunyan) has a stale dependency on node-dtrace 0.5 which is incompatible with iojs 3. There is already a pull request to fix the problem in node-bunyan:

trentm/node-bunyan#294

I'm sure everything will be fixed up over the next few days/weeks. In the meantime, if you must install express-bunyan-logger with iojs 3 I have a forked repo that depends on a node-bunyan fork that has the dtrace fix applied:

https://github.com/MCF/express-bunyan-logger

Add the following line in your package.json to (temporarily) get the install working:

"express-bunyan-logger": "MCF/express-bunyan-logger",

Be sure to switch back to the normal package once the dependency problem in bunyan has been fixed and released.

Thank you for fixing #2 that fast.
I still have more suggestions (see https://github.com/expressjs/morgan):

• add the responseTime
• somehow the "levels" should be changeable to

< 300 -> info
< 400 -> warning
< 500 -> error
>= 500 -> fatal

• parsing of the user agent should be optional

What do you think?
Ognian

It would be awesome if we could choose what meta info goes to the log file (for smaller files). I refer specifically to the "body", "req" and "res" properties of "meta" that output a lot of info.

An option like "excludeMeta" or something like that with an array of keys you want to ignore would be enough.