You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2019/07/28 19:19:10 UTC
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13941
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
IMPALA-8799: prefix Prometheus metrics with "impala_"
If the metric name does not already include "impala", we
add the prefix. This makes it easier to separate Impala metrics
from metrics from other systems that may also be present in
prometheus. It is documented as a good practice, see:
https://prometheus.io/docs/practices/naming/
Testing:
* Added a targeted unit test for the name transformation.
* Updated unit tests to reflect the new names.
* Manually inspected /metrics_prometheus output.
Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
---
M be/src/util/metrics-test.cc
M be/src/util/metrics.cc
M be/src/util/metrics.h
M tests/webserver/test_web_pages.py
4 files changed, 159 insertions(+), 119 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/13941/2
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13941/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13941/2//COMMIT_MSG@9
PS2, Line 9: start w
> nit: start with
Done
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.h
File be/src/util/metrics.h:
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.h@472
PS2, Line 472: impala
> I think I'm in favor of checking after the substitution. Do you think the a
Done.
I doubt it's an issue, it adds a bit of overhead but should be pretty efficient with tcmalloc. The previous code was doing multiple reallocations while substituting the string.
I think that would only result in an issue with prometheus namespaces if we called an impala metric something like impalathing.metric or impala#thing.metric, which would be weird.
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 21:21:11 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 6: Code-Review+2
carry lars' +2
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 21:36:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13941
to look at the new patch set (#3).
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
IMPALA-8799: prefix Prometheus metrics with "impala_"
If the metric name does not already include "impala", we
add the prefix. This makes it easier to separate Impala metrics
from metrics from other systems that may also be present in
prometheus. It is documented as a good practice, see:
https://prometheus.io/docs/practices/naming/
Testing:
* Added a targeted unit test for the name transformation.
* Updated unit tests to reflect the new names.
* Manually inspected /metrics_prometheus output.
Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
---
M be/src/util/metrics-test.cc
M be/src/util/metrics.cc
M be/src/util/metrics.h
M tests/webserver/test_web_pages.py
4 files changed, 170 insertions(+), 119 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/13941/3
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
IMPALA-8799: prefix Prometheus metrics with "impala_"
If the converted metric name does not already start with
"impala_", we add the prefix. This makes it easier to
separate Impala metrics from metrics from other systems
that may also be present in prometheus. It is documented
as a good practice, see:
https://prometheus.io/docs/practices/naming/
Testing:
* Added a targeted unit test for the name transformation.
* Updated unit tests to reflect the new names.
* Manually inspected /metrics_prometheus output.
Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Reviewed-on: http://gerrit.cloudera.org:8080/13941
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/util/metrics-test.cc
M be/src/util/metrics.cc
M be/src/util/metrics.h
M tests/webserver/test_web_pages.py
4 files changed, 165 insertions(+), 119 deletions(-)
Approvals:
Tim Armstrong: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics-test.cc
File be/src/util/metrics-test.cc:
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics-test.cc@467
PS2, Line 467: TEST_F(MetricsTest, PrometheusMetricNames) {
nit: could add a test for "an_impala_metric" and one with uppercase characters
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.h
File be/src/util/metrics.h:
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.h@472
PS2, Line 472: impala
I wonder if we should actually test for "impala_" here to make sure that all impala metrics start with "impala_"?
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.cc
File be/src/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.cc@310
PS2, Line 310: LOG(INFO)
Should this be something like VLOG(3)? Otherwise I think we log this every time, no?
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 19:24:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 29 Jul 2019 04:05:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics-test.cc
File be/src/util/metrics-test.cc:
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics-test.cc@467
PS2, Line 467: TEST_F(MetricsTest, PrometheusMetricNames) {
> nit: could add a test for "an_impala_metric" and one with uppercase charact
Done
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.h
File be/src/util/metrics.h:
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.h@472
PS2, Line 472: impala
> I wonder if we should actually test for "impala_" here to make sure that al
I started off doing that, but I was checking before other transformations so "impala-server.metric" became impala_impala_server_metric.
I added some examples to illustrate the transformation in a few cases, I think that makes it a little clearer why it is doing what it does.
I could alternatively change the implementation so that it checked if "impala_" was the prefix after it did the character substitutions. I was trying to reduce the number of string reallocations required. I also realise that I should have made some comments in the body explaining what I was trying to do there.
So I'm not really attached to the current solution, but it seemed adequate.
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.cc
File be/src/util/metrics.cc:
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.cc@310
PS2, Line 310: LOG(INFO)
> Should this be something like VLOG(3)? Otherwise I think we log this every
Oops, leftover debugging code. Just deleted it.
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 19:57:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4674/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 21:35:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4053/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 20:39:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13941/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13941/4//COMMIT_MSG@9
PS4, Line 9: impala
nit: should this be the same as the comment in the header, i.e. "impala_"? Technically it should be the converted name after substituting some special characters, but I think "impala_" is good enough.
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 21:31:40 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4054/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 22:03:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13941
to look at the new patch set (#5).
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
IMPALA-8799: prefix Prometheus metrics with "impala_"
If the converted metric name does not already start with
"impala_", we add the prefix. This makes it easier to
separate Impala metrics from metrics from other systems
that may also be present in prometheus. It is documented
as a good practice, see:
https://prometheus.io/docs/practices/naming/
Testing:
* Added a targeted unit test for the name transformation.
* Updated unit tests to reflect the new names.
* Manually inspected /metrics_prometheus output.
Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
---
M be/src/util/metrics-test.cc
M be/src/util/metrics.cc
M be/src/util/metrics.h
M tests/webserver/test_web_pages.py
4 files changed, 165 insertions(+), 119 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/13941/5
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13941/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13941/2//COMMIT_MSG@9
PS2, Line 9: include
nit: start with
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.h
File be/src/util/metrics.h:
http://gerrit.cloudera.org:8080/#/c/13941/2/be/src/util/metrics.h@472
PS2, Line 472: impala
> I started off doing that, but I was checking before other transformations s
I think I'm in favor of checking after the substitution. Do you think the additional allocations will be an issue? We could check whether the character after "impala" is a substitution character. The prometheus naming guide reads as it the "_" after the prefix was mandatory.
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 20:09:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13941/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13941/4//COMMIT_MSG@9
PS4, Line 9: impala
> nit: should this be the same as the comment in the header, i.e. "impala_"?
Done
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 21:37:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13941
to look at the new patch set (#4).
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
IMPALA-8799: prefix Prometheus metrics with "impala_"
If the metric name does not already start with "impala", we
add the prefix. This makes it easier to separate Impala metrics
from metrics from other systems that may also be present in
prometheus. It is documented as a good practice, see:
https://prometheus.io/docs/practices/naming/
Testing:
* Added a targeted unit test for the name transformation.
* Updated unit tests to reflect the new names.
* Manually inspected /metrics_prometheus output.
Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
---
M be/src/util/metrics-test.cc
M be/src/util/metrics.cc
M be/src/util/metrics.h
M tests/webserver/test_web_pages.py
4 files changed, 165 insertions(+), 119 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/13941/4
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8799: prefix Prometheus metrics with "impala "
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13941 )
Change subject: IMPALA-8799: prefix Prometheus metrics with "impala_"
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4051/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/13941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I607ee2a456c3ec2380ca996080ae497302b5b44f
Gerrit-Change-Number: 13941
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Jul 2019 20:02:08 +0000
Gerrit-HasComments: No