Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1927723002:
Add a metrics table side panel (Closed)
|
Created:
4 years, 7 months ago by benjhayden Modified:
4 years, 7 months ago CC:
catapult-reviews_chromium.org, tracing-review_chromium.org Base URL:
https://github.com/catapult-project/catapult.git@master Target Ref:
refs/heads/master Project:
catapult Visibility:
Public. |
DescriptionAdd a metrics table side panel
Currently, the only metrics that are visible in trace viewer are a few specific
per-UE system health metrics in single-user-expectation-sub-view.
We need to be able to visualize all properties of all metric values
for any given trace.
This CL adds a simple metrics table side panel.
It's far from perfect, but it's usable.
This is a tiny step towards TBMv2.1 and results2.html.
There will be many follow-up CLs.
BUG=catapult:#2094,catapult:#2035
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/eab9231779a7225b90dd7b2b39766aae89c75477
Patch Set 1 #Patch Set 2 : . #
Total comments: 33
Patch Set 3 : comments #
Total comments: 5
Patch Set 4 : remove diagnosticValues subRows #Patch Set 5 : rename value-set-view #Patch Set 6 : clean up #
Messages
Total messages: 35 (18 generated)
Description was changed from ========== Metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics. We need to be able to see all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. There seems to be a fundamental tension between grouping keys and diagnostic values. BUG=catapult:#2094 ========== to ========== Metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics in single-user-expectation-sub-view. We need to be able to visualize all properties of all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. There seems to be a fundamental tension between grouping keys and diagnostic values. I'm not sure how to group values both by grouping key, and show the relationship between diagnostic values and their owning value. This CL ignores grouping keys altogether. There ought to be a metrics-table widget that can be used both by the side panel and the debugger app. Another CL or this one? BUG=catapult:#2094 ==========
benjhayden@chromium.org changed reviewers: + eakuefner@chromium.org, nednguyen@google.com
nednguyen@google.com changed reviewers: + petrcermak@chromium.org
We haven't settled down how to properly surface the diagnostic value yet. However, my high level architecture comment is we should split the 'tr-ui-sp-metrics-side-panel' to at least 2 ui components: 1st is <tr-ui-sp-metrics-side-panel> 2nd is <tr-ui-results-table> The reason is that in the near future, we will use <tr-ui-results-table> to build results.html version 2 of telemetry. Let's make a design doc on this. memory-infra@ probably also has some opinions on how we would surface tbmv2 values in about://tracing
Patchset #2 (id:20001) has been deleted
Patchset #2 (id:40001) has been deleted
Patchset #2 (id:60001) has been deleted
Patchset #2 (id:80001) has been deleted
Patchset #2 (id:100001) has been deleted
Description was changed from ========== Metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics in single-user-expectation-sub-view. We need to be able to visualize all properties of all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. There seems to be a fundamental tension between grouping keys and diagnostic values. I'm not sure how to group values both by grouping key, and show the relationship between diagnostic values and their owning value. This CL ignores grouping keys altogether. There ought to be a metrics-table widget that can be used both by the side panel and the debugger app. Another CL or this one? BUG=catapult:#2094 ========== to ========== Metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics in single-user-expectation-sub-view. We need to be able to visualize all properties of all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. There seems to be a fundamental tension between grouping keys and diagnostic values. I'm not sure how to group values both by grouping key, and show the relationship between diagnostic values and their owning value. This CL ignores grouping keys altogether. When diagnostic values are reconciled with grouping keys, the results-table should use a grouping-table. BUG=catapult:#2094 ==========
PTAL
Looks good overall. Here are my comments. Also, please change the patch title to "Add a metrics table side panel". Thanks, Petr https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... File tracing/tracing/ui/results_table.html (right): https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:9: <link rel="import" href="/tracing/value/numeric.html"> I don't think you need this import. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:40: (function() { For future-proof-ness, I suggest using |tr.exportTo|: tr.exportTo('tr.ui', function() { Polymer('tr-ui-results-table', { ... }); return {}; }); https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:50: var columns = [ Opinion: I don't think there's any value in storing the list in a local variable when you don't modify it. I suggest you just do: createColumns_: function() { return [ ... ]; } Alternatively, since the columns are always the same, you could just turn them into a (class) constant. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:65: nit: unless intentional, please remove blank line https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:100: (results.pairs.values.length === 0)) { nit: Odd indentation; I'd suggest either aligning vertically with "!" (4 spaces indent) or change the second condition to |!results.pairs.values.length|. Also, there's no need for parentheses around the second condition. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:105: function handleValueDict(value) { maybe "convertValueDictToRows" to be more descriptive? https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:107: if (value.diagnostics && value.diagnostics.values) { nit: no need for braces https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... File tracing/tracing/ui/side_panel/metrics_side_panel.html (right): https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:10: <link rel="import" href="/tracing/ui/side_panel/side_panel.html"> you should import event_set.html, metric_registry.html and dom_helpers.html https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:38: (function() { ditto regarding using |tr.exportTo|. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:47: if (m.constructor.name === 'sampleMetric') return; I think the code is more readable with the body of the if statement on a separate line (still no need for braces). https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:67: return 'Metrics'; nit: invalid indentation (+1 space) https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:74: reason: 'No stats were collected for this file.' Isn't the reason "No model was provided" or something like that? https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:96: if (!model) return; ditto if-statement body on a separate line. Two further issues: 1) There's no need for this check because you do it inside updateContents_ on line 114 anyway. 2) Don't you need to update the UI anyway (i.e. clear it)? https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:105: set selection(selection) { There's no value in hiding this.selection_ behind a getter and setter in my opinion (it just hinders performance and adds more code). Please consider using this.selection instead. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:110: // Not applicable to metrics nit: missing period. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:114: if (!this.model_) return; As I mentioned earlier, don't you need to update the UI anyway (clear it)? +ditto regarding line breaks
Description was changed from ========== Metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics in single-user-expectation-sub-view. We need to be able to visualize all properties of all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. There seems to be a fundamental tension between grouping keys and diagnostic values. I'm not sure how to group values both by grouping key, and show the relationship between diagnostic values and their owning value. This CL ignores grouping keys altogether. When diagnostic values are reconciled with grouping keys, the results-table should use a grouping-table. BUG=catapult:#2094 ========== to ========== Add a metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics in single-user-expectation-sub-view. We need to be able to visualize all properties of all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. There seems to be a fundamental tension between grouping keys and diagnostic values. I'm not sure how to group values both by grouping key, and show the relationship between diagnostic values and their owning value. This CL ignores grouping keys altogether. When diagnostic values are reconciled with grouping keys, the results-table should use a grouping-table. BUG=catapult:#2094 ==========
Thanks for the great comments! This doesn't reflect any of the discussion that we've been having around TBMv2.1, but most of it won't need to change much as we start to get TBMv2.1 working. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... File tracing/tracing/ui/results_table.html (right): https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:9: <link rel="import" href="/tracing/value/numeric.html"> On 2016/05/03 at 10:10:09, petrcermak wrote: > I don't think you need this import. Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:40: (function() { On 2016/05/03 at 10:10:09, petrcermak wrote: > For future-proof-ness, I suggest using |tr.exportTo|: > > tr.exportTo('tr.ui', function() { > > Polymer('tr-ui-results-table', { > ... > }); > > return {}; > }); Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:50: var columns = [ On 2016/05/03 at 10:10:09, petrcermak wrote: > Opinion: I don't think there's any value in storing the list in a local variable when you don't modify it. I suggest you just do: > > createColumns_: function() { > return [ > ... > ]; > } > > Alternatively, since the columns are always the same, you could just turn them into a (class) constant. Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:65: On 2016/05/03 at 10:10:09, petrcermak wrote: > nit: unless intentional, please remove blank line Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:100: (results.pairs.values.length === 0)) { On 2016/05/03 at 10:10:09, petrcermak wrote: > nit: Odd indentation; I'd suggest either aligning vertically with "!" (4 spaces indent) or change the second condition to |!results.pairs.values.length|. Also, there's no need for parentheses around the second condition. Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:105: function handleValueDict(value) { On 2016/05/03 at 10:10:09, petrcermak wrote: > maybe "convertValueDictToRows" to be more descriptive? Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:107: if (value.diagnostics && value.diagnostics.values) { On 2016/05/03 at 10:10:09, petrcermak wrote: > nit: no need for braces Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... File tracing/tracing/ui/side_panel/metrics_side_panel.html (right): https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:10: <link rel="import" href="/tracing/ui/side_panel/side_panel.html"> On 2016/05/03 at 10:10:10, petrcermak wrote: > you should import event_set.html, metric_registry.html and dom_helpers.html Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:38: (function() { On 2016/05/03 at 10:10:10, petrcermak wrote: > ditto regarding using |tr.exportTo|. Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:47: if (m.constructor.name === 'sampleMetric') return; On 2016/05/03 at 10:10:09, petrcermak wrote: > I think the code is more readable with the body of the if statement on a separate line (still no need for braces). Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:67: return 'Metrics'; On 2016/05/03 at 10:10:10, petrcermak wrote: > nit: invalid indentation (+1 space) Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:74: reason: 'No stats were collected for this file.' On 2016/05/03 at 10:10:09, petrcermak wrote: > Isn't the reason "No model was provided" or something like that? Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:96: if (!model) return; On 2016/05/03 at 10:10:10, petrcermak wrote: > ditto if-statement body on a separate line. > > Two further issues: > > 1) There's no need for this check because you do it inside updateContents_ on line 114 anyway. > 2) Don't you need to update the UI anyway (i.e. clear it)? Done * 3. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:105: set selection(selection) { On 2016/05/03 at 10:10:10, petrcermak wrote: > There's no value in hiding this.selection_ behind a getter and setter in my opinion (it just hinders performance and adds more code). Please consider using this.selection instead. The getter and setter need to be overridden in order to prevent the base class from throwing 'Not implemented', so I changed this implementation to just a comment 'Not applicable'. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:110: // Not applicable to metrics On 2016/05/03 at 10:10:10, petrcermak wrote: > nit: missing period. Done. https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:114: if (!this.model_) return; On 2016/05/03 at 10:10:10, petrcermak wrote: > As I mentioned earlier, don't you need to update the UI anyway (clear it)? > > +ditto regarding line breaks Done * 2.
Thanks for the great comments! This doesn't reflect any of the discussion that we've been having around TBMv2.1, but most of it won't need to change much as we start to get TBMv2.1 working.
On 2016/05/05 21:47:32, benjhayden_chromium wrote: > Thanks for the great comments! > > This doesn't reflect any of the discussion that we've been having around > TBMv2.1, but most of it won't need to change much as we start to get TBMv2.1 > working. I just patch your C. Seems like there is bug with memory metrics? http://imgur.com/KiT0Pwu
https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/sid... File tracing/tracing/ui/side_panel/metrics_side_panel.html (right): https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:123: metric.constructor(valueList, this.model_); I think we better use: metricMapFunction(result, model, options) here. For now option can be something we hard coded. https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metr...
LGTM with one more nit and +1 to Ned's comment. Thanks, Petr https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... File tracing/tracing/ui/side_panel/metrics_side_panel.html (right): https://codereview.chromium.org/1927723002/diff/120001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:105: set selection(selection) { On 2016/05/05 21:47:31, benjhayden_chromium wrote: > On 2016/05/03 at 10:10:10, petrcermak wrote: > > There's no value in hiding this.selection_ behind a getter and setter in my > opinion (it just hinders performance and adds more code). Please consider using > this.selection instead. > > The getter and setter need to be overridden in order to prevent the base class > from throwing 'Not implemented', so I changed this implementation to just a > comment 'Not applicable'. Makes sense. Thanks for the explanation :-) https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/res... File tracing/tracing/ui/results_table.html (right): https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:104: if (value.diagnostics && value.diagnostics.values) nit: You should have braces here (more than one line in the if-statement body) https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/sid... File tracing/tracing/ui/side_panel/metrics_side_panel.html (right): https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:123: metric.constructor(valueList, this.model_); On 2016/05/06 01:14:49, nednguyen wrote: > I think we better use: metricMapFunction(result, model, options) here. For now > option can be something we hard coded. > > https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metr... +1
nduca@chromium.org changed reviewers: + nduca@chromium.org
didn't we agree that this is part of tbmv2.1 and that we'd do results.html and figure out dashboard diagnostics, and then come back to the side panel when we had it figured out? I do believe the assertion that much of this will get used in the other places, but I am doing that on gut instinct rather than technical understanding. In an ideal world, we would all decide how we want all the gui use cases to be architected --- e.g. we take the diagrams on the whiteboards and then start labeling the components and show how they all releate. When we have a shared vision, we can then go ahead and start landing code. But, that I thought we agreeed we'd do when Ethan was back, by doing another big whiteboarding athon. Or, did I miss something along the way?
On 2016/05/06 at 17:28:05, nduca wrote: > didn't we agree that this is part of tbmv2.1 and that we'd do results.html and figure out dashboard diagnostics, and then come back to the side panel when we had it figured out? > > I do believe the assertion that much of this will get used in the other places, but I am doing that on gut instinct rather than technical understanding. In an ideal world, we would all decide how we want all the gui use cases to be architected --- e.g. we take the diagrams on the whiteboards and then start labeling the components and show how they all releate. When we have a shared vision, we can then go ahead and start landing code. > > But, that I thought we agreeed we'd do when Ethan was back, by doing another big whiteboarding athon. > > Or, did I miss something along the way? This is an incremental step to improve metric debugging. This may or may not be used by results2.html and/or the dashboard. Should I rename results-table to value-set-view? Before or after ValueList is renamed to ValueSet?
PTAL Making metrics-side-panel use metricMapFunction required moving MreResult, Failure, and FunctionHandle to tracing. https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/res... File tracing/tracing/ui/results_table.html (right): https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/res... tracing/tracing/ui/results_table.html:104: if (value.diagnostics && value.diagnostics.values) On 2016/05/06 at 09:33:56, petrcermak wrote: > nit: You should have braces here (more than one line in the if-statement body) This code was removed because it does not fit in TBMv2.1.
Description was changed from ========== Add a metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics in single-user-expectation-sub-view. We need to be able to visualize all properties of all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. There seems to be a fundamental tension between grouping keys and diagnostic values. I'm not sure how to group values both by grouping key, and show the relationship between diagnostic values and their owning value. This CL ignores grouping keys altogether. When diagnostic values are reconciled with grouping keys, the results-table should use a grouping-table. BUG=catapult:#2094 ========== to ========== Add a metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics in single-user-expectation-sub-view. We need to be able to visualize all properties of all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. This is a tiny step towards TBMv2.1 and results2.html. There will be many follow-up CLs. BUG=catapult:#2094 ==========
On 2016/05/11 22:52:38, benjhayden_chromium wrote: > PTAL > > Making metrics-side-panel use metricMapFunction required moving MreResult, > Failure, and FunctionHandle to tracing. Can you file a bug on this & cc Oysteine? He is working on killing perf_insights/ folder > > https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/res... > File tracing/tracing/ui/results_table.html (right): > > https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/res... > tracing/tracing/ui/results_table.html:104: if (value.diagnostics && > value.diagnostics.values) > On 2016/05/06 at 09:33:56, petrcermak wrote: > > nit: You should have braces here (more than one line in the if-statement body) > > This code was removed because it does not fit in TBMv2.1.
Description was changed from ========== Add a metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics in single-user-expectation-sub-view. We need to be able to visualize all properties of all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. This is a tiny step towards TBMv2.1 and results2.html. There will be many follow-up CLs. BUG=catapult:#2094 ========== to ========== Add a metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics in single-user-expectation-sub-view. We need to be able to visualize all properties of all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. This is a tiny step towards TBMv2.1 and results2.html. There will be many follow-up CLs. BUG=catapult:#2094,catapult:#2035 ==========
lgtm https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/sid... File tracing/tracing/ui/side_panel/metrics_side_panel.html (right): https://codereview.chromium.org/1927723002/diff/140001/tracing/tracing/ui/sid... tracing/tracing/ui/side_panel/metrics_side_panel.html:123: metric.constructor(valueList, this.model_); On 2016/05/06 09:33:57, petrcermak wrote: > On 2016/05/06 01:14:49, nednguyen wrote: > > I think we better use: metricMapFunction(result, model, options) here. For now > > option can be something we hard coded. > > > > > https://github.com/catapult-project/catapult/blob/master/tracing/tracing/metr... > > +1 Sorry, looks like the metricMapFunction may no longer be relevant in the future after talking to Ethan. Fine to keep this as-is.
Patchset #6 (id:200001) has been deleted
Patchset #6 (id:220001) has been deleted
Patch 5 lgtm % comments we discussed in person.
The CQ bit was checked by benjhayden@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from petrcermak@chromium.org, eakuefner@chromium.org, nednguyen@google.com Link to the patchset: https://codereview.chromium.org/1927723002/#ps240001 (title: "clean up")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1927723002/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1927723002/240001
Message was sent while issue was closed.
Description was changed from ========== Add a metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics in single-user-expectation-sub-view. We need to be able to visualize all properties of all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. This is a tiny step towards TBMv2.1 and results2.html. There will be many follow-up CLs. BUG=catapult:#2094,catapult:#2035 ========== to ========== Add a metrics table side panel Currently, the only metrics that are visible in trace viewer are a few specific per-UE system health metrics in single-user-expectation-sub-view. We need to be able to visualize all properties of all metric values for any given trace. This CL adds a simple metrics table side panel. It's far from perfect, but it's usable. This is a tiny step towards TBMv2.1 and results2.html. There will be many follow-up CLs. BUG=catapult:#2094,catapult:#2035 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapu... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:240001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapu... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
