Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added TransportStream option levelOnly as per request… #103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

added TransportStream option levelOnly as per request… #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import * as logform from 'logform';
declare class TransportStream extends stream.Writable {
public format?: logform.Format;
public level?: string;
public levelOnly?: boolean;
public silent?: boolean;
public handleExceptions?: boolean;
public handleRejections?: boolean;
Expand All @@ -26,6 +27,7 @@ declare namespace TransportStream {
interface TransportStreamOptions {
format?: logform.Format;
level?: string;
levelOnly?: boolean;
silent?: boolean;
handleExceptions?: boolean;
handleRejections?: boolean;
Expand Down
95 changes: 58 additions & 37 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const { LEVEL } = require('triple-beam');
* that all `winston >= 3` transports should inherit from.
* @param {Object} options - Options for this TransportStream instance
* @param {String} options.level - Highest level according to RFC5424.
* @param {String} options.levelOnly - If true, will ONLY log info
* with a log level equal to this level, nothing less or more than. default = false
* @param {Boolean} options.handleExceptions - If true, info with
* { exception: true } will be written.
* @param {Function} options.log - Custom log function for simple Transport
Expand All @@ -20,6 +22,7 @@ const TransportStream = module.exports = function TransportStream(options = {})

this.format = options.format;
this.level = options.level;
this.levelOnly = (options.levelOnly === true); // prevoius behaviour = not defined ~= false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.levelOnly = (options.levelOnly === true); // prevoius behaviour = not defined ~= false
this.levelOnly = (options.levelOnly === true); // previous behaviour = not defined ~= false

this.handleExceptions = options.handleExceptions;
this.handleRejections = options.handleRejections;
this.silent = options.silent;
Expand Down Expand Up @@ -58,6 +61,55 @@ const TransportStream = module.exports = function TransportStream(options = {})
*/
util.inherits(TransportStream, Writable);

/**
* Returns true if the info log level means it should be logged
* @param {mixed} info - TODO: add param description.
* @returns {boolean} - TODO: add returns description.
Comment on lines +66 to +67
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR leaves some TODOs open here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These TODOs are inherited from the original method that I extracted the code from. I just split the method out to fix the worst eslint issues that were breaking the npm run test. I didn't want to assume what the descriptions are as I'm not super familiar with the code and project nomenclature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had looked in the diff for that but didn't see a corresponding line being moved from elsewhere. Where was it extracted from?
In any case, those TODOs are added automatically by some JSDoc support tools in IDE packages. With the comment in the line above, these two lines can probably just be removed.

* @private
*/
TransportStream.prototype._infoLevelLoggable = function _infoLevelLoggable(info) {
// Remark: This has to be handled in the base transport now because we
// cannot conditionally write to our pipe targets as stream. We always
// prefer any explicit level set on the Transport itself falling back to
// any level set on the parent.
// We always prefer any explicit level set on the Transport itself
// falling back to any level set on the parent.
const level = this.level || (this.parent && this.parent.level);

return (
!level ||
(!this.levelOnly && this.levels[level] >= this.levels[info[LEVEL]]) ||
(this.levelOnly && this.levels[level] === this.levels[info[LEVEL]])
);
};

/**
* trap any errors generated by the user-provided format
* @param {mixed} info - TODO: add param description.
* @returns {object} - TODO: add returns description.
* @private
*/
TransportStream.prototype._transformCatchErrorState = function _transformCatchErrorState(info) {
let errState;
let transformed;

// We trap(and re-throw) any errors generated by the user-provided format, but also
// guarantee that the streams callback is invoked so that we can continue flowing.
try {
transformed = this.format.transform(
Object.assign({}, info),
this.format.options
);
} catch (err) {
errState = err;
}

return {
transformed,
errState
};
};

/**
* Writes the info object to our transport instance.
* @param {mixed} info - TODO: add param description.
Expand All @@ -71,27 +123,14 @@ TransportStream.prototype._write = function _write(info, enc, callback) {
return callback(null);
}

// Remark: This has to be handled in the base transport now because we
// cannot conditionally write to our pipe targets as stream. We always
// prefer any explicit level set on the Transport itself falling back to
// any level set on the parent.
const level = this.level || (this.parent && this.parent.level);

if (!level || this.levels[level] >= this.levels[info[LEVEL]]) {
if (this._infoLevelLoggable(info)) {
if (info && !this.format) {
return this.log(info, callback);
}

let errState;
let transformed;

// We trap(and re-throw) any errors generated by the user-provided format, but also
// trap(and re-throw) any errors generated by the user-provided format, but also
// guarantee that the streams callback is invoked so that we can continue flowing.
try {
transformed = this.format.transform(Object.assign({}, info), this.format.options);
} catch (err) {
errState = err;
}
const { transformed, errState } = this._transformCatchErrorState(info);

if (errState || !transformed) {
// eslint-disable-next-line callback-return
Expand Down Expand Up @@ -135,19 +174,9 @@ TransportStream.prototype._writev = function _writev(chunks, callback) {
continue;
}

let errState;
let transformed;

// We trap(and re-throw) any errors generated by the user-provided format, but also
// trap(and re-throw) any errors generated by the user-provided format, but also
// guarantee that the streams callback is invoked so that we can continue flowing.
try {
transformed = this.format.transform(
Object.assign({}, chunks[i].chunk),
this.format.options
);
} catch (err) {
errState = err;
}
const { transformed, errState } = this._transformCatchErrorState(chunks[i].chunk);

if (errState || !transformed) {
// eslint-disable-next-line callback-return
Expand Down Expand Up @@ -180,16 +209,8 @@ TransportStream.prototype._accept = function _accept(write) {
return false;
}

// We always prefer any explicit level set on the Transport itself
// falling back to any level set on the parent.
const level = this.level || (this.parent && this.parent.level);

// Immediately check the average case: log level filtering.
if (
info.exception === true ||
!level ||
this.levels[level] >= this.levels[info[LEVEL]]
) {
if (info.exception === true || this._infoLevelLoggable(info)) {
// Ensure the info object is valid based on `{ exception }`:
// 1. { handleExceptions: true }: all `info` objects are valid
// 2. { exception: false }: accepted by all transports.
Expand Down
20 changes: 20 additions & 0 deletions test/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,26 @@ describe('TransportStream', () => {

expected.forEach(transport.write.bind(transport));
});

it('should only log messages with exact priority level when levelOnly=true', done => {
const allMessages = testOrder.map(levelAndMessage);
const expected = allMessages.filter((message) => message.level === 'info');
const transport = new TransportStream({
level: 'info',
levelOnly: true,
log: logFor(expected.length, (err, infos) => {
if (err) {
return done(err);
}
assume(infos.length).equals(expected.length);
assume(infos).deep.equals(expected.slice());
done();
})
});

transport.levels = testLevels;
allMessages.forEach(transport.write.bind(transport));
});
});
});

Expand Down