Metric-ify SystemHealth metrics

Metric-ify SystemHealth metrics.

SystemHealthMetrics is a registered metric that just calls ResponsivenessMetric
and EfficiencyMetric.
ResponsivenessMetric calls AnimationThroughput/SmoothnessMetric before computing
responsiveness for each UserExpectation.
Each metric adds one ScalarNumericValue for each supported UserExpectation.
By "supported", I mean, for example, that IdleExpectations do not afford the
concept of responsiveness; only AnimationExpectations afford throughput and
smoothness.

Each metric value contains groupingKeys "userExpectation" (UE.stableId),
"userExpectationStageTitle", and "userExpectationInitiatorTitle".

The metrics throw in cases that should never happen, e.g. if there are no
frame events associated with an AnimationExpectation,
or if a UE is neither Response/Animation/Idle/Load.

This CL does not plumb the new metrics into any UI.

See also https://codereview.chromium.org/1741843002

Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/ed08f0f1f07a6d16e3626c3a197499763f9b76c4

[benjhayden, 2016-02-26 18:12:22]

benjhayden@chromium.org changed reviewers:
+ eakuefner@chromium.org, nduca@chromium.org

[benjhayden, 2016-02-26 18:12:44]

Description was changed from

==========
Metric-ify SystemHealth metrics

BUG=catapult:#todo
==========

to

==========
Metric-ify SystemHealth metrics

See also https://codereview.chromium.org/1741843002

BUG=catapult:#todo
==========

[benjhayden, 2016-02-26 19:01:01]

Description was changed from

==========
Metric-ify SystemHealth metrics

See also https://codereview.chromium.org/1741843002

BUG=catapult:#todo
==========

to

==========
Metric-ify SystemHealth metrics

See also https://codereview.chromium.org/1741843002
==========

[eakuefner, 2016-02-27 00:45:49]

https://codereview.chromium.org/1730313007/diff/20001/telemetry/telemetry/val...
File telemetry/telemetry/value/translate_common_values.py (right):

https://codereview.chromium.org/1730313007/diff/20001/telemetry/telemetry/val...
telemetry/telemetry/value/translate_common_values.py:10: return
failure.FailureValue.FromMessage(page, mre_failure.stack)
rebase?

[eakuefner, 2016-03-01 20:25:34]

https://codereview.chromium.org/1730313007/diff/20001/tracing/tracing/metrics...
File tracing/tracing/metrics/system_health/efficiency_metric.html (right):

https://codereview.chromium.org/1730313007/diff/20001/tracing/tracing/metrics...
tracing/tracing/metrics/system_health/efficiency_metric.html:23: if (score ===
undefined)
What can cause this score (and the others) to be undefined?

[benjhayden, 2016-03-01 20:59:23]

https://codereview.chromium.org/1730313007/diff/20001/tracing/tracing/metrics...
File tracing/tracing/metrics/system_health/efficiency_metric.html (right):

https://codereview.chromium.org/1730313007/diff/20001/tracing/tracing/metrics...
tracing/tracing/metrics/system_health/efficiency_metric.html:23: if (score ===
undefined)
On 2016/03/01 at 20:25:34, eakuefner wrote:
> What can cause this score (and the others) to be undefined?

Good question! Commented.

[eakuefner, 2016-03-02 19:33:09]

lgtm, but could you throw instead of returning undefined? That will cause an MRE
failure to get inserted automatically.

[benjhayden, 2016-03-02 23:52:27]

Description was changed from

==========
Metric-ify SystemHealth metrics

See also https://codereview.chromium.org/1741843002
==========

to

==========
Metric-ify SystemHealth metrics.

This CL wires up the existing SystemHealth, Responsiveness, and Efficiency
metrics
to the new TBMv2 MetricRegistry.

See also https://codereview.chromium.org/1741843002
==========

[nduca, 2016-03-03 00:31:11]

https://codereview.chromium.org/1730313007/diff/60001/tracing/tracing/metrics...
File tracing/tracing/metrics/system_health/system_health_metric.html (right):

