[Tracing] Add metadata filter

[Tracing] Add metadata filter

Metadata fields are not currently filtered. This CL adds the
filter to strip out sensitive information is needed. It also
adds a whitelist of the metadata fields that are allowed to
be captured.

BUG=560444
Committed: https://crrev.com/ecfd53476eea375a13ce1676a877a8b40f145e02
Cr-Commit-Position: refs/heads/master@{#364778}

[Zhen Wang, 2015-12-06 23:22:12]

zhenw@chromium.org changed reviewers:
+ oysteine@chromium.org, simonhatch@chromium.org

[Zhen Wang, 2015-12-06 23:22:13]

ptal

[shatch, 2015-12-07 18:01:16]

simonhatch@chromium.org changed reviewers:
+ beaudoin@chromium.org

[shatch, 2015-12-07 18:01:16]

Awesome, thanks for taking this Zhen. Wait for Oystein to take a look but lgtm

+beaudoin in case he wants to take a look

https://codereview.chromium.org/1502583003/diff/20001/content/browser/tracing...
File content/browser/tracing/tracing_controller_impl_data_sinks.cc (right):

https://codereview.chromium.org/1502583003/diff/20001/content/browser/tracing...
content/browser/tracing/tracing_controller_impl_data_sinks.cc:358: return
metadata_copy.Pass();;
nit: ;;

[Zhen Wang, 2015-12-07 21:00:41]

Description was changed from

==========
[Tracing] Add metadata filter

BUG=560444
==========

to

==========
[Tracing] Add metadata filter

Metadata fields are not currently filtered. This CL adds the
filter to strip out sensitive information is needed. It also
adds a whitelist of the metadata fields that are allowed to 
be captured.

BUG=560444
==========

[Zhen Wang, 2015-12-07 22:25:29]

https://codereview.chromium.org/1502583003/diff/20001/content/browser/tracing...
File content/browser/tracing/tracing_controller_impl_data_sinks.cc (right):

https://codereview.chromium.org/1502583003/diff/20001/content/browser/tracing...
content/browser/tracing/tracing_controller_impl_data_sinks.cc:358: return
metadata_copy.Pass();;
On 2015/12/07 18:01:16, shatch wrote:
> nit: ;;

Done.

[oystein (OOO til 10th of July), 2015-12-07 22:43:51]

Awesome, thanks!

https://codereview.chromium.org/1502583003/diff/40001/content/browser/tracing...
File content/browser/tracing/tracing_controller_impl_data_sinks.cc (right):

https://codereview.chromium.org/1502583003/diff/40001/content/browser/tracing...
content/browser/tracing/tracing_controller_impl_data_sinks.cc:309: const char*
kMetadataWhitelist[] = {
Hmm, it'd be nice if the whitelists were all living in the same spot.

How about: Instead of passing a predicate down to the TraceLog and checking for
that before invoking this filtering, what we pass down from chrome/ is a
singleton implementation of something like:

class TracingFilter {
public
  virtual  bool IsArgsWhitelisted(const char* category_group_name, const char*
event_name) = 0;
  virtual bool IsMetadataWhitelisted(const char* metadata_name) = 0;
};

Then the separate whitelists and filtering functions can all live happily
together.

[Zhen Wang, 2015-12-07 23:23:27]

https://codereview.chromium.org/1502583003/diff/40001/content/browser/tracing...
File content/browser/tracing/tracing_controller_impl_data_sinks.cc (right):

https://codereview.chromium.org/1502583003/diff/40001/content/browser/tracing...
content/browser/tracing/tracing_controller_impl_data_sinks.cc:309: const char*
kMetadataWhitelist[] = {
On 2015/12/07 22:43:51, Oystein wrote:
> Hmm, it'd be nice if the whitelists were all living in the same spot.
> 
> How about: Instead of passing a predicate down to the TraceLog and checking
for
> that before invoking this filtering, what we pass down from chrome/ is a
> singleton implementation of something like:
> 
> class TracingFilter {
> public
>   virtual  bool IsArgsWhitelisted(const char* category_group_name, const char*
> event_name) = 0;
>   virtual bool IsMetadataWhitelisted(const char* metadata_name) = 0;
> };
> 
> Then the separate whitelists and filtering functions can all live happily
> together. 

Discussed offline, will try the following way:
- TracingController asks ChromeTracingDelegate for the metadata predicate.
- ChromeTracingDelegate asks trace_event_args_whitelist for the actual  metadata
whitelist.

[Zhen Wang, 2015-12-10 00:09:11]

I have updated the CL to use tracing delegate and put the white lists together.
ptal.

[oystein (OOO til 10th of July), 2015-12-10 19:52:18]

lgtm and thanks!

[Zhen Wang, 2015-12-10 20:04:57]

zhenw@chromium.org changed reviewers:
+ sievers@chromium.org

[Zhen Wang, 2015-12-10 20:04:58]

+sievers for //content/public/browser/

[no sievers, 2015-12-11 18:23:01]

On 2015/12/10 20:04:58, Zhen Wang wrote:
> +sievers for //content/public/browser/

lgtm

[Zhen Wang, 2015-12-11 18:27:10]

The CQ bit was checked by zhenw@chromium.org

[Zhen Wang, 2015-12-11 18:27:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
simonhatch@chromium.org
Link to the patchset: https://codereview.chromium.org/1502583003/#ps80001
(title: "fix compile failure on win")

[commit-bot: I haz the power, 2015-12-11 18:28:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1502583003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1502583003/80001

[commit-bot: I haz the power, 2015-12-11 19:48:47]

Description was changed from

==========
[Tracing] Add metadata filter

Metadata fields are not currently filtered. This CL adds the
filter to strip out sensitive information is needed. It also
adds a whitelist of the metadata fields that are allowed to 
be captured.

BUG=560444
==========

to

==========
[Tracing] Add metadata filter

Metadata fields are not currently filtered. This CL adds the
filter to strip out sensitive information is needed. It also
adds a whitelist of the metadata fields that are allowed to 
be captured.

BUG=560444
==========

[commit-bot: I haz the power, 2015-12-11 19:48:48]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-12-11 19:49:40]

Description was changed from

==========
[Tracing] Add metadata filter

Metadata fields are not currently filtered. This CL adds the
filter to strip out sensitive information is needed. It also
adds a whitelist of the metadata fields that are allowed to 
be captured.

BUG=560444
==========

to

==========
[Tracing] Add metadata filter

Metadata fields are not currently filtered. This CL adds the
filter to strip out sensitive information is needed. It also
adds a whitelist of the metadata fields that are allowed to
be captured.

BUG=560444

Committed: https://crrev.com/ecfd53476eea375a13ce1676a877a8b40f145e02
Cr-Commit-Position: refs/heads/master@{#364778}
==========

[commit-bot: I haz the power, 2015-12-11 19:49:40]

Patchset 5 (id:??) landed as
https://crrev.com/ecfd53476eea375a13ce1676a877a8b40f145e02
Cr-Commit-Position: refs/heads/master@{#364778}