Repository: pinojs/hapi-pino (js)

Issue:
proposal for a new option: ignoreFunc
Currently hapi-pino provides `ignorePaths` and `ignoreTags`. They are useful to a certain degree, but for some cases it's still impossible to silent hapi-pino for some routes.

The use case I have in mind are routes for static files (which are usually defined with `path: /foo/bar/{file*}`). I want those to be silent. I can add a tag to these routes and use that tag in `ignoreTags`. That will work for the "request completed" log, however the "request start" log will still happen (because that log is defined in the `onRequest` extension point, and at that point the route is not yet located).

I'd like to propose a 3rd ignore option:
```
options.ignoreFunc: (options, Request) => boolean
```

It would be a simple addition to the internal `isLoggingIgnored` function: if `option.ignoreFunc` is present, it will execute and return the value; else, it will  proceed with the current logic (which will check `ignorePaths` and `ignoreTags`, and also returns a boolean). 

These 3 ignore options wouldn't be compatible obviously (either use `ignorePaths`/`ignoreTags` or `ignoreFunc`).

This way hapi-pino would provide the maximum flexibility to the user. If there's interest in this feature I would make a pr.

Thanks for considering.

Pull request description:
feature: ignoreFunc (closes #112)

Patch:
diff --git a/README.md b/README.md
index 114c785..767a961 100644
--- a/README.md
+++ b/README.md
@@ -217,6 +217,23 @@ events"](#hapievents) section.
   ignoreTags: ['healthcheck']
   ```
 
+### `options.ignoreFunc: (options, request) => boolean`
+  Takes a function that receives the plugin options and the request as parameters, and returns a boolean. Logging will be disabled if the return value is `true`. Useful for scenarios where the `ignorePaths` or `ignoreTags` options can't achieve what is intended.
+
+  **Example**:
+  Do not log routes relative to static content
+  ```js
+  ignoreFunc: (options, request) => request.path.startsWith('/static')
+  ```
+
+  **Note**: if `ignoreFunc` is used, the other two options that can be used to ignore / disable logging (`ignorePaths` and `ignoreTags`) are effectively discarded. So `ignoreFunc` can be seen a more advanced option. For instance, you can easily re-implement the `ignorePaths` functionality as follows:
+
+  ```js
+  ignoreFunc: (options, request) => myIgnorePaths.include(request.path)
+  ```
+
+  (where `myIgnorePaths` would be an array with paths to be ignored).
+
 ### `options.level: Pino.Level`
   **Default**: `'info'`
 
diff --git a/index.js b/index.js
index 895dcb0..dd562c2 100644
--- a/index.js
+++ b/index.js
@@ -172,6 +172,10 @@ async function register (server, options) {
   })
 
   function isLoggingIgnored (options, request) {
+    if (typeof options.ignoreFunc === 'function') {
+      return !!options.ignoreFunc(options, request)
+    }
+
     // note: from hapi@18.0.0 the `request.url` can be undefined
     // based on that we prefer to use request.path instead of request.url.pathname
     if (options.ignorePaths && ignoreTable[request.path]) {


Test patch:
diff --git a/test.js b/test.js
index e65fe70..ca5e6a5 100644
--- a/test.js
+++ b/test.js
@@ -1596,6 +1596,47 @@ experiment('ignore request logs for tags in ignoreTags', () => {
   })
 })
 
+experiment('ignore request logs with ignoreFunc', () => {
+  test('when ignoreFunc returns true, nothing should be logged', async () => {
+    const server = getServer()
+    let resolver
+    const done = new Promise((resolve, reject) => {
+      resolver = resolve
+    })
+    const stream = sink(data => {
+      expect(data.req.url).to.endWith('/foo')
+      resolver()
+    })
+    const logger = require('pino')(stream)
+    const plugin = {
+      plugin: Pino,
+      options: {
+        instance: logger,
+        ignoreFunc: (options, request) => {
+          if (request.path === '/something') {
+            return true
+          }
+
+          return false
+        }
+      }
+    }
+
+    await server.register(plugin)
+
+    await server.inject({
+      method: 'GET',
+      url: '/something'
+    })
+
+    await server.inject({
+      method: 'PUT',
+      url: '/foo'
+    })
+    await done
+  })
+})
+
 experiment('ignore response logs for paths in ignorePaths', () => {
   test('when path matches entry in ignorePaths, nothing should be logged', async () => {
     const server = getServer()
@@ -1670,6 +1711,49 @@ experiment('ignore response logs for tags in ignoreTags', () => {
   })
 })
 
+experiment('ignore response logs with ignoreFunc', () => {
+  test('when ignoreFunc returns true, nothing should be logged', async () => {
+    const server = getServer()
+    let resolver
+    const done = new Promise((resolve, reject) => {
+      resolver = resolve
+    })
+    const stream = sink(data => {
+      expect(data.req.url).to.endWith('/foo')
+      expect(data.msg).to.equal('request completed')
+      resolver()
+    })
+    const logger = require('pino')(stream)
+    const plugin = {
+      plugin: Pino,
+      options: {
+        instance: logger,
+        logEvents: ['response'],
+        ignoreFunc: (options, request) => {
+          if (request.path === '/something') {
+            return true
+          }
+
+          return false
+        }
+      }
+    }
+
+    await server.register(plugin)
+
+    await server.inject({
+      method: 'GET',
+      url: '/something'
+    })
+
+    await server.inject({
+      method: 'PUT',
+      url: '/foo'
+    })
+    await done
+  })
+})
+
 experiment('ignore request.log logs for paths in ignorePaths', () => {
   test('when path matches entry in ignorePaths, nothing should be logged', async () => {
     const level = 'info'
@@ -1787,6 +1871,69 @@ experiment('ignore request.log logs for tags in ignoreTags', () => {
   })
 })
 
+experiment('ignore request.log logs with ignoreFunc', () => {
+  test('when ignoreFunc returns true, nothing should be logged', async () => {
+    const level = 'info'
+    const server = getServer()
+    server.route({
+      method: 'GET',
+      path: '/ignored',
+      handler: (req, h) => {
+        req.log([level], 'hello logger')
+        return 'hello world'
+      }
+    })
+
+    server.route({
+      path: '/foo',
+      method: 'GET',
+      handler: (req, h) => {
+        req.log([level], 'foo')
+        return 'foo'
+      }
+    })
+
+    let resolver
+    const done = new Promise((resolve, reject) => {
+      resolver = resolve
+    })
+    const stream = sink(data => {
+      expect(data.req.url).to.endWith('/foo')
+      expect(data.tags).to.equal([level])
+      expect(data.data).to.equal('foo')
+      resolver()
+    })
+    const logger = require('pino')(stream)
+    const plugin = {
+      plugin: Pino,
+      options: {
+        instance: logger,
+        logEvents: ['request-error'],
+        ignoreFunc: (options, request) => {
+          if (request.path === '/ignored') {
+            return true
+          }
+
+          return false
+        }
+      }
+    }
+
+    await server.register(plugin)
+
+    await server.inject({
+      method: 'GET',
+      url: '/ignored'
+    })
+
+    await server.inject({
+      method: 'GET',
+      url: '/foo'
+    })
+    await done
+  })
+})
+
 experiment('logging with logRouteTags option enabled', () => {
   test('when logRouteTags is true, tags are part of the logged object', async () => {
     const server = getServer()