You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@eventmesh.apache.org by "mxsm (via GitHub)" <gi...@apache.org> on 2023/04/25 15:17:24 UTC
[GitHub] [eventmesh] mxsm opened a new pull request, #3446: [ISSUE #3430]Refactor eventmesh-metrics-prometheus module
mxsm opened a new pull request, #3446:
URL: https://github.com/apache/eventmesh/pull/3446
<!--
### 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 #3430
### Motivation
1. **The current project is using a lower version of OpenTelemetry components, version 1.3.0, while the latest version is 1.24.0. The related versions need to be upgraded.**
2. **The version upgrade brings new specifications and features, requiring code refactoring.**
3. **The instrumentation of metrics needs to be adjusted accordingly based on the related semantic conversion specifications of OpenTelemetry. (https://opentelemetry.io/docs/reference/specification/metrics/semantic_conventions/)**
4. **Optimize and refactor the related code logic to make the code clearer.**
### Modifications
*Refactor eventmesh-metrics-prometheus module*
### Documentation
- Does this pull request introduce a new feature? (yes / no)
- If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
- If a feature is not applicable for documentation, explain why?
- If a feature is not documented yet in this PR, please create a followup issue for adding the documentation
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1622669662
> He can simply rebase his branch onto the master branch when the time comes, but this process may involve resolving conflicts.
Do you mean that the development of refactoring "admin" can be concurrent with the rest works of this PR?
--
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
[GitHub] [incubator-eventmesh] mxsm commented on pull request #3446: [ISSUE #3430]Refactor eventmesh-metrics-prometheus module
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm commented on PR #3446:
URL: https://github.com/apache/incubator-eventmesh/pull/3446#issuecomment-1468382383
I will try fix the License Check not pass and resubmit later
--
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
[GitHub] [eventmesh] mxsm closed pull request #3446: [ISSUE #3430]Refactor eventmesh-metrics-prometheus module
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm closed pull request #3446: [ISSUE #3430]Refactor eventmesh-metrics-prometheus module
URL: https://github.com/apache/eventmesh/pull/3446
--
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
[GitHub] [eventmesh] mxsm commented on pull request #3446: [ISSUE #3430]Refactor eventmesh-metrics-prometheus module
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1514546530
I will refactor this pr code and resubmit
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1887005272
Any progress we can make here?
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1622097735
> @pandaapo @horoc Refactoring the `org.apache.eventmesh.runtime.admin` package into the `eventmesh-admin` module is one of the pending tasks on our to-do list. It is advisable to merge this PR before the refactoring takes place, in order to avoid potential conflicts. PTAL at your convenience. 😉
@Pil0tXia There will be a biweekly meeting on Thursday where I can bring up this issue. Since it involves some refactoring, I will explain the problem during the meeting. Meanwhile, I will provide the corresponding documentation to the community as soon as possible. Additionally, everyone can help by reviewing the code. Let's aim to have the community merge this pull request as soon as possible.
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm closed pull request #3446: [ISSUE #3430]Refactor eventmesh-metrics-prometheus module
URL: https://github.com/apache/eventmesh/pull/3446
--
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
[GitHub] [eventmesh] mxsm commented on pull request #3446: [ISSUE #3430]Refactor eventmesh-metrics-prometheus module
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1537084127
@xwm1992 PTAL~
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "pandaapo (via GitHub)" <gi...@apache.org>.
pandaapo commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1622013414
@Pil0tXia I'm sorry about that I'm not familiar with this module for now, and in fact I do not have permission to perform an effective approving or decide merging a PR.
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1887150709
> Any progress we can make here?
@Pil0tXia
I will resolve the conflicts. Can you help review the code? I've provided explanations for the code previously.
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-2054102540
This pr will be closed, the new is #4840
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1621980939
@pandaapo @horoc
Refactoring the `org.apache.eventmesh.runtime.admin` package into the `eventmesh-admin` module is one of the pending tasks on our to-do list. It is advisable to merge this PR before the refactoring takes place, in order to avoid potential conflicts. PTAL at your convenience. 😉
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1622102671
> @Pil0tXia I'm sorry about that I'm not familiar with this module for now, and in fact I do not have permission to perform an effective approving or decide merging a PR.
>
> If you are worried about potential conflicts during the refactoring of "eventmesh admin", you can try asking for advice from maintainers. They may advise you to refactor first.
@pandaapo The refactoring of the admin on his side is not a major issue, and my pull request may be merged before his. He can simply rebase his branch onto the master branch when the time comes, but this process may involve resolving conflicts. The overall changes related to observability are quite significant. The main upgrades are due to version changes, resulting in differences from the previous approach. There is also a re-design of the top-level interface, among other things.
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on code in PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#discussion_r1450991643
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/MetricsConstants.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.common;
+
+
+public abstract class MetricsConstants {
+
+ private MetricsConstants() {
+
+ }
Review Comment:
Maybe decorating this constant with `class` is enough, instead of `abstract class`.
The usages of this constant are all in metrics-plugin module, so perhaps placing this class in metrics-plugin module will be better.
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/core/protocol/http/processor/HeartBeatProcessor.java:
##########
@@ -68,37 +67,44 @@ public HeartBeatProcessor(final EventMeshHTTPServer eventMeshHTTPServer) {
public void processRequest(final ChannelHandlerContext ctx, final AsyncContext<HttpCommand> asyncContext) throws Exception {
HttpCommand responseEventMeshCommand;
final String localAddress = IPUtils.getLocalAddress();
- HttpCommand request = asyncContext.getRequest();
+
if (log.isInfoEnabled()) {
log.info("cmd={}|{}|client2eventMesh|from={}|to={}",
- RequestCode.get(Integer.valueOf(request.getRequestCode())),
+ RequestCode.get(Integer.valueOf(asyncContext.getRequest().getRequestCode())),
EventMeshConstants.PROTOCOL_HTTP,
RemotingHelper.parseChannelRemoteAddr(ctx.channel()), localAddress);
}
- final HeartbeatRequestHeader heartbeatRequestHeader = (HeartbeatRequestHeader) request.getHeader();
- final HeartbeatRequestBody heartbeatRequestBody = (HeartbeatRequestBody) request.getBody();
- EventMeshHTTPConfiguration httpConfiguration = eventMeshHTTPServer.getEventMeshHttpConfiguration();
+ final HeartbeatRequestHeader heartbeatRequestHeader = (HeartbeatRequestHeader) asyncContext.getRequest().getHeader();
+ final HeartbeatRequestBody heartbeatRequestBody = (HeartbeatRequestBody) asyncContext.getRequest().getBody();
final HeartbeatResponseHeader heartbeatResponseHeader =
- HeartbeatResponseHeader.buildHeader(Integer.valueOf(request.getRequestCode()),
- httpConfiguration.getEventMeshCluster(),
- localAddress, httpConfiguration.getEventMeshEnv(),
- httpConfiguration.getEventMeshIDC());
+ HeartbeatResponseHeader.buildHeader(Integer.valueOf(asyncContext.getRequest().getRequestCode()),
+ eventMeshHTTPServer.getEventMeshHttpConfiguration().getEventMeshCluster(),
+ localAddress, eventMeshHTTPServer.getEventMeshHttpConfiguration().getEventMeshEnv(),
+ eventMeshHTTPServer.getEventMeshHttpConfiguration().getEventMeshIDC());
//validate header
-
- if (StringUtils.isAnyBlank(
- heartbeatRequestHeader.getIdc(), heartbeatRequestHeader.getPid(), heartbeatRequestHeader.getSys())
- || !StringUtils.isNumeric(heartbeatRequestHeader.getPid())) {
- completeResponse(request, asyncContext, heartbeatResponseHeader,
- EventMeshRetCode.EVENTMESH_PROTOCOL_HEADER_ERR, null, HeartbeatResponseBody.class);
+ if (StringUtils.isBlank(heartbeatRequestHeader.getIdc())
+ || StringUtils.isBlank(heartbeatRequestHeader.getPid())
+ || !StringUtils.isNumeric(heartbeatRequestHeader.getPid())
+ || StringUtils.isBlank(heartbeatRequestHeader.getSys())) {
+ responseEventMeshCommand = asyncContext.getRequest().createHttpCommandResponse(
+ heartbeatResponseHeader,
+ HeartbeatResponseBody.buildBody(EventMeshRetCode.EVENTMESH_PROTOCOL_HEADER_ERR.getRetCode(),
+ EventMeshRetCode.EVENTMESH_PROTOCOL_HEADER_ERR.getErrMsg()));
+ asyncContext.onComplete(responseEventMeshCommand);
Review Comment:
It seems that the main change here is the invocation of `createHttpCommandResponse`. What are the advantages of the validation logic here compared to the original? Bringing a more comprehensive validation logic?
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/Pair.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.common;
+
+public class Pair<Left, Right> {
+
+ private Left left;
Review Comment:
We have two `Pair` classes in our project:
- org.apache.eventmesh.common.Pair
- org.apache.eventmesh.runtime.common.Pair
The usages of the latter one can be unified to the former one.
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/metrics/MetricsManager.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * 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.runtime.metrics;
+
+import org.apache.eventmesh.metrics.api.model.Metric;
+
+import java.util.List;
+
+/**
+ * metric manager interface
+ */
+
+/**
+ * MetricsManager is an interface for managing metrics.
+ */
+public interface MetricsManager {
Review Comment:
duplicate javadoc
##########
eventmesh-metrics-plugin/eventmesh-metrics-api/src/main/java/org/apache/eventmesh/metrics/api/model/AbstractMetric.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.api.model;
+
+public abstract class AbstractMetric implements Metric {
+
Review Comment:
The classes under `org.apache.eventmesh.metrics.api.model` are too many and their inheritance relationship seems a little messy.
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/boot/EventMeshServer.java:
##########
@@ -96,6 +104,12 @@ public EventMeshServer() {
if (BOOTSTRAP_LIST.isEmpty()) {
BOOTSTRAP_LIST.add(new EventMeshTcpBootstrap(this));
}
+ List<String> metricsPluginTypes = configuration.getEventMeshMetricsPluginType();
+ if (CollectionUtils.isNotEmpty(metricsPluginTypes)) {
+ List<MetricsRegistry> metricsRegistries = metricsPluginTypes.stream().map(metric -> MetricsPluginFactory.getMetricsRegistry(metric))
+ .collect(Collectors.toList());
+ eventMeshMetricsManager = new EventMeshMetricsManager(metricsRegistries);
+ }
Review Comment:
I think that maybe it is not a good design to place a list of metrics plugin types here. Is there a similar way to init metrics like other plugins (L79~L83)?
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/metrics/tcp/TcpMetricsCalculator.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.runtime.metrics.tcp;
+
+import org.apache.eventmesh.common.EventMeshThreadFactory;
+import org.apache.eventmesh.common.ThreadPoolFactory;
+import org.apache.eventmesh.runtime.boot.EventMeshTCPServer;
+import org.apache.eventmesh.runtime.constants.EventMeshConstants;
+import org.apache.eventmesh.runtime.core.protocol.tcp.client.session.Session;
+import org.apache.eventmesh.runtime.metrics.MonitorMetricConstants;
+
+import java.math.BigDecimal;
+import java.net.InetSocketAddress;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+
+import lombok.extern.slf4j.Slf4j;
+
+@Slf4j
+class TcpMetricsCalculator {
+
+ private static final int period = 30 * 1000;
Review Comment:
Why TCP needs a "Calculator" comparing to HTTP and gRPC?
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/metrics/GeneralMetricsManager.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * 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.runtime.metrics;
+
+
+import org.apache.eventmesh.metrics.api.MetricsRegistry;
+
+import org.apache.commons.collections4.MapUtils;
+
+import java.util.List;
+import java.util.Map;
+
+import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.common.AttributesBuilder;
+
+import lombok.experimental.UtilityClass;
+
+/**
+ * Managing general metrics.
+ */
+@UtilityClass
+public class GeneralMetricsManager {
Review Comment:
The naming of `GeneralMetricsManager` is a little confusing comparing with `MetricsManager`. How about putting them in different packages and rename it to a more suitable name?
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/metrics/MetricInstrumentUnit.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.runtime.metrics;
+
+
+public abstract class MetricInstrumentUnit {
+
+ private MetricInstrumentUnit() {
+
+ }
Review Comment:
We can put this constant class into `constant` package together with `MonitorMetricConstants`.
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/boot/EventMeshHttpBootstrap.java:
##########
@@ -43,6 +43,7 @@ public void init() throws Exception {
// server init
if (eventMeshHttpConfiguration != null) {
eventMeshHttpServer = new EventMeshHTTPServer(eventMeshServer, eventMeshHttpConfiguration);
+ eventMeshHttpServer.init();
}
Review Comment:
Is it necessary to init eventMeshHttpServer here?
##########
eventmesh-runtime/src/main/java/org/apache/eventmesh/runtime/core/protocol/http/processor/HeartBeatProcessor.java:
##########
@@ -147,50 +156,54 @@ public void processRequest(final ChannelHandlerContext ctx, final AsyncContext<H
}
- ConcurrentHashMap<String, List<Client>> clientInfoMap =
- eventMeshHTTPServer.getSubscriptionManager().getLocalClientInfoMapping();
- synchronized (clientInfoMap) {
+ synchronized (eventMeshHTTPServer.getSubscriptionManager().getLocalClientInfoMapping()) {
for (final Map.Entry<String, List<Client>> groupTopicClientMapping : tmpMap.entrySet()) {
- final List<Client> localClientList = clientInfoMap.get(groupTopicClientMapping.getKey());
+ final List<Client> localClientList =
+ eventMeshHTTPServer.getSubscriptionManager().getLocalClientInfoMapping().get(groupTopicClientMapping.getKey());
if (CollectionUtils.isEmpty(localClientList)) {
- clientInfoMap.put(groupTopicClientMapping.getKey(), groupTopicClientMapping.getValue());
+ eventMeshHTTPServer.getSubscriptionManager().getLocalClientInfoMapping()
+ .put(groupTopicClientMapping.getKey(), groupTopicClientMapping.getValue());
} else {
final List<Client> tmpClientList = groupTopicClientMapping.getValue();
supplyClientInfoList(tmpClientList, localClientList);
- clientInfoMap.put(groupTopicClientMapping.getKey(), localClientList);
+ eventMeshHTTPServer.getSubscriptionManager().getLocalClientInfoMapping().put(groupTopicClientMapping.getKey(), localClientList);
}
}
}
final long startTime = System.currentTimeMillis();
- HttpSummaryMetrics summaryMetrics = eventMeshHTTPServer.getMetrics().getSummaryMetrics();
try {
- final CompleteHandler<HttpCommand> handler = httpCommand -> {
- try {
- if (log.isDebugEnabled()) {
- log.debug("{}", httpCommand);
+
+ final CompleteHandler<HttpCommand> handler = new CompleteHandler<HttpCommand>() {
+ @Override
+ public void onResponse(final HttpCommand httpCommand) {
+ try {
+ if (log.isDebugEnabled()) {
+ log.debug("{}", httpCommand);
+ }
Review Comment:
`isDebugEnabled` condition is redundant.
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1673249397
@mxsm Hi~eventmesh master often throws this exception during normal operation, a feedback to you~
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1673349871
> @mxsm Hi~ EventMesh master often throws this exception during normal operation, a feedback to you~
>
> 
Thanks for your feedback, I will try to find it and fix
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1908398249
@Pil0tXia This PR has been open for too long, and many code changes make resolving conflicts during refactoring challenging. I will submit a new PR based on the latest code. The overall design will still be based on the current PR, and I will address the issues you mentioned above one by one.
--
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
[GitHub] [incubator-eventmesh] codecov[bot] commented on pull request #3446: [ISSUE #3430]Refactor eventmesh-metrics-prometheus module
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #3446:
URL: https://github.com/apache/incubator-eventmesh/pull/3446#issuecomment-1467231990
## [Codecov](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3446](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a9d36ab) into [master](https://codecov.io/gh/apache/incubator-eventmesh/commit/b6d85a44baddb2dfbf2d84369143b95b5a51f4f5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b6d85a4) will **decrease** coverage by `0.03%`.
> The diff coverage is `1.89%`.
> :exclamation: Current head a9d36ab differs from pull request most recent head 5d14b94. Consider uploading reports for the commit 5d14b94 to get more accurate results
```diff
@@ Coverage Diff @@
## master #3446 +/- ##
============================================
- Coverage 13.92% 13.89% -0.03%
- Complexity 1438 1439 +1
============================================
Files 636 638 +2
Lines 31375 31466 +91
Branches 3008 3008
============================================
+ Hits 4368 4372 +4
- Misses 26630 26717 +87
Partials 377 377
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ventmesh/metrics/api/model/GrpcSummaryMetrics.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZXZlbnRtZXNoL21ldHJpY3MvYXBpL21vZGVsL0dycGNTdW1tYXJ5TWV0cmljcy5qYXZh) | `80.00% <ø> (ø)` | |
| [...ventmesh/metrics/api/model/HttpSummaryMetrics.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZXZlbnRtZXNoL21ldHJpY3MvYXBpL21vZGVsL0h0dHBTdW1tYXJ5TWV0cmljcy5qYXZh) | `0.00% <ø> (ø)` | |
| [...eventmesh/metrics/api/model/TcpSummaryMetrics.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZXZlbnRtZXNoL21ldHJpY3MvYXBpL21vZGVsL1RjcFN1bW1hcnlNZXRyaWNzLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ics/prometheus/OpenTelemetryPrometheusManager.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLXByb21ldGhldXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9tZXRyaWNzL3Byb21ldGhldXMvT3BlblRlbGVtZXRyeVByb21ldGhldXNNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ics/prometheus/metrics/PrometheusGrpcExporter.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLXByb21ldGhldXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9tZXRyaWNzL3Byb21ldGhldXMvbWV0cmljcy9Qcm9tZXRoZXVzR3JwY0V4cG9ydGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ics/prometheus/metrics/PrometheusHttpExporter.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLXByb21ldGhldXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9tZXRyaWNzL3Byb21ldGhldXMvbWV0cmljcy9Qcm9tZXRoZXVzSHR0cEV4cG9ydGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...rics/prometheus/metrics/PrometheusTcpExporter.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLW1ldHJpY3MtcGx1Z2luL2V2ZW50bWVzaC1tZXRyaWNzLXByb21ldGhldXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9tZXRyaWNzL3Byb21ldGhldXMvbWV0cmljcy9Qcm9tZXRoZXVzVGNwRXhwb3J0ZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...che/eventmesh/runtime/boot/AbstractHTTPServer.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2Jvb3QvQWJzdHJhY3RIVFRQU2VydmVyLmphdmE=) | `1.12% <0.00%> (ø)` | |
| [...eventmesh/runtime/boot/AbstractRemotingServer.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2Jvb3QvQWJzdHJhY3RSZW1vdGluZ1NlcnZlci5qYXZh) | `4.25% <0.00%> (-0.30%)` | :arrow_down: |
| [...he/eventmesh/runtime/boot/EventMeshGrpcServer.java](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXZlbnRtZXNoLXJ1bnRpbWUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2V2ZW50bWVzaC9ydW50aW1lL2Jvb3QvRXZlbnRNZXNoR3JwY1NlcnZlci5qYXZh) | `0.75% <0.00%> (+<0.01%)` | :arrow_up: |
| ... and [34 more](https://codecov.io/gh/apache/incubator-eventmesh/pull/3446?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
: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=The+Apache+Software+Foundation)
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "Pil0tXia (via GitHub)" <gi...@apache.org>.
Pil0tXia commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1622815819
Concurrent work is technically feasible, while refactoring after this PR is merged will have fewer conflicts.
I will start refactoring maybe 1 month later. Not so urgent.
--
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 #3430]Refactor eventmesh-metrics-prometheus module (eventmesh)
Posted by "mxsm (via GitHub)" <gi...@apache.org>.
mxsm commented on PR #3446:
URL: https://github.com/apache/eventmesh/pull/3446#issuecomment-1638364499
[chinese document](https://blog.ljbmxsm.com/docs/event-mesh/core/quick-start/metric) about metrics
--
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