You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by "HattoriHenzo (via GitHub)" <gi...@apache.org> on 2023/07/27 16:53:24 UTC
[PR] [Issue #4201] Do some code optimization.[metrics] (eventmesh)
HattoriHenzo opened a new pull request, #4296:
URL: https://github.com/apache/eventmesh/pull/4296
<!--
### Contribution Checklist
- Name the pull request in the form "[ISSUE #XXXX] Title of the pull request",
where *XXXX* should be replaced by the actual issue number.
Skip *[ISSUE #XXXX]* if there is no associated github issue for this pull request.
- Fill out the template below to describe the changes contributed by the pull request.
That will give reviewers the context they need to do the review.
- Each pull request should address only one issue.
Please do not mix up code from multiple issues.
- Each commit in the pull request should have a meaningful commit message.
- Once all items of the checklist are addressed, remove the above text and this checklist,
leaving only the filled out template below.
(The sections below can be removed for hotfixes of typos)
-->
<!--
(If this PR fixes a GitHub issue, please add `Fixes #<XXX>` or `Closes #<XXX>`.)
-->
Fixes https://github.com/apache/eventmesh/issues/4201
### Motivation
Refactoring
### Modifications
Better variable initialization
--
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: dev-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on code in PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#discussion_r1277670963
##########
eventmesh-metrics-plugin/eventmesh-metrics-prometheus/src/main/java/org/apache/eventmesh/metrics/prometheus/metrics/PrometheusExporter.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.eventmesh.metrics.prometheus.metrics;
+
+import static org.apache.eventmesh.metrics.prometheus.utils.PrometheusExporterUtils.observeOfValue;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.Meter;
+
+public abstract class PrometheusExporter<T> {
+
+ /**
+ * Map structure : [metric name, description of name] -> the method of get corresponding metric.
+ */
+ protected final Map<String[], Function<T, Number>> paramPairs = new HashMap<>();
+
+ protected abstract String getMetricName(String[] metricInfo);
+
+ protected abstract String getMetricDescription(String[] metricInfo);
Review Comment:
I I mean method `getMetricDescription`, not method `getMetricName`.
##########
eventmesh-metrics-plugin/eventmesh-metrics-prometheus/src/main/java/org/apache/eventmesh/metrics/prometheus/metrics/PrometheusExporter.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.eventmesh.metrics.prometheus.metrics;
+
+import static org.apache.eventmesh.metrics.prometheus.utils.PrometheusExporterUtils.observeOfValue;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.Meter;
+
+public abstract class PrometheusExporter<T> {
+
+ /**
+ * Map structure : [metric name, description of name] -> the method of get corresponding metric.
+ */
+ protected final Map<String[], Function<T, Number>> paramPairs = new HashMap<>();
+
+ protected abstract String getMetricName(String[] metricInfo);
+
+ protected abstract String getMetricDescription(String[] metricInfo);
Review Comment:
I mean method `getMetricDescription`, not method `getMetricName`.
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "HattoriHenzo (via GitHub)" <gi...@apache.org>.
HattoriHenzo commented on code in PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#discussion_r1276988533
##########
eventmesh-metrics-plugin/eventmesh-metrics-prometheus/src/main/java/org/apache/eventmesh/metrics/prometheus/metrics/PrometheusExporter.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.eventmesh.metrics.prometheus.metrics;
+
+import static org.apache.eventmesh.metrics.prometheus.utils.PrometheusExporterUtils.observeOfValue;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.Meter;
+
+public abstract class PrometheusExporter<T> {
+
+ /**
+ * Map structure : [metric name, description of name] -> the method of get corresponding metric.
+ */
+ protected final Map<String[], Function<T, Number>> paramPairs = new HashMap<>();
+
+ protected abstract String getMetricName(String[] metricInfo);
+
+ protected abstract String getMetricDescription(String[] metricInfo);
+
+ protected abstract String getProtocol();
+
+ public void export(final String meterName, final Metric metric) {
Review Comment:
@pandaapo This is a good point. Because of the `paramPairs` hashMap variable used in this method, setting it as `static` will force the variable to be static too. Also, each implementation has to initialise his own `paramPairs`. For simplicity I choose not to set it `static`.
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "HattoriHenzo (via GitHub)" <gi...@apache.org>.
HattoriHenzo commented on code in PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#discussion_r1277714049
##########
eventmesh-metrics-plugin/eventmesh-metrics-prometheus/src/main/java/org/apache/eventmesh/metrics/prometheus/metrics/PrometheusExporter.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.eventmesh.metrics.prometheus.metrics;
+
+import static org.apache.eventmesh.metrics.prometheus.utils.PrometheusExporterUtils.observeOfValue;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.Meter;
+
+public abstract class PrometheusExporter<T> {
+
+ /**
+ * Map structure : [metric name, description of name] -> the method of get corresponding metric.
+ */
+ protected final Map<String[], Function<T, Number>> paramPairs = new HashMap<>();
+
+ protected abstract String getMetricName(String[] metricInfo);
+
+ protected abstract String getMetricDescription(String[] metricInfo);
Review Comment:
@pandaapo it is a good point, I will take a look on it.
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "HattoriHenzo (via GitHub)" <gi...@apache.org>.
HattoriHenzo commented on code in PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#discussion_r1278112702
##########
eventmesh-metrics-plugin/eventmesh-metrics-prometheus/src/main/java/org/apache/eventmesh/metrics/prometheus/metrics/PrometheusExporter.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.eventmesh.metrics.prometheus.metrics;
+
+import static org.apache.eventmesh.metrics.prometheus.utils.PrometheusExporterUtils.observeOfValue;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.Meter;
+
+public abstract class PrometheusExporter<T> {
+
+ /**
+ * Map structure : [metric name, description of name] the map variable to get corresponding metric
Review Comment:
@pandaapo I have modified the comment by removing the `->`. The text remain globally the same.
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on code in PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#discussion_r1276954584
##########
eventmesh-metrics-plugin/eventmesh-metrics-prometheus/src/main/java/org/apache/eventmesh/metrics/prometheus/metrics/PrometheusExporter.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.eventmesh.metrics.prometheus.metrics;
+
+import static org.apache.eventmesh.metrics.prometheus.utils.PrometheusExporterUtils.observeOfValue;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.Meter;
+
+public abstract class PrometheusExporter<T> {
+
+ /**
+ * Map structure : [metric name, description of name] -> the method of get corresponding metric.
+ */
+ protected final Map<String[], Function<T, Number>> paramPairs = new HashMap<>();
+
+ protected abstract String getMetricName(String[] metricInfo);
+
+ protected abstract String getMetricDescription(String[] metricInfo);
Review Comment:
This method can be written as a specific method.
##########
eventmesh-metrics-plugin/eventmesh-metrics-prometheus/src/main/java/org/apache/eventmesh/metrics/prometheus/metrics/PrometheusExporter.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.eventmesh.metrics.prometheus.metrics;
+
+import static org.apache.eventmesh.metrics.prometheus.utils.PrometheusExporterUtils.observeOfValue;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.Meter;
+
+public abstract class PrometheusExporter<T> {
+
+ /**
+ * Map structure : [metric name, description of name] -> the method of get corresponding metric.
+ */
+ protected final Map<String[], Function<T, Number>> paramPairs = new HashMap<>();
+
+ protected abstract String getMetricName(String[] metricInfo);
+
+ protected abstract String getMetricDescription(String[] metricInfo);
+
+ protected abstract String getProtocol();
+
+ public void export(final String meterName, final Metric metric) {
Review Comment:
Can this method be defined as a static method?
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "HattoriHenzo (via GitHub)" <gi...@apache.org>.
HattoriHenzo commented on code in PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#discussion_r1276986245
##########
eventmesh-metrics-plugin/eventmesh-metrics-prometheus/src/main/java/org/apache/eventmesh/metrics/prometheus/metrics/PrometheusExporter.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.eventmesh.metrics.prometheus.metrics;
+
+import static org.apache.eventmesh.metrics.prometheus.utils.PrometheusExporterUtils.observeOfValue;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.Meter;
+
+public abstract class PrometheusExporter<T> {
+
+ /**
+ * Map structure : [metric name, description of name] -> the method of get corresponding metric.
+ */
+ protected final Map<String[], Function<T, Number>> paramPairs = new HashMap<>();
+
+ protected abstract String getMetricName(String[] metricInfo);
+
+ protected abstract String getMetricDescription(String[] metricInfo);
Review Comment:
@pandaapo These methods are implemented by each `PrometheusXXXExporter ` classes because in some cases a concatenation is made with the `metricInfo` parameter.
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on code in PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#discussion_r1277725046
##########
eventmesh-metrics-plugin/eventmesh-metrics-prometheus/src/main/java/org/apache/eventmesh/metrics/prometheus/metrics/PrometheusExporter.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.eventmesh.metrics.prometheus.metrics;
+
+import static org.apache.eventmesh.metrics.prometheus.utils.PrometheusExporterUtils.observeOfValue;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.Meter;
+
+public abstract class PrometheusExporter<T> {
+
+ /**
+ * Map structure : [metric name, description of name] the map variable to get corresponding metric
Review Comment:
I think the modified comments should also make it easy for developers to see the structure of this map, just like the original comments `[metric name, description of name] -> the method of get corresponding metric` intended to do.
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "Alonexc (via GitHub)" <gi...@apache.org>.
Alonexc commented on PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#issuecomment-1657376925
please check code style.
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "Alonexc (via GitHub)" <gi...@apache.org>.
Alonexc commented on PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#issuecomment-1654860183
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "HattoriHenzo (via GitHub)" <gi...@apache.org>.
HattoriHenzo commented on PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#issuecomment-1658316626
@Alonexc When I issue the command `gradlew check`, there is no error message
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#issuecomment-1655188821
## [Codecov](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#4296](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (ca7a7e4) into [master](https://app.codecov.io/gh/apache/eventmesh/commit/5b8616bf308217cd362a279eb3986095940e49aa?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (5b8616b) will **decrease** coverage by `0.01%`.
> The diff coverage is `0.00%`.
> :exclamation: Current head ca7a7e4 differs from pull request most recent head fa07024. Consider uploading reports for the commit fa07024 to get more accurate results
```diff
@@ Coverage Diff @@
## master #4296 +/- ##
============================================
- Coverage 16.87% 16.86% -0.01%
Complexity 1428 1428
============================================
Files 593 594 +1
Lines 26035 26042 +7
Branches 2366 2378 +12
============================================
- Hits 4394 4393 -1
- Misses 21215 21220 +5
- Partials 426 429 +3
```
| [Files Changed](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...he/eventmesh/metrics/api/MetricsPluginFactory.java](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZXZlbnRtZXNoL21ldHJpY3MvYXBpL01ldHJpY3NQbHVnaW5GYWN0b3J5LmphdmE=) | `50.00% <ø> (ø)` | |
| [...ventmesh/metrics/api/model/HttpSummaryMetrics.java](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZXZlbnRtZXNoL21ldHJpY3MvYXBpL21vZGVsL0h0dHBTdW1tYXJ5TWV0cmljcy5qYXZh) | `0.00% <ø> (ø)` | |
| [.../metrics/prometheus/PrometheusMetricsRegistry.java](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLXByb21ldGhldXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9tZXRyaWNzL3Byb21ldGhldXMvUHJvbWV0aGV1c01ldHJpY3NSZWdpc3RyeS5qYXZh) | `9.67% <0.00%> (-1.04%)` | :arrow_down: |
| [...metrics/prometheus/metrics/PrometheusExporter.java](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLXByb21ldGhldXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9tZXRyaWNzL3Byb21ldGhldXMvbWV0cmljcy9Qcm9tZXRoZXVzRXhwb3J0ZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ics/prometheus/metrics/PrometheusGrpcExporter.java](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLXByb21ldGhldXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9tZXRyaWNzL3Byb21ldGhldXMvbWV0cmljcy9Qcm9tZXRoZXVzR3JwY0V4cG9ydGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ics/prometheus/metrics/PrometheusHttpExporter.java](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLXByb21ldGhldXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9tZXRyaWNzL3Byb21ldGhldXMvbWV0cmljcy9Qcm9tZXRoZXVzSHR0cEV4cG9ydGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...rics/prometheus/metrics/PrometheusTcpExporter.java](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLXByb21ldGhldXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9tZXRyaWNzL3Byb21ldGhldXMvbWV0cmljcy9Qcm9tZXRoZXVzVGNwRXhwb3J0ZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [.../prometheus/utils/PrometheusExporterConstants.java](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLXByb21ldGhldXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9tZXRyaWNzL3Byb21ldGhldXMvdXRpbHMvUHJvbWV0aGV1c0V4cG9ydGVyQ29uc3RhbnRzLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...rics/prometheus/utils/PrometheusExporterUtils.java](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLXByb21ldGhldXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9tZXRyaWNzL3Byb21ldGhldXMvdXRpbHMvUHJvbWV0aGV1c0V4cG9ydGVyVXRpbHMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ventmesh/runtime/admin/handler/MetricsHandler.java](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2FkbWluL2hhbmRsZXIvTWV0cmljc0hhbmRsZXIuamF2YQ==) | `5.88% <0.00%> (ø)` | |
| ... and [1 more](https://app.codecov.io/gh/apache/eventmesh/pull/4296?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [13 files with indirect coverage changes](https://app.codecov.io/gh/apache/eventmesh/pull/4296/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "HattoriHenzo (via GitHub)" <gi...@apache.org>.
HattoriHenzo commented on code in PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#discussion_r1276986245
##########
eventmesh-metrics-plugin/eventmesh-metrics-prometheus/src/main/java/org/apache/eventmesh/metrics/prometheus/metrics/PrometheusExporter.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.eventmesh.metrics.prometheus.metrics;
+
+import static org.apache.eventmesh.metrics.prometheus.utils.PrometheusExporterUtils.observeOfValue;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.Meter;
+
+public abstract class PrometheusExporter<T> {
+
+ /**
+ * Map structure : [metric name, description of name] -> the method of get corresponding metric.
+ */
+ protected final Map<String[], Function<T, Number>> paramPairs = new HashMap<>();
+
+ protected abstract String getMetricName(String[] metricInfo);
+
+ protected abstract String getMetricDescription(String[] metricInfo);
Review Comment:
@pandaapo These methods are implemented by each `PrometheusXXXExporter ` classes because in some cases a concatenation of another constant is made with the `metricInfo` parameter.
```
@Override
public String getMetricName(String[] metricInfo) {
return METRICS_GRPC_PREFIX + metricInfo[0];
}
```
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org
Re: [PR] [ISSUE #4201] Do some code optimization.[metrics] (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on code in PR #4296:
URL: https://github.com/apache/eventmesh/pull/4296#discussion_r1278144759
##########
eventmesh-metrics-plugin/eventmesh-metrics-prometheus/src/main/java/org/apache/eventmesh/metrics/prometheus/metrics/PrometheusExporter.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.eventmesh.metrics.prometheus.metrics;
+
+import static org.apache.eventmesh.metrics.prometheus.utils.PrometheusExporterUtils.observeOfValue;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+import io.opentelemetry.api.metrics.GlobalMeterProvider;
+import io.opentelemetry.api.metrics.Meter;
+
+public abstract class PrometheusExporter<T> {
+
+ /**
+ * Map structure : [metric name, description of name] the map variable to get corresponding metric
Review Comment:
I knew your removing because of CI error.
Maybe because my English is poor. I not only feel that the modified comments cannot express the structure of the map as clearly as before, but I even feel that the modified comments is not coherent.
`key: [metric name, description of name], value: the method of get corresponding metric` or other better comment may be friendlier to developers.
--
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: issues-unsubscribe@eventmesh.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: issues-help@eventmesh.apache.org