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.

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

make config.scrubFields work globally, not just for Rollbar #53

Merged
merged 2 commits into from
Jan 6, 2017

Conversation

mattjstar
Copy link
Contributor

In this pr #47 I added functionality to scrub all fields being passed through before being sent to Rollbar.

This pr extends that functionality to both the fields object (first argument in logger functions if it's an object), or the objects you might pass in to be logger messages:

// Scrubs `scrubMe` key in both use cases, assuming config.scrubFields == [ 'scrubMe' ]
log.info({ scrubMe: 'This should be ignored', tlc: 'No Scrubs' });
log.info('Testing Log', { scrubMe: 'This should be ignored', tlc: 'No Scrubs' });

@coveralls
Copy link

Coverage Status

Coverage increased (+4.4%) to 76.404% when pulling e6b03a5c72f8ba9852a7288f523387bf0b3d37fa on feature/globalscrubfields into 1acbe52 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+4.4%) to 76.404% when pulling 6345059 on feature/globalscrubfields into 1acbe52 on master.

Copy link
Contributor

@nason nason left a comment

Choose a reason for hiding this comment

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

Sweeeet

* https://github.com/trentm/node-bunyan#levels
* @type {Array}
*/
export const BUNYAN_LOGGER_LEVELS = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Its unlikely, but we'll need to keep this in sync with any changes in bunyan. What if we pulled this from bunyan itself:

import { levelFromName } from 'node-bunyan';

...

export const BUNYAN_LOGGER_LEVELS = Object.keys(levelFromName)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh nice, yeah I like this better

@@ -99,8 +99,41 @@ describe('we-js-logger', () => {
it.skip('accepts custom serializers', () => {});
});

describe('config.scrubFields', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Dope. Can we test this with a log child as well here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

totally

Copy link
Contributor

@i8ramin i8ramin left a comment

Choose a reason for hiding this comment

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

dope

@coveralls
Copy link

Coverage Status

Coverage increased (+4.7%) to 76.667% when pulling 87687fc on feature/globalscrubfields into 1acbe52 on master.

@nason
Copy link
Contributor

nason commented Jan 6, 2017

👍 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants