You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2019/10/29 17:48:07 UTC

[kudu-CR] www: fix metrics.html

Hello Alexey Serbin, Andrew Wong,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/14570

to review the following change.


Change subject: www: fix metrics.html
......................................................................

www: fix metrics.html

This page broke a long time ago, probably when we added metric entities. I
doubt anyone uses it; it was really just there for demoing Kudu metrics. If
you really want to see what it looked like, there's a screenshot[1] from
commit 6384f42b0.

Anyway, fixing it is easy enough, though I'm going to remove it in a
follow-on commit.

1. http://cloudera-todd.s3.amazonaws.com/charts.png

Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
---
M www/metrics-epoch.js
M www/metrics.html
2 files changed, 71 insertions(+), 67 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/14570/1
-- 
To view, visit http://gerrit.cloudera.org:8080/14570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
Gerrit-Change-Number: 14570
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>

[kudu-CR] www: fix metrics.html

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14570 )

Change subject: www: fix metrics.html
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14570/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14570/1//COMMIT_MSG@14
PS1, Line 14: Anyway, fixing it is easy enough, though I'm going to remove it in a
            : follow-on commit.
> I'm not arguing that it has been useful -- I'm arguing it seems like it _ca
OK.



-- 
To view, visit http://gerrit.cloudera.org:8080/14570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
Gerrit-Change-Number: 14570
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 30 Oct 2019 00:28:23 +0000
Gerrit-HasComments: Yes

[kudu-CR] www: fix metrics.html

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14570 )

Change subject: www: fix metrics.html
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/14570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
Gerrit-Change-Number: 14570
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 31 Oct 2019 18:37:12 +0000
Gerrit-HasComments: No

[kudu-CR] www: fix metrics.html

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14570 )

Change subject: www: fix metrics.html
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14570/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14570/1//COMMIT_MSG@14
PS1, Line 14: Anyway, fixing it is easy enough, though I'm going to remove it in a
            : follow-on commit.
> /metrics.html has been broken since April 2015 when we introduced metric en
I'm not arguing that it has been useful -- I'm arguing it seems like it _can_ be useful, especially since the thirdparty tools I know don't do anything useful with our histogram metrics. Yeah, I'd prefer keeping the page around, if it doesn't get in the way of your other webserver work. That is, unless you have an argument for it not having much utility.



-- 
To view, visit http://gerrit.cloudera.org:8080/14570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
Gerrit-Change-Number: 14570
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 29 Oct 2019 23:04:28 +0000
Gerrit-HasComments: Yes

[kudu-CR] www: fix metrics.html

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/14570

to look at the new patch set (#2).

Change subject: www: fix metrics.html
......................................................................

www: fix metrics.html

It broke when we added metric entities over four years ago. If you want to
see what it looked like, there's a screenshot[1] from commit 6384f42b0.

Fixing it is easy enough, and Andrew thinks it's interesting enough to
preserve.

1. http://cloudera-todd.s3.amazonaws.com/charts.png

Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
---
M www/metrics-epoch.js
M www/metrics.html
2 files changed, 75 insertions(+), 69 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/14570/2
-- 
To view, visit http://gerrit.cloudera.org:8080/14570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
Gerrit-Change-Number: 14570
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] www: fix metrics.html

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14570 )

Change subject: www: fix metrics.html
......................................................................

www: fix metrics.html

It broke when we added metric entities over four years ago. If you want to
see what it looked like, there's a screenshot[1] from commit 6384f42b0.

Fixing it is easy enough, and Andrew thinks it's interesting enough to
preserve.

1. http://cloudera-todd.s3.amazonaws.com/charts.png

Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
Reviewed-on: http://gerrit.cloudera.org:8080/14570
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M www/metrics-epoch.js
M www/metrics.html
2 files changed, 75 insertions(+), 69 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Andrew Wong: Looks good to me, approved
  Alexey Serbin: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/14570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
Gerrit-Change-Number: 14570
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] www: fix metrics.html

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14570 )

Change subject: www: fix metrics.html
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/14570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
Gerrit-Change-Number: 14570
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 30 Oct 2019 03:50:12 +0000
Gerrit-HasComments: No

[kudu-CR] www: fix metrics.html

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14570 )

Change subject: www: fix metrics.html
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/14570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
Gerrit-Change-Number: 14570
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 31 Oct 2019 18:00:27 +0000
Gerrit-HasComments: No

[kudu-CR] www: fix metrics.html

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14570 )

Change subject: www: fix metrics.html
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14570/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14570/1//COMMIT_MSG@14
PS1, Line 14: Anyway, fixing it is easy enough, though I'm going to remove it in a
            : follow-on commit.
> As much as we generally try to rely on thirdparty software, the histogram d
/metrics.html has been broken since April 2015 when we introduced metric entities, so I don't think you can argue that it's been useful in the truest sense of the word.

But since you're interested, I fixed up the /metrics handling so that now the histograms work too. Would you like me to drop the patch that removes /metrics.html?



-- 
To view, visit http://gerrit.cloudera.org:8080/14570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
Gerrit-Change-Number: 14570
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 29 Oct 2019 22:59:25 +0000
Gerrit-HasComments: Yes

[kudu-CR] www: fix metrics.html

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14570 )

Change subject: www: fix metrics.html
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14570/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14570/1//COMMIT_MSG@14
PS1, Line 14: Anyway, fixing it is easy enough, though I'm going to remove it in a
            : follow-on commit.
As much as we generally try to rely on thirdparty software, the histogram display here seems very useful. Why are you removing it?



-- 
To view, visit http://gerrit.cloudera.org:8080/14570
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I892febb19638724dcc6d2e278a38c0e8b23930e9
Gerrit-Change-Number: 14570
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 29 Oct 2019 17:51:04 +0000
Gerrit-HasComments: Yes