Skip to content

Commit

Permalink
Remove PINOHTTP_DEP001 and PINOHTTP_DEP002 (#229)
Browse files Browse the repository at this point in the history 
* remove autoLogging.ignorePaths and autoLogging.getPath

* restore deprecations file

* restore PINOHTTP_DEP002 warning
  • Loading branch information
@Puppo
Puppo authored Jun 14, 2022
1 parent 2c35b49 commit 51e7546
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 242 deletions.
2 changes: 0 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ $ 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(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.
Expand Down
9 changes: 4 additions & 5 deletions deprecations.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,9 @@

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.')
// Warning example
// 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
22 changes: 1 addition & 21 deletions index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,26 +44,6 @@ 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;
}

export interface CustomAttributeKeys {
Expand All @@ -81,7 +61,7 @@ export interface StdSerializers {
}

export default PinoHttp;
export { PinoHttp as pinoHttp }
export { PinoHttp as pinoHttp };

export const startTime: unique symbol;

Expand Down
12 changes: 3 additions & 9 deletions index.test-d.ts
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@

/// <reference path="index.d.ts"/>

import { Writable } from 'stream';
import { IncomingMessage, ServerResponse } from 'http';
import { IncomingMessage, RequestListener, ServerResponse } from 'http';
import { Socket } from 'net';
import pino from 'pino';
import pinoHttp, { HttpLogger, ReqId, Options, GenReqId, AutoLoggingOptions, CustomAttributeKeys, StdSerializers, startTime } from '.';
import { RequestListener } from 'http';
import { Writable } from 'stream';
import pinoHttp, { AutoLoggingOptions, CustomAttributeKeys, GenReqId, HttpLogger, Options, ReqId, startTime, StdSerializers } from '.';

const logger = pino();

Expand All @@ -21,9 +20,6 @@ pinoHttp({ prettyPrint: true }); // deprecated but still present in pino.
pinoHttp({ transport: { target: 'pino-pretty', options: { colorize: true } } });
pinoHttp({ autoLogging: false });
pinoHttp({ autoLogging: { ignore: (req: IncomingMessage) => req.headers['user-agent'] === 'ELB-HealthChecker/2.0' } });
pinoHttp({ autoLogging: { ignorePaths: ['/health'] } });
pinoHttp({ autoLogging: { ignorePaths: [/\/health/] } });
pinoHttp({ autoLogging: { ignorePaths: ['/health'], getPath: (req: IncomingMessage) => req.url } });
pinoHttp({ customSuccessMessage: (req: IncomingMessage, res: ServerResponse) => 'Success' });
pinoHttp({ customErrorMessage: (req: IncomingMessage, res: ServerResponse, error: Error) => `Error - ${error}` });
pinoHttp({ customAttributeKeys: { req: 'req' } });
Expand Down Expand Up @@ -87,8 +83,6 @@ const autoLoggingOptions = (() => {
if (rand()) {
rtn = {
ignore: canBeUndefined(() => true),
ignorePaths: canBeUndefined(['str', /regex/, new RegExp('regex', 'g')]),
getPath: canBeUndefined(() => '/path'),
};
} else if (rand()) {
rtn = false;
Expand Down
29 changes: 0 additions & 29 deletions logger.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
const pino = require('pino')
const serializers = require('pino-std-serializers')
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) {
Expand Down Expand Up @@ -64,14 +62,6 @@ 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
Expand Down Expand Up @@ -151,25 +141,6 @@ function pinoLogger (opts, stream) {
res[reqObject] = req

if (autoLogging) {
if (autoLoggingIgnorePaths.length) {
let url
if (autoLoggingGetPath) {
url = URL.parse(autoLoggingGetPath(req))
} else {
url = URL.parse(req.url)
}

const isPathIgnored = autoLoggingIgnorePaths.find(ignorePath => {
if (ignorePath instanceof RegExp) {
return ignorePath.test(url.pathname)
}

return ignorePath === url.pathname
})

shouldLogSuccess = !isPathIgnored
}

if (autoLoggingIgnore !== null && shouldLogSuccess === true) {
const isIgnored = autoLoggingIgnore !== null && autoLoggingIgnore(req)
shouldLogSuccess = !isIgnored
Expand Down
176 changes: 0 additions & 176 deletions test/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,182 +464,6 @@ 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({
autoLogging: {
ignorePaths: ['/ignorethis']
}
}, dest)

setup(t, logger, function (err, server) {
t.error(err)
doGet(server, '/ignorethis', function () {
const line = dest.read()
t.equal(line, null)
t.end()
})
})
})

/**
* @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({
autoLogging: {
ignorePaths: ['/ignorethis']
}
}, dest)

setup(t, logger, function (err, server) {
t.error(err)
doGet(server, '/shouldlogthis')
})

dest.on('data', function (line) {
t.pass('path should log')
t.end()
})
})

/**
* @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({
autoLogging: {
ignorePaths: ['/ignorethis'],
getPath: function (req) {
return req.url
}
}
}, dest)

setup(t, logger, function (err, server) {
t.error(err)
doGet(server, '/ignorethis', function () {
const line = dest.read()
t.equal(line, null)
t.end()
})
})
})

/**
* @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({
autoLogging: {
ignorePaths: ['/ignorethis'],
getPath: function (req) {
return req.url
}
}
}, dest)

setup(t, logger, function (err, server) {
t.error(err)
doGet(server, '/shouldlogthis')
})

dest.on('data', function (line) {
t.pass('path should log')
t.end()
})
})

/**
* @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({
autoLogging: {
ignorePaths: [/\/[A-z]{4}\/ignorethis/, '/another-ignored-path'],
getPath: function (req) {
return req.url
}
}
}, dest)

setup(t, logger, function (err, server) {
t.error(err)
doGet(server, '/abcd/ignorethis')
doGet(server, '/another-ignored-path')
doGet(server, '/abcd0/shouldlogthis')
})

dest.on('data', function (line) {
t.pass('path should log')
t.end()
})
})

/**
* @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({
Expand Down

0 comments on commit 51e7546

Please sign in to comment.