Multi-dimension rappor metrics

Multi-dimension rappor metrics

This adds support for collecting a set of flags along with a string
sample.  The two parts use seperate bits, so they can be analyzed both
independently and as correlated metrics.

BUG=

[Steven Holte, 2015-04-03 22:30:28]

holte@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Steven Holte, 2015-04-09 17:31:35]

On 2015/04/03 22:30:29, Steven Holte wrote:

Any thought interface?

[Steven Holte, 2015-04-09 17:32:08]

On 2015/04/09 17:31:35, Steven Holte wrote:
> On 2015/04/03 22:30:29, Steven Holte wrote:
> 
> Any thought interface?

*Any thoughts on the interface.

[Alexei Svitkine (slow), 2015-04-09 17:38:34]

Sorry, this slipped by my radar. I'll take a look today.

On Thu, Apr 9, 2015 at 1:32 PM, <holte@chromium.org> wrote:

> On 2015/04/09 17:31:35, Steven Holte wrote:
>
>> On 2015/04/03 22:30:29, Steven Holte wrote:
>>
>
>  Any thought interface?
>>
>
> *Any thoughts on the interface.
>
> https://codereview.chromium.org/1058333002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Alexei Svitkine (slow), 2015-04-09 19:20:31]

Hey Steve,

I guess I don't understand the use case for this - and since your CL is just in
a WIP state lacking comments, I can't seem to infer the use case from the code.

Could you give an example of what kind of thing this would be used for?

Is the idea that flags will be the index of a metric? e.g. if you have 5 url
entries, that you report each url with 0, 1, 2, 3, 4 as the value? Or something
else?

[Steven Holte, 2015-04-09 19:36:10]

On 2015/04/09 19:20:31, Alexei Svitkine wrote:
> Hey Steve,
> 
> I guess I don't understand the use case for this - and since your CL is just
in
> a WIP state lacking comments, I can't seem to infer the use case from the
code.
> 
> Could you give an example of what kind of thing this would be used for?
> 
> Is the idea that flags will be the index of a metric? e.g. if you have 5 url
> entries, that you report each url with 0, 1, 2, 3, 4 as the value? Or
something
> else?

This is mainly for things like the
ContentSettings.PermissionActions_Geolocation.*.Url type use cases, where we
want to collect the data about the domain, along with what action was taken on
it, and whether it was using HTTP or HTTPS.  In this case, the flags would
encode that extra information, by having a list of flag bits [HTTPS, GRANTED,
DISMISSED, REVOKED].  Since this API records the flags in separate bits, we can
use the correlation analysis and get both overall rates and correlations of bits
with different domains.

[Alexei Svitkine (slow), 2015-04-09 21:25:03]

API makes sense to me for the use case you mentioned, some initial comments but
there's other commenting and other things that you still need to polish.

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_pa...
File components/rappor/rappor_parameters.h (right):

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_pa...
components/rappor/rappor_parameters.h:51: // The number of flag bits this metric
contains.
Nit: Comment says bits, field name says bytes.

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
File components/rappor/rappor_service.h (right):

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
components/rappor/rappor_service.h:38: SB_STRING_W_FLAGS_TYPE,
Please comment these - also I don't understand this - what is the "W"? is it
"WITH" or does it stand for something else? I actually suggest spelling out the
acronyms.

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
components/rappor/rappor_service.h:75: RapporType type,
Nit: Align and add a comment.

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
components/rappor/rappor_service.h:77: uint64_t flags);
Are you planning to add rappor.xml mechanism for specifying the flags values?

[Steven Holte, 2015-04-14 20:07:38]

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_pa...
File components/rappor/rappor_parameters.h (right):

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_pa...
components/rappor/rappor_parameters.h:51: // The number of flag bits this metric
contains.
On 2015/04/09 21:25:03, Alexei Svitkine wrote:
> Nit: Comment says bits, field name says bytes.

Done.

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
File components/rappor/rappor_service.h (right):

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
components/rappor/rappor_service.h:38: SB_STRING_W_FLAGS_TYPE,
On 2015/04/09 21:25:03, Alexei Svitkine wrote:
> Please comment these - also I don't understand this - what is the "W"? is it
> "WITH" or does it stand for something else? I actually suggest spelling out
the
> acronyms.