https://codereview.chromium.org/1730313007/diff/60001/tracing/tracing/metrics...
tracing/tracing/metrics/system_health/system_health_metric.html:22: var score =
SystemHealthMetric.forModel(model);
might want to push the throw into the SystemHealthmetric itself. It seems like a
metric should be clear about either "i'm giving you a score, or i couldn't and
its an error"

https://codereview.chromium.org/1730313007/diff/60001/tracing/tracing/metrics...
tracing/tracing/metrics/system_health/system_health_metric.html:30:
valueList.addValue(new tr.v.NumericValue(
instead of reporting a single number overall, i think we might want to consider
reporting two numbers:
- responsiveness
- efficiency

the idea of an aggregate number for SH is something that i think is longer term.

[benjhayden, 2016-03-03 00:40:21]

https://codereview.chromium.org/1730313007/diff/60001/tracing/tracing/metrics...
File tracing/tracing/metrics/system_health/system_health_metric.html (right):

https://codereview.chromium.org/1730313007/diff/60001/tracing/tracing/metrics...
tracing/tracing/metrics/system_health/system_health_metric.html:22: var score =
SystemHealthMetric.forModel(model);
On 2016/03/03 at 00:31:10, nduca wrote:
> might want to push the throw into the SystemHealthmetric itself. It seems like
a metric should be clear about either "i'm giving you a score, or i couldn't and
its an error"

There are cases where a trace is useful and usable even though there are some
UEs that don't support one or more metrics. E.g. if flow events are not traced,
or if cpuSelfTime is not traced, then input-based Responses/Animations will not
be able to compute efficiency. Or if there is only one long IdleExpectation,
then we can't compute Responsiveness.
The side panel and subview call metric.forModel/forExpectation directly.
Should the side panel and subview catch and ignore such exceptions?
Can this be addressed in another CL?

https://codereview.chromium.org/1730313007/diff/60001/tracing/tracing/metrics...
tracing/tracing/metrics/system_health/system_health_metric.html:30:
valueList.addValue(new tr.v.NumericValue(
On 2016/03/03 at 00:31:10, nduca wrote:
> instead of reporting a single number overall, i think we might want to
consider reporting two numbers:
> - responsiveness
> - efficiency
> 
> the idea of an aggregate number for SH is something that i think is longer
term.

The benchmark does report responsiveness and efficiency.
https://codereview.chromium.org/1741843002
I want to poke at the benchmark a bit more before landing it.
But I can make the benchmark not report overall sh, and just report
responsiveness and efficiency for now if you want.

[nduca, 2016-03-03 00:43:26]

oh i think the way this is hooked up is not how i expected this to be hooked up.

i expected to see system_health_metric that outputs responsiveness and
efficiency.

whereas it looks like you have two metrics. so this suggests to me have a
conceptual issue we need to debug.

[benjhayden, 2016-03-04 18:34:33]

On 2016/03/03 at 00:43:26, nduca wrote:
> oh i think the way this is hooked up is not how i expected this to be hooked
up.
> 
> i expected to see system_health_metric that outputs responsiveness and
efficiency.
> 
> whereas it looks like you have two metrics. so this suggests to me have a
conceptual issue we need to debug.

I'll try to summarize what Nat and I discussed yesterday, and he can correct me.

It sounds like he doesn't actually care whether
Responsiveness/EfficiencyMetric.forModel throws vs returns undefined.
I can see an argument for eventually making these helper methods throw, but I
think there are reasons for keeping them non-flammable for now.

The purpose of the old RAILScore object, which was carried over into the current
SystemHealthMetric, was to help devs figure out what to do when they improve one
metric but degrade another. It effectively automated analysis like "At these
levels of responsiveness and efficiency, this metric is this much more
important, so it's a net win if you improve it this much more than you degrade
the other one."
That is a very opinionated algorithm. We did not have the data nor expertise to
exercise those opinions.
In the near term, we want to only report the metrics themselves (responsiveness,
efficiency, memory, etc.), not an opinionated weightedMean like the current
SystemHealthMetric. When devs improve one but degrade another, we will connect
them with experts who will help them evaluate their changes on a case-by-case
basis. Perhaps in a year, we might have enough experience evaluating and
resolving such conflicts that we can think about re-introducing an opinionated
weightedMean to automatically help devs balance metrics.

So, we don't need SystemHealthMetric as it currently is, nor system-health-span.
SystemHealthMetric is also used in perf-insights, so I'll chat with Ethan about
how thoroughly perf-insights is going away.

Nat had been thinking that the benchmark would specify a single tbmv2 metric,
which would add multiple values to the valueList. So I can change
SystemHealthMetric to SystemHealthMetric*s*, which would not compute anything
itself, but would just call ResponsivenessMetric(valueList, model),
EfficiencyMetric(valueList, model), MemoryMetric(valueList, model), etc.
I'm not sure how this will work with the MetricRegistry, which calls all
registered metrics. So if SHMs calls RM, EM, MM, etc. then they would need to be
imported, so they would be registered, so MetricRegistry would run them as well
as SHMs. I guess SHMs could be an empty file that just imports those other
metrics, or maybe MetricRegistry could take a list of metric names instead of
files to import? Then it would need to always import all metrics, but that
doesn't seem like a problem?

Something like the current system-health-side-panel might intersect Ned's ideas
for a metric debugger. I'll chat with him.

[eakuefner, 2016-03-04 20:22:52]

On 2016/03/04 at 18:34:33, benjhayden wrote:
> On 2016/03/03 at 00:43:26, nduca wrote:
> > oh i think the way this is hooked up is not how i expected this to be hooked
up.
> > 
> > i expected to see system_health_metric that outputs responsiveness and
efficiency.
> > 
> > whereas it looks like you have two metrics. so this suggests to me have a
conceptual issue we need to debug.
> 
> I'll try to summarize what Nat and I discussed yesterday, and he can correct
me.
> 
> It sounds like he doesn't actually care whether
Responsiveness/EfficiencyMetric.forModel throws vs returns undefined.
> I can see an argument for eventually making these helper methods throw, but I
think there are reasons for keeping them non-flammable for now.
> 
> The purpose of the old RAILScore object, which was carried over into the
current SystemHealthMetric, was to help devs figure out what to do when they
improve one metric but degrade another. It effectively automated analysis like
"At these levels of responsiveness and efficiency, this metric is this much more
important, so it's a net win if you improve it this much more than you degrade
the other one."
> That is a very opinionated algorithm. We did not have the data nor expertise
to exercise those opinions.
> In the near term, we want to only report the metrics themselves
(responsiveness, efficiency, memory, etc.), not an opinionated weightedMean like
the current SystemHealthMetric. When devs improve one but degrade another, we
will connect them with experts who will help them evaluate their changes on a
case-by-case basis. Perhaps in a year, we might have enough experience
evaluating and resolving such conflicts that we can think about re-introducing
an opinionated weightedMean to automatically help devs balance metrics.
> 
> So, we don't need SystemHealthMetric as it currently is, nor
system-health-span. SystemHealthMetric is also used in perf-insights, so I'll
chat with Ethan about how thoroughly perf-insights is going away.
> 
> Nat had been thinking that the benchmark would specify a single tbmv2 metric,
which would add multiple values to the valueList. So I can change
SystemHealthMetric to SystemHealthMetric*s*, which would not compute anything
itself, but would just call ResponsivenessMetric(valueList, model),
EfficiencyMetric(valueList, model), MemoryMetric(valueList, model), etc.
> I'm not sure how this will work with the MetricRegistry, which calls all
registered metrics. So if SHMs calls RM, EM, MM, etc. then they would need to be
imported, so they would be registered, so MetricRegistry would run them as well
as SHMs. I guess SHMs could be an empty file that just imports those other
metrics, or maybe MetricRegistry could take a list of metric names instead of
files to import? Then it would need to always import all metrics, but that
doesn't seem like a problem?
> 
> Something like the current system-health-side-panel might intersect Ned's
ideas for a metric debugger. I'll chat with him.

Update: Ben talked to Ned and I offline and now there are 6 post-its on my desk
with bugs I mean to file and assign variously to myself, Ned, and Ben.

[nduca, 2016-03-04 22:05:24]

This is all very exciting! Glad you two are talking. I think we're headed
somewhere great. Ben, do you feel like you have concrete steps forward here?

[benjhayden, 2016-03-04 22:21:22]

On 2016/03/04 at 22:05:24, nduca wrote:
> This is all very exciting! Glad you two are talking. I think we're headed
somewhere great. Ben, do you feel like you have concrete steps forward here?

Yeah, I think I understand how these metrics should work now. We'll see if I do.

[benjhayden, 2016-03-08 23:23:04]

The CQ bit was checked by benjhayden@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-08 23:23:13]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1730313007/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1730313007/140001

[benjhayden, 2016-03-08 23:28:01]

Description was changed from

==========
Metric-ify SystemHealth metrics.

This CL wires up the existing SystemHealth, Responsiveness, and Efficiency
metrics
to the new TBMv2 MetricRegistry.

See also https://codereview.chromium.org/1741843002
==========

to

==========
Metric-ify SystemHealth metrics.

SystemHealthMetrics is a registered metric that just calls ResponsivenessMetric
and EfficiencyMetric.
ResponsivenessMetric calls AnimationThroughput/SmoothnessMetric before computing
responsiveness for each UserExpectation.
Each metric adds one ScalarNumericValue for each supported UserExpectation.
By "supported", I mean, for example, that IdleExpectations do not afford the
concept of responsiveness; only AnimationExpectations afford throughput and
smoothness.

Each metric value contains groupingKeys "userExpectation" (UE.stableId),
"userExpectationStageTitle", and "userExpectationInitiatorTitle".

This CL does not plumb the new metrics into any UI.

See also https://codereview.chromium.org/1741843002
==========

[benjhayden, 2016-03-08 23:29:30]

Description was changed from

==========
Metric-ify SystemHealth metrics.

SystemHealthMetrics is a registered metric that just calls ResponsivenessMetric
and EfficiencyMetric.
ResponsivenessMetric calls AnimationThroughput/SmoothnessMetric before computing
responsiveness for each UserExpectation.
Each metric adds one ScalarNumericValue for each supported UserExpectation.
By "supported", I mean, for example, that IdleExpectations do not afford the
concept of responsiveness; only AnimationExpectations afford throughput and
smoothness.

Each metric value contains groupingKeys "userExpectation" (UE.stableId),
"userExpectationStageTitle", and "userExpectationInitiatorTitle".

This CL does not plumb the new metrics into any UI.

See also https://codereview.chromium.org/1741843002
==========

to

==========
Metric-ify SystemHealth metrics.

SystemHealthMetrics is a registered metric that just calls ResponsivenessMetric
and EfficiencyMetric.
ResponsivenessMetric calls AnimationThroughput/SmoothnessMetric before computing
responsiveness for each UserExpectation.
Each metric adds one ScalarNumericValue for each supported UserExpectation.
By "supported", I mean, for example, that IdleExpectations do not afford the
concept of responsiveness; only AnimationExpectations afford throughput and
smoothness.

Each metric value contains groupingKeys "userExpectation" (UE.stableId),
"userExpectationStageTitle", and "userExpectationInitiatorTitle".

The metrics throw in cases that should never happen, e.g. if there are no
frame events associated with an AnimationExpectation, or if a UE is neither
Response/Animation/Idle/Load.

This CL does not plumb the new metrics into any UI.

See also https://codereview.chromium.org/1741843002
==========

[benjhayden, 2016-03-08 23:29:38]

Description was changed from

==========
Metric-ify SystemHealth metrics.

SystemHealthMetrics is a registered metric that just calls ResponsivenessMetric
and EfficiencyMetric.
ResponsivenessMetric calls AnimationThroughput/SmoothnessMetric before computing
responsiveness for each UserExpectation.
Each metric adds one ScalarNumericValue for each supported UserExpectation.
By "supported", I mean, for example, that IdleExpectations do not afford the
concept of responsiveness; only AnimationExpectations afford throughput and
smoothness.

Each metric value contains groupingKeys "userExpectation" (UE.stableId),
"userExpectationStageTitle", and "userExpectationInitiatorTitle".

The metrics throw in cases that should never happen, e.g. if there are no
frame events associated with an AnimationExpectation, or if a UE is neither
Response/Animation/Idle/Load.

This CL does not plumb the new metrics into any UI.

See also https://codereview.chromium.org/1741843002
==========

to

==========
Metric-ify SystemHealth metrics.

SystemHealthMetrics is a registered metric that just calls ResponsivenessMetric
and EfficiencyMetric.
ResponsivenessMetric calls AnimationThroughput/SmoothnessMetric before computing
responsiveness for each UserExpectation.
Each metric adds one ScalarNumericValue for each supported UserExpectation.
By "supported", I mean, for example, that IdleExpectations do not afford the
concept of responsiveness; only AnimationExpectations afford throughput and
smoothness.

Each metric value contains groupingKeys "userExpectation" (UE.stableId),
"userExpectationStageTitle", and "userExpectationInitiatorTitle".

The metrics throw in cases that should never happen, e.g. if there are no
frame events associated with an AnimationExpectation,
or if a UE is neither Response/Animation/Idle/Load.

This CL does not plumb the new metrics into any UI.

See also https://codereview.chromium.org/1741843002
==========

[benjhayden, 2016-03-08 23:29:46]

PTAL

[commit-bot: I haz the power, 2016-03-08 23:37:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-08 23:37:47]

Dry run: This issue passed the CQ dry run.

[benjhayden, 2016-03-11 01:39:47]

The CQ bit was checked by benjhayden@chromium.org

[benjhayden, 2016-03-11 01:39:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
eakuefner@chromium.org
Link to the patchset: https://codereview.chromium.org/1730313007/#ps160001
(title: "rebase")

[commit-bot: I haz the power, 2016-03-11 01:39:56]

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

[commit-bot: I haz the power, 2016-03-11 01:54:33]

Description was changed from

==========
Metric-ify SystemHealth metrics.

SystemHealthMetrics is a registered metric that just calls ResponsivenessMetric
and EfficiencyMetric.
ResponsivenessMetric calls AnimationThroughput/SmoothnessMetric before computing
responsiveness for each UserExpectation.
Each metric adds one ScalarNumericValue for each supported UserExpectation.
By "supported", I mean, for example, that IdleExpectations do not afford the
concept of responsiveness; only AnimationExpectations afford throughput and
smoothness.

Each metric value contains groupingKeys "userExpectation" (UE.stableId),
"userExpectationStageTitle", and "userExpectationInitiatorTitle".

The metrics throw in cases that should never happen, e.g. if there are no
frame events associated with an AnimationExpectation,
or if a UE is neither Response/Animation/Idle/Load.

This CL does not plumb the new metrics into any UI.

See also https://codereview.chromium.org/1741843002
==========

to

==========
Metric-ify SystemHealth metrics.

SystemHealthMetrics is a registered metric that just calls ResponsivenessMetric
and EfficiencyMetric.
ResponsivenessMetric calls AnimationThroughput/SmoothnessMetric before computing
responsiveness for each UserExpectation.
Each metric adds one ScalarNumericValue for each supported UserExpectation.
By "supported", I mean, for example, that IdleExpectations do not afford the
concept of responsiveness; only AnimationExpectations afford throughput and
smoothness.

Each metric value contains groupingKeys "userExpectation" (UE.stableId),
"userExpectationStageTitle", and "userExpectationInitiatorTitle".

The metrics throw in cases that should never happen, e.g. if there are no
frame events associated with an AnimationExpectation,
or if a UE is neither Response/Animation/Idle/Load.

This CL does not plumb the new metrics into any UI.

See also https://codereview.chromium.org/1741843002

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-03-11 01:54:34]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...