Initial UI for Metrics in Observatory

Initial UI for Metrics in Observatory

- s/href/_href to silence new polymer warnings.
- Add fixed pollers that metrics can be registered with.
- UI to control sample buffer size
- UI to control refresh rate
- UI to see registered counters

R=koda@google.com

Committed: https://code.google.com/p/dart/source/detail?r=39679

[Cutch, 2014-08-08 21:33:41]

PTAL, note the first three files are stale from the deployed directory, will be
fixed before commit.

[koda, 2014-08-11 16:34:54]

First pass. Eager to see a demo!

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/app/chart.dart (right):

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/app/chart.dart:69: int
findColumnIndexWithLabel(String label) {
Is this used? Implementation suggests not.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/app/page.dart (right):

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/app/page.dart:179: for (var i = 0; i <
pollers.length; i++) {
Consider using a map keyed by period instead.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/elements/metrics.dart (right):

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.dart:1: // Copyright
(c) 2013, the Dart project authors.  Please see the AUTHORS file
2014

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.dart:106: pollPeriod
= new Duration(milliseconds: 1000);
seconds: 1

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/elements/metrics.html (right):

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.html:87: <!-- These
must be kept in sync with constants in Metric in
Why can't you just parse the value?

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.html:89: <option
value="8000">Once every 8 seconds</option>
These seem a bit verbose; consider dropping "Once".

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.html:92: <option
value="1000">Once every 1 seconds</option>
"1 seconds" -> "second"?

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.html:93: <option
value="100">Ten times per second</option>
Consider "Ten" -> 10.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.html:111: <p>Select a
metric from the list on the left.</p>
Isn't it possible to ensure that at least one item selected (defaulting to the
first, or something)?

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/service/object.dart (right):

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2238: static final
List<int> pollPeriods = [8000, 4000, 2000, 1000, 100];
Couldn't you traverse the DOM to get this list instead of hard-coding it?

If you need it, it should be const POLL_PERIODS.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2257:
samples.removeRange(0, count);
I assume ObservableList is backed by an array? If you have many large buffers,
the quadratic behavior might become costly.

At least remark that using a circular buffer is possible if needed.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2266: bool get
isGauge => (min != null) && (max != null);
The fact that 'null' is valid and has this particular meaning should also be
noted on the fields themselves, since it's not obvious (+/-inf would be another
option).

Did you consider having Counter and Gauge subclasses? (Encoding types with
special values is fine if it's internal only, but in this case, it becomes part
of the interface.)

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2281: final int
period;
Redundant? In any case, avoid untyped times (or at least add "Ms" suffix).

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2303: if
(metrics.length == 0) {
Redundant.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2307: for (var i =
0; i < metrics.length; i++) {
Use for-in when possible.

[Cutch, 2014-08-27 22:25:40]

Finally getting back to this.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/app/chart.dart (right):

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/app/chart.dart:69: int
findColumnIndexWithLabel(String label) {
On 2014/08/11 16:34:53, koda wrote:
> Is this used? Implementation suggests not.

Done.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/app/page.dart (right):

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/app/page.dart:179: for (var i = 0; i <
pollers.length; i++) {
On 2014/08/11 16:34:53, koda wrote:
> Consider using a map keyed by period instead.

Done.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/elements/metrics.dart (right):

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.dart:1: // Copyright
(c) 2013, the Dart project authors.  Please see the AUTHORS file
On 2014/08/11 16:34:53, koda wrote:
> 2014

Done.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.dart:106: pollPeriod
= new Duration(milliseconds: 1000);
On 2014/08/11 16:34:53, koda wrote:
> seconds: 1

Done.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/elements/metrics.html (right):

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.html:87: <!-- These
must be kept in sync with constants in Metric in
On 2014/08/11 16:34:53, koda wrote:
> Why can't you just parse the value?

1) Because the table is a static and polymer expressions cannot access statics
2) Because of the human readable string that goes with it (this could be a side
table) but see #1.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.html:89: <option
value="8000">Once every 8 seconds</option>
On 2014/08/11 16:34:53, koda wrote:
> These seem a bit verbose; consider dropping "Once".

Done.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.html:92: <option
value="1000">Once every 1 seconds</option>
On 2014/08/11 16:34:53, koda wrote:
> "1 seconds" -> "second"?

Done.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.html:93: <option
value="100">Ten times per second</option>
On 2014/08/11 16:34:53, koda wrote:
> Consider "Ten" -> 10.

Acknowledged.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/metrics.html:111: <p>Select a
metric from the list on the left.</p>
On 2014/08/11 16:34:53, koda wrote:
> Isn't it possible to ensure that at least one item selected (defaulting to the
> first, or something)?

Done.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/service/object.dart (right):

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2238: static final
List<int> pollPeriods = [8000, 4000, 2000, 1000, 100];
On 2014/08/11 16:34:53, koda wrote:
> Couldn't you traverse the DOM to get this list instead of hard-coding it?
> 

The service library doesn't import dart:html. Also, traversing the DOM requires
waiting for Polymer to "settle" and then traverse- which is heavy. I've moved
this into the MetricsPage (it didn't belong here) but am still not traversing
the DOM.

> If you need it, it should be const POLL_PERIODS.

Done.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2257:
samples.removeRange(0, count);
On 2014/08/11 16:34:54, koda wrote:
> I assume ObservableList is backed by an array? If you have many large buffers,
> the quadratic behavior might become costly.
> 
> At least remark that using a circular buffer is possible if needed.

Done.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2266: bool get
isGauge => (min != null) && (max != null);
On 2014/08/11 16:34:53, koda wrote:
> The fact that 'null' is valid and has this particular meaning should also be
> noted on the fields themselves, since it's not obvious (+/-inf would be
another
> option).
> 

Noted.

> Did you consider having Counter and Gauge subclasses? (Encoding types with
> special values is fine if it's internal only, but in this case, it becomes
part
> of the interface.)

Yes, I've considered it and may do this in the future. For now the distinction
is a minor one- proper gauge support will come in a follow up CL and at that
point I may make distinct Counter and Gauge types.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2281: final int
period;
On 2014/08/11 16:34:54, koda wrote:
> Redundant? In any case, avoid untyped times (or at least add "Ms" suffix).

Redundant. Removed.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2303: if
(metrics.length == 0) {
On 2014/08/11 16:34:53, koda wrote:
> Redundant.

Done.

https://codereview.chromium.org/457803002/diff/60001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/service/object.dart:2307: for (var i =
0; i < metrics.length; i++) {
On 2014/08/11 16:34:53, koda wrote:
> Use for-in when possible.

Done.

[Cutch, 2014-08-28 18:52:57]

PTAL

[koda, 2014-08-28 20:04:53]

lgtm

[Cutch, 2014-08-28 21:17:28]

Committed patchset #8 (id:130001) manually as 39679 (presubmit successful).