You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/09/17 08:35:17 UTC
[GitHub] [pulsar] poorbarcode opened a new pull request, #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
poorbarcode opened a new pull request, #17704:
URL: https://github.com/apache/pulsar/pull/17704
### Motivation
If `label_cluster` specified `cluster_123`, the expected metrics text is:
```
metrics_xxx_total{cluster="cluster_123"} 1.0
```
but the exactly metrics text is :
```
metrics_xxx_totalcluster="cluster_123"} 1.0
```
### Modifications
Make metrics text generate correct
### Documentation
- [ ] `doc-required`
- [ ] `doc-not-needed`
- [ ] `doc`
- [ ] `doc-complete`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a diff in pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on code in PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#discussion_r973878222
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGeneratorUtils.java:
##########
@@ -67,14 +68,21 @@ public static void generateSystemMetrics(SimpleTextOutputStream stream, String c
stream.write(sample.name);
if (!sample.labelNames.contains("cluster")) {
Review Comment:
Maybe we can add this `stream.write("{");` before the check `if (!sample.labelNames.contains("cluster")) {`. The symbol `{` is necessary and it's easier to understand.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a diff in pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on code in PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#discussion_r974139799
##########
pulsar-broker-common/src/test/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGeneratorUtilsTest.java:
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.stats.prometheus;
+
+import static org.testng.Assert.*;
+import io.prometheus.client.CollectorRegistry;
+import io.prometheus.client.Counter;
+import java.io.ByteArrayOutputStream;
+import java.util.Collections;
+import java.util.UUID;
+import org.testng.annotations.Test;
+
Review Comment:
Please add test group.
##########
pulsar-broker-common/src/test/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGeneratorUtilsTest.java:
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.stats.prometheus;
+
+import static org.testng.Assert.*;
+import io.prometheus.client.CollectorRegistry;
+import io.prometheus.client.Counter;
+import java.io.ByteArrayOutputStream;
+import java.util.Collections;
+import java.util.UUID;
+import org.testng.annotations.Test;
+
Review Comment:
Please add the test group.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#issuecomment-1258854347
Hi @Technoboy-
Can this PR merge? (^_^)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#discussion_r974045430
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGeneratorUtils.java:
##########
@@ -67,14 +68,21 @@ public static void generateSystemMetrics(SimpleTextOutputStream stream, String c
stream.write(sample.name);
if (!sample.labelNames.contains("cluster")) {
Review Comment:
That's a good idea. The code has been changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a diff in pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on code in PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#discussion_r973878222
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGeneratorUtils.java:
##########
@@ -67,14 +68,21 @@ public static void generateSystemMetrics(SimpleTextOutputStream stream, String c
stream.write(sample.name);
if (!sample.labelNames.contains("cluster")) {
Review Comment:
Maybe we can add this `stream.write("{");` before the check `if (!sample.labelNames.contains("cluster"))`. The symbol `{` is necessary and it's easier to understand.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode closed pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
poorbarcode closed pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
URL: https://github.com/apache/pulsar/pull/17704
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#issuecomment-1255004295
/pulsarbot rerun-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#issuecomment-1250030825
This PR should merge to the following branches:
- master
- branch-2.10
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a diff in pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on code in PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#discussion_r973878222
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGeneratorUtils.java:
##########
@@ -67,14 +68,21 @@ public static void generateSystemMetrics(SimpleTextOutputStream stream, String c
stream.write(sample.name);
if (!sample.labelNames.contains("cluster")) {
Review Comment:
Maybe we can add this `stream.write("{");` before this check `if (!sample.labelNames.contains("cluster")) {`. The char `{` is necessary and it's easier to understand.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#discussion_r974398298
##########
pulsar-broker-common/src/test/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGeneratorUtilsTest.java:
##########
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.broker.stats.prometheus;
+
+import static org.testng.Assert.*;
+import io.prometheus.client.CollectorRegistry;
+import io.prometheus.client.Counter;
+import java.io.ByteArrayOutputStream;
+import java.util.Collections;
+import java.util.UUID;
+import org.testng.annotations.Test;
+
Review Comment:
Already add test group
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a diff in pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on code in PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#discussion_r973878222
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGeneratorUtils.java:
##########
@@ -67,14 +68,21 @@ public static void generateSystemMetrics(SimpleTextOutputStream stream, String c
stream.write(sample.name);
if (!sample.labelNames.contains("cluster")) {
Review Comment:
Maybe we can add this `stream.write("{");` before the check `if (!sample.labelNames.contains("cluster")) {`. The char `{` is necessary and it's easier to understand.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on a diff in pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on code in PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#discussion_r973878222
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGeneratorUtils.java:
##########
@@ -67,14 +68,21 @@ public static void generateSystemMetrics(SimpleTextOutputStream stream, String c
stream.write(sample.name);
if (!sample.labelNames.contains("cluster")) {
Review Comment:
Maybe we can add this `stream.write("{");` before this check. The char `{` is necessary and it's easier to understand.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#issuecomment-1251215921
/pulsarbot rerun-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #17704:
URL: https://github.com/apache/pulsar/pull/17704#issuecomment-1250029648
@poorbarcode Please provide a correct documentation label for your PR.
Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nodece merged pull request #17704: [fix][metrics]wrong metrics text generated when label_cluster specified
Posted by GitBox <gi...@apache.org>.
nodece merged PR #17704:
URL: https://github.com/apache/pulsar/pull/17704
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org