Renamed, this and the other metrics, to be more consistent, and understandable.

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
components/rappor/rappor_service.h:75: RapporType type,
On 2015/04/09 21:25:03, Alexei Svitkine wrote:
> Nit: Align and add a comment.

Done.

https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
components/rappor/rappor_service.h:77: uint64_t flags);
On 2015/04/09 21:25:03, Alexei Svitkine wrote:
> Are you planning to add rappor.xml mechanism for specifying the flags values?

Done.

[lgarron, 2015-04-17 22:52:09]

On 2015/04/14 at 20:07:38, holte wrote:
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_pa...
> File components/rappor/rappor_parameters.h (right):
> 
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_pa...
> components/rappor/rappor_parameters.h:51: // The number of flag bits this
metric contains.
> On 2015/04/09 21:25:03, Alexei Svitkine wrote:
> > Nit: Comment says bits, field name says bytes.
> 
> Done.
> 
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
> File components/rappor/rappor_service.h (right):
> 
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
> components/rappor/rappor_service.h:38: SB_STRING_W_FLAGS_TYPE,
> On 2015/04/09 21:25:03, Alexei Svitkine wrote:
> > Please comment these - also I don't understand this - what is the "W"? is it
> > "WITH" or does it stand for something else? I actually suggest spelling out
the
> > acronyms.
> 
> Renamed, this and the other metrics, to be more consistent, and
understandable.
> 
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
> components/rappor/rappor_service.h:75: RapporType type,
> On 2015/04/09 21:25:03, Alexei Svitkine wrote:
> > Nit: Align and add a comment.
> 
> Done.
> 
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
> components/rappor/rappor_service.h:77: uint64_t flags);
> On 2015/04/09 21:25:03, Alexei Svitkine wrote:
> > Are you planning to add rappor.xml mechanism for specifying the flags
values?
> 
> Done.

What's the timeline on this change being landed and usable for new metrics?

Enamel has been punting on creating some RAPPOR stats in hopes of
cross-correlation being ready (otherwise, we have to create lots of them). We
could at least use this CL, if it's ready.

[Alexei Svitkine (slow), 2015-04-20 17:20:03]

I think we're going with an alternative API that Steve implemented here:
https://codereview.chromium.org/1090683003/

On Fri, Apr 17, 2015 at 6:52 PM, <lgarron@chromium.org> wrote:

> On 2015/04/14 at 20:07:38, holte wrote:
>
>
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_pa...
>
>> File components/rappor/rappor_parameters.h (right):
>>
>
>
>
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_pa...
>
>> components/rappor/rappor_parameters.h:51: // The number of flag bits this
>>
> metric contains.
>
>> On 2015/04/09 21:25:03, Alexei Svitkine wrote:
>> > Nit: Comment says bits, field name says bytes.
>>
>
>  Done.
>>
>
>
>
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
>
>> File components/rappor/rappor_service.h (right):
>>
>
>
>
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
>
>> components/rappor/rappor_service.h:38: SB_STRING_W_FLAGS_TYPE,
>> On 2015/04/09 21:25:03, Alexei Svitkine wrote:
>> > Please comment these - also I don't understand this - what is the "W"?
>> is it
>> > "WITH" or does it stand for something else? I actually suggest spelling
>> out
>>
> the
>
>> > acronyms.
>>
>
>  Renamed, this and the other metrics, to be more consistent, and
>>
> understandable.
>
>
>
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
>
>> components/rappor/rappor_service.h:75: RapporType type,
>> On 2015/04/09 21:25:03, Alexei Svitkine wrote:
>> > Nit: Align and add a comment.
>>
>
>  Done.
>>
>
>
>
>
https://codereview.chromium.org/1058333002/diff/1/components/rappor/rappor_se...
>
>> components/rappor/rappor_service.h:77: uint64_t flags);
>> On 2015/04/09 21:25:03, Alexei Svitkine wrote:
>> > Are you planning to add rappor.xml mechanism for specifying the flags
>>
> values?
>
>  Done.
>>
>
> What's the timeline on this change being landed and usable for new metrics?
>
> Enamel has been punting on creating some RAPPOR stats in hopes of
> cross-correlation being ready (otherwise, we have to create lots of them).
> We
> could at least use this CL, if it's ready.
>
> https://codereview.chromium.org/1058333002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Steven Holte, 2015-04-28 19:05:02]

Closing in favor of: https://codereview.chromium.org/1090683003/