Added feature to exclude entries from being logged (e.g. sensitive data) #16

benleen · 2016-09-14T10:05:06Z

Hey arb! Had discussed this with @Marsup a while back:

Additional feature to exclude log entries from making it through to being logged.

benleen · 2016-09-14T10:29:04Z

So { log: 'user', ops: '*', request: { include: ['hapi', 'foo'] } } would be the same as doing { log: 'user', ops: '*', request: ['hapi', 'foo'] }.

You get the extra exclude array, which will reject an event if it matches one of the tags, no matter what else is matched in an include.

benleen · 2016-09-14T11:01:08Z

As I added another test for completion I uncovered an existing bug which was duplicating all tags in the mapped object.
All tags were being duplicated in case the tags were an array; tags = filter > tags.concat(filter)

arb · 2016-09-14T13:22:32Z

This looks nice so far. Can you add some documentation? Also, just to verify, this doesn't looks like a breaking change...

benleen · 2016-09-14T13:32:36Z

Not a breaking change. The emphasis is still on the inclusion of tag.
Looking at the tests, I've moved to a different intermediary mapping object in the subscription but all existing filter tests remained to output the same results as before.

Will update the documentation!

benleen · 2016-09-14T13:42:57Z

@arb I did notice in your documentation (example) you actually described what I considered to be a bug:
request: [ 'hapi', 'foo', 'hapi', 'foo' ] (repetition of tags)

Was this intended?

benleen · 2016-09-14T13:53:52Z

Also, seems I'm wrong about the breaking change. :-)

It IS a breaking if we considering projects using the subscription function directly, which is now returning a different type of object (include/exclude object). Though I expect the amount of people using this is limited?

benleen · 2016-09-14T14:07:39Z

You know what, I'll make that bit backwards compatible as well. ;-)

benleen · 2016-09-14T14:29:22Z

Fully backwards compatible now, including subscription.

benleen · 2016-09-14T14:36:48Z

Readme updated.

lloydbenson · 2016-09-14T15:10:58Z

Thanks for the work @benleen

arb · 2016-09-14T15:20:29Z

Are we just rolling the dice that people aren't using "include" and "exclude" as existing filtering tags?

As far as repetition of tags... not sure what that was there for of it was a typo. That being said, duplicate tags shouldn't have any impact on the filtering right?

arb

Can you run npm outdated while you're in there and bump the versions of the dependencies.

arb · 2016-09-14T18:38:03Z

README.md

@@ -17,7 +17,7 @@ good-squeeze is a collection of small transform streams. The `Squeeze` stream is

 Creates a new Squeeze transform stream where:

- `events` an object where each key is a valid good event, and the value is a string or array of strings representing event tags. "\*" indicates no filtering. `null` and `undefined` are assumed to be "\*". Defaults to `{}`
+- `events` an object where each key is a valid good event, and the value is a string, array of strings representing event tags to include or an object defining tags to include and/or exclude. "\*" indicates all tags match. `null` and `undefined` are assumed to be "\*". Defaults to `{}`


Can you show an example of the different ways to configure these events... surrounding include and exclude would help highlight the fact that they are keywords in an object.

arb · 2016-09-14T18:43:55Z

lib/squeeze.js

+                if (subEventTags.exclude.indexOf(eventTag) > -1) {
+                    return false;
+                }
+                result = result || subEventTags.include.indexOf(eventTag) > -1 || subEventTags.include.length === 0;


Are you using include.length of 0 to mean don't filter anything and include every event, regardless of tag?

Yes, this is consistent with how the previous 'include' string array was being interpreted:
https://github.com/hapijs/good-squeeze/blob/master/lib/squeeze.js#L59

benleen · 2016-09-15T07:21:44Z

To be sure: I was talking about all tags being doubled out in the subscription result. Which seems obsolete. The validation logic will also loop through all subEventTags which means it loops twice over the same array of strings.

Will look at dependencies.

benleen · 2016-09-15T07:23:34Z

Package              Current  Wanted  Latest  Location
code                   2.3.0   2.3.0   3.0.2  good-squeeze
fast-safe-stringify    1.0.9   1.0.9   1.1.0  good-squeeze
hoek                   3.0.4   3.0.4   4.0.2  good-squeeze
lab                   10.9.0  10.9.0  11.0.1  good-squeeze

benleen · 2016-09-15T07:44:11Z

I see what you mean with people using include or exclude as filtering tags.

I do wonder if anyone was using the filter or subscription with anything else but a string or array of strings (as the tests were assuming).
Hapi tags are strings & the subscription function is also forcing the input filter into being an array of strings, no matter if the was an object.

That being said though, if we want to be semantically correct we could release it as a breaking change.

benleen · 2016-09-15T07:55:04Z

README.md

@@ -17,7 +17,7 @@ good-squeeze is a collection of small transform streams. The `Squeeze` stream is

 Creates a new Squeeze transform stream where:

- `events` an object where each key is a valid good event, and the value is a string or array of strings representing event tags. "\*" indicates no filtering. `null` and `undefined` are assumed to be "\*". Defaults to `{}`
+- `events` an object where each key is a valid good event, and the value is a string, array of strings representing event tags to include or an object defining tags to `include` and/or `exclude`. "\*" indicates all tags match. `null` and `undefined` are assumed to be "\*". Defaults to `{}`


