chore: deprecate autoLogging .ignorePaths and .getPaths in favor of .…

…ignore (#218)

README.md

@@ -99,8 +99,8 @@ $ node example.js | pino-pretty
 * `customLogLevel`: set to a `function (req, res, err) => { /* returns level name string */ }`. This function will be invoked to determine the level at which the log should be issued (`silent` will prevent logging). This option is mutually exclusive with the `useLevel` option. The first two arguments are the HTTP request and response. The third argument is an error object if an error has occurred in the request.
 * `autoLogging`: set to `false`, to disable the automatic "request completed" and "request errored" logging. Defaults to `true`. If set to an object, you can provide more options.
 * `autoLogging.ignore`: set to a `function (req) => { /* returns boolean */ }`. Useful for defining logic based on req properties (such as a user-agent header) to ignore successful requests.
-* `autoLogging.ignorePaths`: array that holds one or many paths that should not autolog on completion. Paths will be matched exactly to the url path `req.url` (using Node class `URL.pathname`). This is useful for ignoring e.g. health check paths that get called every X seconds, and would fill out the logs unnecessarily. If the path matches and succeeds (http 200), it will not log any text. If it fails, it will log the error (as with any other path).
-* `autoLogging.getPath`: set to a `function (req) => { /* returns path string */ }`. This function will be invoked to return the current path as a string. This is useful for checking `autoLogging.ignorePaths` against a path other than the default `req.url`. e.g. An express server where `req.originalUrl` is preferred.
+* `autoLogging.ignorePaths(deprecated in favor of autoLogging.ignore)`: array that holds one or many paths that should not autolog on completion. Paths will be matched exactly to the url path `req.url` (using Node class `URL.pathname`). This is useful for ignoring e.g. health check paths that get called every X seconds, and would fill out the logs unnecessarily. If the path matches and succeeds (http 200), it will not log any text. If it fails, it will log the error (as with any other path).
+* `autoLogging.getPath(deprecated in favor of autoLogging.ignore)`: set to a `function (req) => { /* returns path string */ }`. This function will be invoked to return the current path as a string. This is useful for checking `autoLogging.ignorePaths` against a path other than the default `req.url`. e.g. An express server where `req.originalUrl` is preferred.
 * `stream`: same as the second parameter
 * `customReceivedMessage`: set to a `function (req, res) => { /* returns message string */ }` This function will be invoked at each request received, setting "msg" property to returned string. If not set, nothing value will be used.
 * `customSuccessMessage`: set to a `function (req, res) => { /* returns message string */ }` This function will be invoked at each successful response, setting "msg" property to returned string. If not set, default value will be used.

benchmarks/http-ndjson-equivalent-server.js

@@ -10,8 +10,8 @@ const hostname = require('os').hostname()
 function handle (req, res) {
   res.end('hello world')
   const opts = {
-    pid: pid,
-    hostname: hostname,
+    pid,
+    hostname,
     level: 30,
     res: {
       statusCode: res.statusCode,

deprecations.js

@@ -0,0 +1,11 @@
+'use strict'
+
+const warning = require('process-warning')()
+
+const warnName = 'PinoHttpWarning'
+
+warning.create(warnName, 'PINOHTTP_DEP001', 'Use of "autoLogging.ignorePaths" is deprecated since version 7.1.0, use "opts.autoLogging.ignore" instead.')
+
+warning.create(warnName, 'PINOHTTP_DEP002', 'Use of "autoLogging.getPath" is deprecated since version 7.1.0, use "opts.autoLogging.ignore" instead.')
+
+module.exports = warning

index.d.ts

@@ -44,7 +44,25 @@ export interface GenReqId {
 
 export interface AutoLoggingOptions {
     ignore?: ((req: IncomingMessage) => boolean);
+    /**
+     * @deprecated since version 7.1.0, use autologging.ignore instead
+     * @example
+     * const logger = pinoHttp({
+     *   autoLogging: {
+     *     ignore: req => req.url === '/ignorethis'
+     *   }
+     * })
+     */
     ignorePaths?: Array<string | RegExp> | undefined;
+    /**
+     * @deprecated since version 7.1.0, use autologging.ignore instead
+     * @example
+     * const logger = pinoHttp({
+     *   autoLogging: {
+     *     ignore: req => req.originalUrl === '/ignorethis'
+     *   }
+     * })
+     */
     getPath?: ((req: IncomingMessage) => string | undefined) | undefined;
 }
 
@@ -74,13 +92,12 @@ declare module "http" {
       id: ReqId;
       log: pino.Logger;
     }
-  
+
     interface ServerResponse {
       err?: Error | undefined;
     }
-  
+
     interface OutgoingMessage {
       [startTime]: number;
     }
   }
-

logger.js

@@ -6,6 +6,7 @@ const getCallerFile = require('get-caller-file')
 const URL = require('fast-url-parser')
 const startTime = Symbol('startTime')
 const reqObject = Symbol('reqObject')
+const warning = require('./deprecations')
 
 function pinoLogger (opts, stream) {
   if (opts && opts._writableState) {
@@ -67,7 +68,13 @@ function pinoLogger (opts, stream) {
   const autoLogging = (opts.autoLogging !== false)
   const autoLoggingIgnore = opts.autoLogging && opts.autoLogging.ignore ? opts.autoLogging.ignore : null
   const autoLoggingIgnorePaths = (opts.autoLogging && opts.autoLogging.ignorePaths) ? opts.autoLogging.ignorePaths : []
+  if (opts.autoLogging !== undefined && opts.autoLogging.ignorePaths !== undefined) {
+    warning.emit('PINOHTTP_DEP001')
+  }
   const autoLoggingGetPath = opts.autoLogging && opts.autoLogging.getPath ? opts.autoLogging.getPath : null
+  if (opts.autoLogging !== undefined && opts.autoLogging.getPath !== undefined) {
+    warning.emit('PINOHTTP_DEP002')
+  }
   delete opts.autoLogging
 
   const receivedMessage = opts.customReceivedMessage && typeof opts.customReceivedMessage === 'function' ? opts.customReceivedMessage : undefined

package.json

@@ -8,7 +8,8 @@
     "fast-url-parser": "^1.1.3",
     "get-caller-file": "^2.0.5",
     "pino": "^7.5.0",
-    "pino-std-serializers": "^5.0.0"
+    "pino-std-serializers": "^5.0.0",
+    "process-warning": "^2.0.0"
   },
   "devDependencies": {
     "@types/node": "^17.0.0",

test/test.js

@@ -242,7 +242,7 @@ test('uses a custom genReqId function', function (t) {
     return idToTest
   }
 
-  const logger = pinoHttp({ genReqId: genReqId }, dest)
+  const logger = pinoHttp({ genReqId }, dest)
   setup(t, logger, function (err, server) {
     t.error(err)
     doGet(server)
@@ -368,6 +368,40 @@ test('no auto logging with autoLogging set to false', function (t) {
   })
 })
 
+test('should warn upon use of autLogging.ignorePaths', test => {
+  test.plan(1)
+
+  process.once('warning', warning => {
+    test.equal(warning.code, 'PINOHTTP_DEP001')
+    test.end()
+  })
+
+  pinoHttp({
+    autoLogging: {
+      ignorePaths: ['/ignorethis']
+    }
+  })
+})
+
+test('should warn upon use of autLogging.getPath', test => {
+  test.plan(1)
+
+  process.once('warning', warning => {
+    test.equal(warning.code, 'PINOHTTP_DEP002')
+    test.end()
+  })
+
+  pinoHttp({
+    autoLogging: {
+      getPath: req => req.url
+    }
+  })
+})
+
+/**
+ * @deprecated since version 7.1.0
+ * TODO: remove test when autoLogging.ignorePaths is removed from code base
+ */
 test('no auto logging with autoLogging set to true and path ignored', function (t) {
   const dest = split(JSON.parse)
   const logger = pinoHttp({
@@ -386,6 +420,10 @@ test('no auto logging with autoLogging set to true and path ignored', function (
   })
 })
 
+/**
+ * @deprecated since version 7.1.0
+ * TODO: remove test when autoLogging.ignorePaths is removed from code base
+ */
 test('auto logging with autoLogging set to true and path not ignored', function (t) {
   const dest = split(JSON.parse)
   const logger = pinoHttp({
@@ -405,6 +443,10 @@ test('auto logging with autoLogging set to true and path not ignored', function
   })
 })
 
+/**
+ * @deprecated since version 7.1.0
+ * TODO: remove test when autoLogging.ignorePaths & autoLogging.getPath are removed from code base
+ */
 test('no auto logging with autoLogging set to true and getPath result is ignored', function (t) {
   const dest = split(JSON.parse)
   const logger = pinoHttp({
@@ -426,6 +468,10 @@ test('no auto logging with autoLogging set to true and getPath result is ignored
   })
 })
 
+/**
+ * @deprecated since version 7.1.0
+ * TODO: remove test when autoLogging.ignorePaths & autoLogging.getPath are removed from code base
+ */
 test('auto logging with autoLogging set to true and getPath result is not ignored', function (t) {
   const dest = split(JSON.parse)
   const logger = pinoHttp({
@@ -448,6 +494,10 @@ test('auto logging with autoLogging set to true and getPath result is not ignore
   })
 })
 
+/**
+ * @deprecated since version 7.1.0
+ * TODO: remove test when autoLogging.ignorePaths & autoLogging.getPath are removed from code base
+ */
 test('no auto logging with autoLogging set to use regular expressions. result is ignored', function (t) {
   const dest = split(JSON.parse)
   const logger = pinoHttp({
@@ -472,6 +522,47 @@ test('no auto logging with autoLogging set to use regular expressions. result is
   })
 })
 
+/**
+ * @deprecated since version 7.1.0
+ * TODO: remove test when autoLogging.ignorePaths & autoLogging.getPath are removed from code base
+ */
+test('autoLogging set to true and path ignored', test => {
+  const dest = split(JSON.parse)
+  const logger = pinoHttp({
+    autoLogging: {
+      ignore: req => req.url === '/ignorethis'
+    }
+  }, dest)
+
+  setup(test, logger, (err, server) => {
+    test.error(err)
+    doGet(server, '/ignorethis', () => {
+      const line = dest.read()
+      test.equal(line, null)
+      test.end()
+    })
+  })
+})
+
+test('autoLogging set to true and path not ignored', test => {
+  const dest = split(JSON.parse)
+  const logger = pinoHttp({
+    autoLogging: {
+      ignore: req => req.url === '/ignorethis'
+    }
+  }, dest)
+
+  setup(test, logger, function (err, server) {
+    test.error(err)
+    doGet(server, '/shouldlogthis')
+  })
+
+  dest.on('data', function (line) {
+    test.pass('path should log')
+    test.end()
+  })
+})
+
 test('no auto logging with autoLogging set to true and ignoring a specific user-agent', function (t) {
   const dest = split(JSON.parse)
   const logger = pinoHttp({
@@ -533,7 +624,7 @@ test('support a custom instance with custom genReqId function', function (t) {
 
   const logger = pinoHttp({
     logger: pino(dest),
-    genReqId: genReqId
+    genReqId
   })
 
   setup(t, logger, function (err, server) {