Skip to content

Commit

Permalink
🎨 logging improvements (#7597)
Browse files Browse the repository at this point in the history
* 🎨  rotation config
  - every parameter is configureable
  - increase default number of files to 100
* 🎨  ghost.log location
  - example: content/logs/http___my_ghost_blog_com_ghost.log
  - user can change the path to something custom by setting logging.path
* 🛠   add response-time as dependency
* 🎨  readable PrettyStream
  - tidy up
  - generic handling (was important to support more use cases, for example: logging.info({ anyKey: anyValue }))
  - common log format
  - less code 🕵🏻
* 🎨  GhostLogger cleanup
  - remove setLoggers -> this function had too much of redundant code
  - instead: add smart this.log function
  - remove logging.request (---> GhostLogger just forwards the values, it doesn't matter if that is a request or not a request)
  - make .warn .debug .info .error small and smart
* 🎨  app.js: add response time as middleware and remove logging.request
* 🎨  setStdoutStream and setFileStream
  - redesign GhostLogger to add CustomLoggers very easily

----> Example CustomLogger

function CustomLogger(options) {
  // Base iterates over defined transports
  // EXAMPLE: ['stdout', 'elasticsearch']
  Base.call(this, options);
}
util.inherits(...);

// OVERRIDE default stdout stream and your own!!!
CustomLogger.prototype.setStdoutStream = function() {}

// add a new stream
// get's called automatically when transport elasticsearch is defined
CustomLogger.prototype.setElasticsearchStream = function() {}

* 🎨  log into multiple file by default
  - content/logs/domain.error.log --> contains only the errors
  - content/logs/domain.log --> contains everything
  - rotation for both files
* 🔥  remove logging.debug and use npm debug only
* ✨  shortcuts for mode and level
* 🎨  jshint/jscs
* 🎨  stdout as much as possible for an error
* 🎨  fix tests
* 🎨  remove req.ip from log output, remove response-time dependency
* 🎨  create middleware for logging
  - added TODO to move logging middleware to ignition
  • Loading branch information
kirrg001 authored and ErisDS committed Oct 25, 2016
1 parent ce8517f commit 0e13ef8
Show file tree
Hide file tree
Showing 10 changed files with 296 additions and 247 deletions.
15 changes: 3 additions & 12 deletions core/server/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ var debug = require('debug')('ghost:app'),

// app requires
config = require('./config'),
logging = require('./logging'),

// middleware
compress = require('compression'),
netjet = require('netjet'),

// local middleware
ghostLocals = require('./middleware/ghost-locals');
ghostLocals = require('./middleware/ghost-locals'),
logRequest = require('./middleware/log-request');

module.exports = function setupParentApp() {
debug('ParentApp setup start');
Expand All @@ -22,16 +22,7 @@ module.exports = function setupParentApp() {
// (X-Forwarded-Proto header will be checked, if present)
parentApp.enable('trust proxy');

/**
* request logging
*/
parentApp.use(function expressLogging(req, res, next) {
res.once('finish', function () {
logging.request({req: req, res: res, err: req.err});
});

next();
});
parentApp.use(logRequest);

if (debug.enabled) {
// debug keeps a timer, so this is super useful
Expand Down
7 changes: 4 additions & 3 deletions core/server/auth/passport.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var ClientPasswordStrategy = require('passport-oauth2-client-password').Strategy
BearerStrategy = require('passport-http-bearer').Strategy,
GhostOAuth2Strategy = require('passport-ghost').Strategy,
passport = require('passport'),
debug = require('debug')('ghost:auth'),
Promise = require('bluebird'),
authStrategies = require('./auth-strategies'),
utils = require('../utils'),
Expand All @@ -28,7 +29,7 @@ _private.registerClient = function (options) {
};
}

logging.debug('Update ghost client callback url...');
debug('Update ghost client callback url...');
return ghostOAuth2Strategy.changeCallbackURL({
callbackURL: url + '/ghost/',
clientId: client.get('uuid'),
Expand Down Expand Up @@ -80,7 +81,7 @@ _private.startPublicClientRegistration = function startPublicClientRegistration(
}));
}

logging.debug('Trying to register Public Client...');
debug('Trying to register Public Client...');
var timeout = setTimeout(function () {
clearTimeout(timeout);

Expand Down Expand Up @@ -119,7 +120,7 @@ exports.init = function initPassport(options) {
ghostOAuth2Strategy: ghostOAuth2Strategy,
url: utils.url.getBaseUrl()
}).then(function setClient(client) {
logging.debug('Public Client Registration was successful');
debug('Public Client Registration was successful');

ghostOAuth2Strategy.setClient(client);
passport.use(ghostOAuth2Strategy);
Expand Down
4 changes: 3 additions & 1 deletion core/server/config/defaults.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
},
"logging": {
"level": "info",
"rotation": false,
"rotation": {
"enabled": false
},
"transports": ["stdout"]
}
}
4 changes: 3 additions & 1 deletion core/server/config/env/config.production.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
},
"logging": {
"level": "info",
"rotation": true,
"rotation": {
"enabled": true
},
"transports": ["file"]
}
}
2 changes: 2 additions & 0 deletions core/server/config/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ exports.getContentPath = function getContentPath(type) {
return path.join(this.get('paths:contentPath'), 'themes/');
case 'scheduling':
return path.join(this.get('paths:contentPath'), 'scheduling/');
case 'logs':
return path.join(this.get('paths:contentPath'), 'logs/');
default:
throw new Error('getContentPath was called with: ' + type);
}
Expand Down
Loading

0 comments on commit 0e13ef8

Please sign in to comment.