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

Trace filters get additional parameter with method/sub name #195

Merged
merged 2 commits into from
Oct 30, 2015

Conversation

roonyh
Copy link
Contributor

@roonyh roonyh commented Oct 29, 2015

No description provided.

@arunoda
Copy link
Member

arunoda commented Oct 29, 2015

This looks good. Let's extend our two existing filters to write like this.

Kadira.tracer.addFilter(Kadira.Tracer.stripSensitive([ "start", "http", "email" ], 'method', 'abc'));
Kadira.tracer.addFilter(Kadira.Tracer.stripSensitive([ "start", "http", "email" ], 'sub', 'bbc'));

if(typesToStrip.length > 0 && !strippedTypes[type])
return data;

if(receiverType && receiverType!=info.type)
Copy link
Member

Choose a reason for hiding this comment

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

Add spaces before and after !=

@arunoda
Copy link
Member

arunoda commented Oct 29, 2015

This is pretty solid. Add this to stripSelectors default filter as well.

@roonyh
Copy link
Contributor Author

roonyh commented Oct 30, 2015

@arunoda, Added to stripSelectors

@arunoda
Copy link
Member

arunoda commented Oct 30, 2015

Did you add(or update) test cases for those changes?

@roonyh
Copy link
Contributor Author

roonyh commented Oct 30, 2015

Yes. Added here

@arunoda
Copy link
Member

arunoda commented Oct 30, 2015

Okay great. I'll take these in and do a release.

arunoda added a commit that referenced this pull request Oct 30, 2015
Trace filters get additional parameter with method/sub name
@arunoda arunoda merged commit 9419846 into master Oct 30, 2015
@arunoda arunoda deleted the method-sub-filters branch October 30, 2015 10:24
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.

2 participants