@arb About the examples. It didn't seem right to inline them here, so as you noticed I added an example a bit down in this readme next to the other one. Should I add an example or context (similar to my initial comments) on the filter function instead?
Right now the tests are telling the story about how the subscription is interpreted.

benleen · 2016-09-15T12:12:38Z

I've updated the readme though I'm not to happy with it either.. Maybe less is more? I'm new to writing this kind of documentation, suggestions are welcome!

arb · 2016-09-15T13:44:36Z

README.md

 ## Methods

 ### `Squeeze(events, [options])`

 Creates a new Squeeze transform stream where:

- `events` an object where each key is a valid good event, and the value is a string or array of strings representing event tags. "\*" indicates no filtering. `null` and `undefined` are assumed to be "\*". Defaults to `{}`
+- `events` an object where each key is a valid good event, defaults to `{}`. The value which can be one of the following types representing tags to filter


Maybe word this something like...

events an object where each key is a valid good event and the value is one of the following:

string - a tag to include when filtering. '*' indicates no filtering.

array - an array of tags to include when filtering. [] indicates no filtering.

object - an object with the following values

include - string or array representing tag(s) to include when filtering

exclude - string or array representing tag(s) to exclude when filtering. exclude takes precedence over any include tags.

arb · 2016-09-15T13:47:25Z

I've decided to just make this a breaking change... In that case we may want to just change the API to always pass an object of include and exclude? Not sure...

Clean up the readme a little more
Bump all the outdated deps (and fix the tests)

Then this will be ready to land.

Thanks a lot!

benleen · 2016-09-15T14:16:55Z

Sounds good!

benleen · 2016-09-15T15:34:01Z

Updated dependencies.

Reason tests were updated:
hapijs/code@710aaf0#diff-88333ba11114e014e87198d04680144dR95

benleen · 2016-09-15T15:42:58Z

Should be good now. Apologies for the force pushes (just wanted to keep the commits a bit cleaner).

lock · 2020-01-09T01:49:48Z

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

benleen added 2 commits September 14, 2016 11:58

Added feature to specifically exclude entries from being logged

f188855

Cleanup test titles

a1ae66a

benleen added 2 commits September 14, 2016 12:33

Added additional test for exclude tags handling

3b4e387

Fix duplication of tags in map

405f479

Made sure the include array is optional

7736ff8

arb added the feature New functionality or improvement label Sep 14, 2016

arb added this to the 4.1.0 milestone Sep 14, 2016

arb self-assigned this Sep 14, 2016

arb suggested changes Sep 14, 2016

View reviewed changes

benleen commented Sep 15, 2016

View reviewed changes

arb reviewed Sep 15, 2016

View reviewed changes

arb added the breaking changes Change that can breaking existing code label Sep 15, 2016

Update readme

017053c

Updated dependencies & tests to match dependencies

3345f0c

arb approved these changes Sep 15, 2016

View reviewed changes

arb merged commit b92e834 into hapijs:master Sep 15, 2016

stjohnjohnson mentioned this pull request Sep 21, 2016

Bumping out of date dependencies screwdriver-cd/screwdriver#201

Merged

lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added feature to exclude entries from being logged (e.g. sensitive data) #16

Added feature to exclude entries from being logged (e.g. sensitive data) #16

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016 •

edited

benleen commented Sep 14, 2016

arb commented Sep 14, 2016 •

edited

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016

lloydbenson commented Sep 14, 2016

arb commented Sep 14, 2016

arb left a comment

arb Sep 14, 2016

arb Sep 14, 2016

benleen Sep 15, 2016

benleen commented Sep 15, 2016

benleen commented Sep 15, 2016 •

edited

benleen commented Sep 15, 2016

benleen Sep 15, 2016

benleen commented Sep 15, 2016

arb Sep 15, 2016

arb commented Sep 15, 2016 •

edited

benleen commented Sep 15, 2016

benleen commented Sep 15, 2016 •

edited

benleen commented Sep 15, 2016

lock bot commented Jan 9, 2020

Added feature to exclude entries from being logged (e.g. sensitive data) #16

Added feature to exclude entries from being logged (e.g. sensitive data) #16

Conversation

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016 • edited

benleen commented Sep 14, 2016

arb commented Sep 14, 2016 • edited

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016

benleen commented Sep 14, 2016

lloydbenson commented Sep 14, 2016

arb commented Sep 14, 2016

arb left a comment

Choose a reason for hiding this comment

arb Sep 14, 2016

Choose a reason for hiding this comment

arb Sep 14, 2016

Choose a reason for hiding this comment

benleen Sep 15, 2016

Choose a reason for hiding this comment

benleen commented Sep 15, 2016

benleen commented Sep 15, 2016 • edited

benleen commented Sep 15, 2016

benleen Sep 15, 2016

Choose a reason for hiding this comment

benleen commented Sep 15, 2016

arb Sep 15, 2016

Choose a reason for hiding this comment

arb commented Sep 15, 2016 • edited

benleen commented Sep 15, 2016

benleen commented Sep 15, 2016 • edited

benleen commented Sep 15, 2016

lock bot commented Jan 9, 2020

benleen commented Sep 14, 2016 •

edited

arb commented Sep 14, 2016 •

edited

benleen commented Sep 15, 2016 •

edited

arb commented Sep 15, 2016 •

edited

benleen commented Sep 15, 2016 •

edited