[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16431896#comment-16431896
]
ASF GitHub Bot commented on SCB-422:
wujimin closed pull request #639: [SCB-422] prometheus switch to new mechanism
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/639
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
index fa53abc16..da0765dce 100644
---
a/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
+++
b/demo/demo-springmvc/springmvc-client/src/main/java/org/apache/servicecomb/demo/springmvc/client/SpringmvcClient.java
@@ -109,13 +109,13 @@ public static void run() {
try {
String content =
restTemplate.getForObject("cse://springmvc/codeFirstSpringmvc/prometheusForTest",
String.class);
- TestMgr.check(true,
content.contains("servicecomb_invocation_springmvc_codeFirst_addDate"));
- TestMgr.check(true,
content.contains("servicecomb_invocation_springmvc_codeFirst_sayHello"));
- TestMgr.check(true,
content.contains("servicecomb_invocation_springmvc_codeFirst_fallbackFromCache"));
- TestMgr.check(true,
content.contains("servicecomb_invocation_springmvc_codeFirst_isTrue"));
- TestMgr.check(true,
content.contains("servicecomb_invocation_springmvc_codeFirst_add"));
- TestMgr.check(true,
content.contains("servicecomb_invocation_springmvc_codeFirst_sayHi2"));
- TestMgr.check(true,
content.contains("servicecomb_invocation_springmvc_codeFirst_saySomething"));
+ TestMgr.check(true,
content.contains("servicecomb_invocation{operation=\"springmvc.codeFirst.addDate"));
+ TestMgr.check(true,
content.contains("servicecomb_invocation{operation=\"springmvc.codeFirst.sayHello"));
+ TestMgr.check(true,
content.contains("servicecomb_invocation{operation=\"springmvc.codeFirst.fallbackFromCache"));
+ TestMgr.check(true,
content.contains("servicecomb_invocation{operation=\"springmvc.codeFirst.isTrue"));
+ TestMgr.check(true,
content.contains("servicecomb_invocation{operation=\"springmvc.codeFirst.add"));
+ TestMgr.check(true,
content.contains("servicecomb_invocation{operation=\"springmvc.codeFirst.sayHi2"));
+ TestMgr.check(true,
content.contains("servicecomb_invocation{operation=\"springmvc.codeFirst.saySomething"));
String[] metricLines = content.split("\n");
if (metricLines.length > 0) {
diff --git a/metrics/metrics-integration/metrics-prometheus/pom.xml
b/metrics/metrics-integration/metrics-prometheus/pom.xml
index dcaeb646d..3def8e768 100644
--- a/metrics/metrics-integration/metrics-prometheus/pom.xml
+++ b/metrics/metrics-integration/metrics-prometheus/pom.xml
@@ -16,8 +16,7 @@
~ limitations under the License.
-->
-http://maven.apache.org/POM/4.0.0;
- xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
metrics-integration
@@ -43,7 +42,10 @@
org.apache.servicecomb
metrics-core
-
+
+ org.apache.servicecomb
+ foundation-test-scaffolding
+
\ No newline at end of file
diff --git
a/metrics/metrics-integration/metrics-prometheus/src/main/java/org/apache/servicecomb/metrics/prometheus/MetricsCollector.java
b/metrics/metrics-integration/metrics-prometheus/src/main/java/org/apache/servicecomb/metrics/prometheus/MetricsCollector.java
deleted file mode 100644
index a99cce4c3..0
---
a/metrics/metrics-integration/metrics-prometheus/src/main/java/org/apache/servicecomb/metrics/prometheus/MetricsCollector.java
+++ /dev/null
@@ -1,76 +0,0 @@
-/*
- * 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
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16431892#comment-16431892
]
ASF GitHub Bot commented on SCB-422:
WillemJiang commented on a change in pull request #639: [SCB-422] prometheus
switch to new mechanism
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r180329100
##
File path:
metrics/metrics-integration/metrics-prometheus/src/test/java/org/apache/servicecomb/metrics/prometheus/TestPrometheusPublisher.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.servicecomb.metrics.prometheus;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import
org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
+import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.Counter;
+import com.netflix.spectator.api.DefaultRegistry;
+import com.netflix.spectator.api.ManualClock;
+import com.netflix.spectator.api.Registry;
+import com.netflix.spectator.api.SpectatorUtils;
+import com.sun.net.httpserver.HttpServer;
+
+import io.prometheus.client.exporter.HTTPServer;
+
+@SuppressWarnings("restriction")
+public class TestPrometheusPublisher {
+ CompositeRegistry globalRegistry =
SpectatorUtils.createCompositeRegistry(new ManualClock());
+
+ PrometheusPublisher publisher = new PrometheusPublisher();
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ @AfterClass
+ public static void teardown() {
+ArchaiusUtils.resetConfig();
+ }
+
+ @Test
+ public void testBadPublishAddress() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"a:b:c");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void testBadPublishAddress_BadPort() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void testBadPublishAddress_TooLargePort() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:999");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void collect() throws IllegalAccessException, IOException {
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:0");
+publisher.init(globalRegistry, null, null);
+
+Registry registry = new DefaultRegistry(new ManualClock());
+globalRegistry.add(registry);
+
+Counter counter = registry.counter("count.name", "tag1", "tag1v", "tag2",
"tag2v");
+counter.increment();
+
+HTTPServer httpServer = (HTTPServer) FieldUtils.readField(publisher,
"httpServer", true);
Review comment:
Fine.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
> Fix For: java-chassis-1.0.0-m2
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16430184#comment-16430184
]
ASF GitHub Bot commented on SCB-422:
wujimin commented on a change in pull request #639: [SCB-422] prometheus switch
to new mechanism
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r180006147
##
File path:
metrics/metrics-integration/metrics-prometheus/src/test/java/org/apache/servicecomb/metrics/prometheus/TestPrometheusPublisher.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.servicecomb.metrics.prometheus;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import
org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
+import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.Counter;
+import com.netflix.spectator.api.DefaultRegistry;
+import com.netflix.spectator.api.ManualClock;
+import com.netflix.spectator.api.Registry;
+import com.netflix.spectator.api.SpectatorUtils;
+import com.sun.net.httpserver.HttpServer;
+
+import io.prometheus.client.exporter.HTTPServer;
+
+@SuppressWarnings("restriction")
+public class TestPrometheusPublisher {
+ CompositeRegistry globalRegistry =
SpectatorUtils.createCompositeRegistry(new ManualClock());
+
+ PrometheusPublisher publisher = new PrometheusPublisher();
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ @AfterClass
+ public static void teardown() {
+ArchaiusUtils.resetConfig();
+ }
+
+ @Test
+ public void testBadPublishAddress() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"a:b:c");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void testBadPublishAddress_BadPort() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void testBadPublishAddress_TooLargePort() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:999");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void collect() throws IllegalAccessException, IOException {
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:0");
+publisher.init(globalRegistry, null, null);
+
+Registry registry = new DefaultRegistry(new ManualClock());
+globalRegistry.add(registry);
+
+Counter counter = registry.counter("count.name", "tag1", "tag1v", "tag2",
"tag2v");
+counter.increment();
+
+HTTPServer httpServer = (HTTPServer) FieldUtils.readField(publisher,
"httpServer", true);
Review comment:
we can use port 0 to try listen, and got the real port, and then close it
use the found real port to init prometheus.
but so complex, no need to do this, just a test case. i think reflection is
enough.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16430181#comment-16430181
]
ASF GitHub Bot commented on SCB-422:
wujimin commented on a change in pull request #639: [SCB-422] prometheus switch
to new mechanism
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r180006147
##
File path:
metrics/metrics-integration/metrics-prometheus/src/test/java/org/apache/servicecomb/metrics/prometheus/TestPrometheusPublisher.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.servicecomb.metrics.prometheus;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import
org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
+import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.Counter;
+import com.netflix.spectator.api.DefaultRegistry;
+import com.netflix.spectator.api.ManualClock;
+import com.netflix.spectator.api.Registry;
+import com.netflix.spectator.api.SpectatorUtils;
+import com.sun.net.httpserver.HttpServer;
+
+import io.prometheus.client.exporter.HTTPServer;
+
+@SuppressWarnings("restriction")
+public class TestPrometheusPublisher {
+ CompositeRegistry globalRegistry =
SpectatorUtils.createCompositeRegistry(new ManualClock());
+
+ PrometheusPublisher publisher = new PrometheusPublisher();
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ @AfterClass
+ public static void teardown() {
+ArchaiusUtils.resetConfig();
+ }
+
+ @Test
+ public void testBadPublishAddress() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"a:b:c");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void testBadPublishAddress_BadPort() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void testBadPublishAddress_TooLargePort() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:999");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void collect() throws IllegalAccessException, IOException {
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:0");
+publisher.init(globalRegistry, null, null);
+
+Registry registry = new DefaultRegistry(new ManualClock());
+globalRegistry.add(registry);
+
+Counter counter = registry.counter("count.name", "tag1", "tag1v", "tag2",
"tag2v");
+counter.increment();
+
+HTTPServer httpServer = (HTTPServer) FieldUtils.readField(publisher,
"httpServer", true);
Review comment:
we can use port 0 to try listen, and got the real port, and the close it
use the found real port to init prometheus.
but so complex, no need to do this, just a test case. i think reflection is
enough.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16430178#comment-16430178 ] ASF GitHub Bot commented on SCB-422: wujimin commented on a change in pull request #639: [SCB-422] prometheus switch to new mechanism URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r180005837 ## File path: metrics/metrics-integration/metrics-prometheus/src/test/java/org/apache/servicecomb/metrics/prometheus/TestPrometheusPublisher.java ## @@ -0,0 +1,107 @@ +/* + * 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.servicecomb.metrics.prometheus; + +import java.io.IOException; +import java.io.InputStream; +import java.net.HttpURLConnection; +import java.net.URL; + +import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.reflect.FieldUtils; +import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException; +import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import com.netflix.spectator.api.CompositeRegistry; +import com.netflix.spectator.api.Counter; +import com.netflix.spectator.api.DefaultRegistry; +import com.netflix.spectator.api.ManualClock; +import com.netflix.spectator.api.Registry; +import com.netflix.spectator.api.SpectatorUtils; +import com.sun.net.httpserver.HttpServer; Review comment: prometheus depend on com.sun.net.httpserver.HttpServer This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > Fix For: java-chassis-1.0.0-m2 > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16430177#comment-16430177 ] ASF GitHub Bot commented on SCB-422: wujimin commented on a change in pull request #639: [SCB-422] prometheus switch to new mechanism URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r180005756 ## File path: metrics/metrics-integration/metrics-prometheus/pom.xml ## @@ -43,7 +42,10 @@ org.apache.servicecomb metrics-core - + Review comment: 'foundation-test-scaffolding' already in 'test' scope, no need to set again. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > Fix For: java-chassis-1.0.0-m2 > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16430158#comment-16430158
]
ASF GitHub Bot commented on SCB-422:
WillemJiang commented on a change in pull request #639: [SCB-422] prometheus
switch to new mechanism
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r180003465
##
File path:
metrics/metrics-integration/metrics-prometheus/src/test/java/org/apache/servicecomb/metrics/prometheus/TestPrometheusPublisher.java
##
@@ -0,0 +1,107 @@
+/*
+ * 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.servicecomb.metrics.prometheus;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import
org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
+import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
+import org.junit.AfterClass;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.Counter;
+import com.netflix.spectator.api.DefaultRegistry;
+import com.netflix.spectator.api.ManualClock;
+import com.netflix.spectator.api.Registry;
+import com.netflix.spectator.api.SpectatorUtils;
+import com.sun.net.httpserver.HttpServer;
+
+import io.prometheus.client.exporter.HTTPServer;
+
+@SuppressWarnings("restriction")
+public class TestPrometheusPublisher {
+ CompositeRegistry globalRegistry =
SpectatorUtils.createCompositeRegistry(new ManualClock());
+
+ PrometheusPublisher publisher = new PrometheusPublisher();
+
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ @AfterClass
+ public static void teardown() {
+ArchaiusUtils.resetConfig();
+ }
+
+ @Test
+ public void testBadPublishAddress() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"a:b:c");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void testBadPublishAddress_BadPort() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void testBadPublishAddress_TooLargePort() {
+thrown.expect(ServiceCombException.class);
+
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:999");
+publisher.init(globalRegistry, null, null);
+ }
+
+ @Test
+ public void collect() throws IllegalAccessException, IOException {
+ArchaiusUtils.setProperty(PrometheusPublisher.METRICS_PROMETHEUS_ADDRESS,
"localhost:0");
+publisher.init(globalRegistry, null, null);
+
+Registry registry = new DefaultRegistry(new ManualClock());
+globalRegistry.add(registry);
+
+Counter counter = registry.counter("count.name", "tag1", "tag1v", "tag2",
"tag2v");
+counter.increment();
+
+HTTPServer httpServer = (HTTPServer) FieldUtils.readField(publisher,
"httpServer", true);
Review comment:
Can we find another way to find the server published address?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
> Fix For: java-chassis-1.0.0-m2
>
>
--
This message
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16430156#comment-16430156 ] ASF GitHub Bot commented on SCB-422: WillemJiang commented on a change in pull request #639: [SCB-422] prometheus switch to new mechanism URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r180003196 ## File path: metrics/metrics-integration/metrics-prometheus/src/test/java/org/apache/servicecomb/metrics/prometheus/TestPrometheusPublisher.java ## @@ -0,0 +1,107 @@ +/* + * 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.servicecomb.metrics.prometheus; + +import java.io.IOException; +import java.io.InputStream; +import java.net.HttpURLConnection; +import java.net.URL; + +import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.reflect.FieldUtils; +import org.apache.servicecomb.foundation.common.exceptions.ServiceCombException; +import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import com.netflix.spectator.api.CompositeRegistry; +import com.netflix.spectator.api.Counter; +import com.netflix.spectator.api.DefaultRegistry; +import com.netflix.spectator.api.ManualClock; +import com.netflix.spectator.api.Registry; +import com.netflix.spectator.api.SpectatorUtils; +import com.sun.net.httpserver.HttpServer; Review comment: Do we still need to use com.sun package ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > Fix For: java-chassis-1.0.0-m2 > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16430147#comment-16430147 ] ASF GitHub Bot commented on SCB-422: yangbor commented on a change in pull request #639: [SCB-422] prometheus switch to new mechanism URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r18908 ## File path: metrics/metrics-integration/metrics-prometheus/pom.xml ## @@ -43,7 +42,10 @@ org.apache.servicecomb metrics-core - + Review comment: I mean 'foundation-test-scaffolding' This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > Fix For: java-chassis-1.0.0-m2 > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16430132#comment-16430132 ] ASF GitHub Bot commented on SCB-422: yangbor commented on issue #639: [SCB-422] prometheus switch to new mechanism URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#issuecomment-379647394 Some files don't have proper final newline. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > Fix For: java-chassis-1.0.0-m2 > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16430129#comment-16430129 ] ASF GitHub Bot commented on SCB-422: yangbor commented on a change in pull request #639: [SCB-422] prometheus switch to new mechanism URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r179996570 ## File path: metrics/metrics-integration/metrics-prometheus/pom.xml ## @@ -43,7 +42,10 @@ org.apache.servicecomb metrics-core - + Review comment: Shouldn't the scope set to 'test' here? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > Fix For: java-chassis-1.0.0-m2 > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16430021#comment-16430021
]
ASF GitHub Bot commented on SCB-422:
wujimin commented on a change in pull request #639: [SCB-422] prometheus switch
to new mechanism
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r179985745
##
File path:
metrics/metrics-integration/metrics-prometheus/src/main/java/org/apache/servicecomb/metrics/prometheus/PrometheusPublisher.java
##
@@ -0,0 +1,126 @@
+/*
+ * 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.servicecomb.metrics.prometheus;
+
+import java.net.InetSocketAddress;
+import java.util.ArrayList;
+import java.util.List;
+
+import
org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
+import org.apache.servicecomb.foundation.metrics.MetricsBootstrapConfig;
+import org.apache.servicecomb.foundation.metrics.MetricsInitializer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.eventbus.EventBus;
+import com.netflix.config.DynamicPropertyFactory;
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.Measurement;
+import com.netflix.spectator.api.Tag;
+
+import io.prometheus.client.Collector;
+import io.prometheus.client.Collector.MetricFamilySamples.Sample;
+import io.prometheus.client.CollectorRegistry;
+import io.prometheus.client.exporter.HTTPServer;
+
+public class PrometheusPublisher extends Collector implements
Collector.Describable, MetricsInitializer {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PrometheusPublisher.class);
+
+ static final String METRICS_PROMETHEUS_ADDRESS =
"servicecomb.metrics.prometheus.address";
+
+ private HTTPServer httpServer;
+
+ private CompositeRegistry globalRegistry;
+
+ @Override
+ public void init(CompositeRegistry globalRegistry, EventBus eventBus,
MetricsBootstrapConfig config) {
+this.globalRegistry = globalRegistry;
+
+//prometheus default port allocation is here :
https://github.com/prometheus/prometheus/wiki/Default-port-allocations
+String address =
+
DynamicPropertyFactory.getInstance().getStringProperty(METRICS_PROMETHEUS_ADDRESS,
"0.0.0.0:9696").get();
+
+try {
+ InetSocketAddress socketAddress = getSocketAddress(address);
+ register();
+ this.httpServer = new HTTPServer(socketAddress,
CollectorRegistry.defaultRegistry, true);
Review comment:
By MetricsRestPublisher in previous PR, we already support metrics endpoint:
/metrics
and created a issue for contribute:
https://issues.apache.org/jira/browse/SCB-466
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
> Fix For: java-chassis-1.0.0-m2
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16429996#comment-16429996
]
ASF GitHub Bot commented on SCB-422:
liubao68 commented on a change in pull request #639: [SCB-422] prometheus
switch to new mechanism
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r179981976
##
File path:
metrics/metrics-integration/metrics-prometheus/src/main/java/org/apache/servicecomb/metrics/prometheus/PrometheusPublisher.java
##
@@ -0,0 +1,126 @@
+/*
+ * 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.servicecomb.metrics.prometheus;
+
+import java.net.InetSocketAddress;
+import java.util.ArrayList;
+import java.util.List;
+
+import
org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
+import org.apache.servicecomb.foundation.metrics.MetricsBootstrapConfig;
+import org.apache.servicecomb.foundation.metrics.MetricsInitializer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.eventbus.EventBus;
+import com.netflix.config.DynamicPropertyFactory;
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.Measurement;
+import com.netflix.spectator.api.Tag;
+
+import io.prometheus.client.Collector;
+import io.prometheus.client.Collector.MetricFamilySamples.Sample;
+import io.prometheus.client.CollectorRegistry;
+import io.prometheus.client.exporter.HTTPServer;
+
+public class PrometheusPublisher extends Collector implements
Collector.Describable, MetricsInitializer {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PrometheusPublisher.class);
+
+ static final String METRICS_PROMETHEUS_ADDRESS =
"servicecomb.metrics.prometheus.address";
+
+ private HTTPServer httpServer;
+
+ private CompositeRegistry globalRegistry;
+
+ @Override
+ public void init(CompositeRegistry globalRegistry, EventBus eventBus,
MetricsBootstrapConfig config) {
+this.globalRegistry = globalRegistry;
+
+//prometheus default port allocation is here :
https://github.com/prometheus/prometheus/wiki/Default-port-allocations
+String address =
+
DynamicPropertyFactory.getInstance().getStringProperty(METRICS_PROMETHEUS_ADDRESS,
"0.0.0.0:9696").get();
+
+try {
+ InetSocketAddress socketAddress = getSocketAddress(address);
+ register();
+ this.httpServer = new HTTPServer(socketAddress,
CollectorRegistry.defaultRegistry, true);
Review comment:
For future customization, I think we 'd provide way to export to servicecomb
rest framework. And contribute the code to
https://github.com/prometheus/client_java. If we can create a task to tracking
this?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
> Fix For: java-chassis-1.0.0-m2
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16429995#comment-16429995
]
ASF GitHub Bot commented on SCB-422:
liubao68 commented on a change in pull request #639: [SCB-422] prometheus
switch to new mechanism
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/639#discussion_r179981976
##
File path:
metrics/metrics-integration/metrics-prometheus/src/main/java/org/apache/servicecomb/metrics/prometheus/PrometheusPublisher.java
##
@@ -0,0 +1,126 @@
+/*
+ * 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.servicecomb.metrics.prometheus;
+
+import java.net.InetSocketAddress;
+import java.util.ArrayList;
+import java.util.List;
+
+import
org.apache.servicecomb.foundation.common.exceptions.ServiceCombException;
+import org.apache.servicecomb.foundation.metrics.MetricsBootstrapConfig;
+import org.apache.servicecomb.foundation.metrics.MetricsInitializer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.eventbus.EventBus;
+import com.netflix.config.DynamicPropertyFactory;
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.Measurement;
+import com.netflix.spectator.api.Tag;
+
+import io.prometheus.client.Collector;
+import io.prometheus.client.Collector.MetricFamilySamples.Sample;
+import io.prometheus.client.CollectorRegistry;
+import io.prometheus.client.exporter.HTTPServer;
+
+public class PrometheusPublisher extends Collector implements
Collector.Describable, MetricsInitializer {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PrometheusPublisher.class);
+
+ static final String METRICS_PROMETHEUS_ADDRESS =
"servicecomb.metrics.prometheus.address";
+
+ private HTTPServer httpServer;
+
+ private CompositeRegistry globalRegistry;
+
+ @Override
+ public void init(CompositeRegistry globalRegistry, EventBus eventBus,
MetricsBootstrapConfig config) {
+this.globalRegistry = globalRegistry;
+
+//prometheus default port allocation is here :
https://github.com/prometheus/prometheus/wiki/Default-port-allocations
+String address =
+
DynamicPropertyFactory.getInstance().getStringProperty(METRICS_PROMETHEUS_ADDRESS,
"0.0.0.0:9696").get();
+
+try {
+ InetSocketAddress socketAddress = getSocketAddress(address);
+ register();
+ this.httpServer = new HTTPServer(socketAddress,
CollectorRegistry.defaultRegistry, true);
Review comment:
For future customization, I think we 'd provide way to export to servicecomb
rest framework. And contribute the code to
https://github.com/prometheus/client_java
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
> Fix For: java-chassis-1.0.0-m2
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16429784#comment-16429784
]
ASF GitHub Bot commented on SCB-422:
wujimin closed pull request #633: [SCB-422] Threadpool metrics
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/633
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/core/src/main/java/org/apache/servicecomb/core/BootListener.java
b/core/src/main/java/org/apache/servicecomb/core/BootListener.java
index f6e50cc03..b3b875543 100644
--- a/core/src/main/java/org/apache/servicecomb/core/BootListener.java
+++ b/core/src/main/java/org/apache/servicecomb/core/BootListener.java
@@ -28,7 +28,9 @@
BEFORE_TRANSPORT,
AFTER_TRANSPORT,
BEFORE_REGISTRY,
-AFTER_REGISTRY
+AFTER_REGISTRY,
+BEFORE_CLOSE,
+AFTER_CLOSE
}
class BootEvent {
diff --git
a/core/src/main/java/org/apache/servicecomb/core/CseApplicationListener.java
b/core/src/main/java/org/apache/servicecomb/core/CseApplicationListener.java
index 5cff9ca5e..4952f35f3 100644
--- a/core/src/main/java/org/apache/servicecomb/core/CseApplicationListener.java
+++ b/core/src/main/java/org/apache/servicecomb/core/CseApplicationListener.java
@@ -147,7 +147,9 @@ public void onApplicationEvent(ApplicationEvent event) {
}
} else if (event instanceof ContextClosedEvent) {
LOGGER.warn("cse is closing now...");
+ triggerEvent(EventType.BEFORE_CLOSE);
RegistryUtils.destroy();
+ triggerEvent(EventType.AFTER_CLOSE);
isInit = false;
}
}
diff --git
a/core/src/main/java/org/apache/servicecomb/core/executor/FixedThreadExecutor.java
b/core/src/main/java/org/apache/servicecomb/core/executor/FixedThreadExecutor.java
index e70cc1bd0..2600ebb53 100644
---
a/core/src/main/java/org/apache/servicecomb/core/executor/FixedThreadExecutor.java
+++
b/core/src/main/java/org/apache/servicecomb/core/executor/FixedThreadExecutor.java
@@ -58,6 +58,10 @@ public FixedThreadExecutor() {
}
}
+ public List getExecutorList() {
+return executorList;
+ }
+
@Override
public void execute(Runnable command) {
long threadId = Thread.currentThread().getId();
diff --git
a/core/src/test/java/org/apache/servicecomb/core/TestCseApplicationListener.java
b/core/src/test/java/org/apache/servicecomb/core/TestCseApplicationListener.java
index fb473b751..e5eaec84a 100644
---
a/core/src/test/java/org/apache/servicecomb/core/TestCseApplicationListener.java
+++
b/core/src/test/java/org/apache/servicecomb/core/TestCseApplicationListener.java
@@ -22,11 +22,17 @@
import static org.mockito.Mockito.verify;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
+import javax.xml.ws.Holder;
+
+import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.servicecomb.core.BootListener.BootEvent;
+import org.apache.servicecomb.core.BootListener.EventType;
import org.apache.servicecomb.core.definition.loader.SchemaListenerManager;
import org.apache.servicecomb.core.endpoint.AbstractEndpointsCache;
import org.apache.servicecomb.core.provider.consumer.ConsumerProviderManager;
@@ -39,6 +45,7 @@
import org.apache.servicecomb.serviceregistry.RegistryUtils;
import
org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
import
org.apache.servicecomb.serviceregistry.task.MicroserviceInstanceRegisterTask;
+import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Assert;
@@ -53,6 +60,8 @@
import mockit.Deencapsulation;
import mockit.Expectations;
import mockit.Injectable;
+import mockit.Mock;
+import mockit.MockUp;
import mockit.Mocked;
public class TestCseApplicationListener {
@@ -147,15 +156,26 @@ public void
testCseApplicationListenerParentNotnull(@Injectable ContextRefreshed
}
@Test
- public void testCseApplicationListenerShutdown(@Injectable
ContextClosedEvent event,
- @Mocked RegistryUtils ru) {
-new Expectations() {
- {
-RegistryUtils.destroy();
+ public void testCseApplicationListenerShutdown(@Mocked ApplicationContext
context) throws IllegalAccessException {
+Holder destroyHolder = new Holder<>();
+new MockUp() {
+ @Mock
+ void destroy() {
+destroyHolder.value = true;
}
};
CseApplicationListener cal = new CseApplicationListener();
+ContextClosedEvent event = new ContextClosedEvent(context);
+
+List eventTypes = new ArrayList<>();
+BootListener bootListener = e -> {
+ eventTypes.add(e.getEventType());
+};
+FieldUtils.writeField(cal,
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16429697#comment-16429697 ] ASF GitHub Bot commented on SCB-422: wujimin commented on a change in pull request #633: [SCB-422] Threadpool metrics URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179941587 ## File path: metrics/metrics-core/src/main/java/com/netflix/spectator/api/patterns/ThreadPoolMonitorPublishModelFactory.java ## @@ -0,0 +1,98 @@ +/* + * 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 com.netflix.spectator.api.patterns; Review comment: yes ThreadPoolMonitor.TASK_COUNT and so on, is package visible This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16429685#comment-16429685 ] ASF GitHub Bot commented on SCB-422: liubao68 commented on a change in pull request #633: [SCB-422] Threadpool metrics URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179940415 ## File path: metrics/metrics-core/src/main/java/com/netflix/spectator/api/patterns/ThreadPoolMonitorPublishModelFactory.java ## @@ -0,0 +1,98 @@ +/* + * 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 com.netflix.spectator.api.patterns; Review comment: Why add this package? Access package visible variables ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16429654#comment-16429654
]
ASF GitHub Bot commented on SCB-422:
wujimin commented on a change in pull request #633: [SCB-422] Threadpool metrics
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179932884
##
File path:
metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestDefaultRegistryInitializer.java
##
@@ -0,0 +1,44 @@
+/*
+ * 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.servicecomb.metrics.core;
+
+import org.apache.servicecomb.foundation.metrics.MetricsBootstrapConfig;
+import org.hamcrest.Matchers;
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.google.common.eventbus.EventBus;
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.ManualClock;
+import com.netflix.spectator.api.SpectatorUtils;
+import com.netflix.spectator.servo.ServoRegistry;
+
+public class TestDefaultRegistryInitializer {
+ CompositeRegistry globalRegistry =
SpectatorUtils.createCompositeRegistry(new ManualClock());
+
+ DefaultRegistryInitializer registryInitializer = new
DefaultRegistryInitializer();
+
+ @Test
+ public void init() {
+registryInitializer.init(globalRegistry, new EventBus(), new
MetricsBootstrapConfig());
+
+Assert.assertEquals(-10, registryInitializer.getOrder());
+Assert.assertThat(registryInitializer.getRegistry(),
Matchers.instanceOf(ServoRegistry.class));
+
+registryInitializer.uninit();
Review comment:
done
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16429646#comment-16429646 ] ASF GitHub Bot commented on SCB-422: coveralls commented on issue #633: [SCB-422] Threadpool metrics URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#issuecomment-378193360 [](https://coveralls.io/builds/16403644) Coverage increased (+0.1%) to 87.73% when pulling **3d8d1a5bc04375bb9b22c86b507fc221b28cab8d on wujimin:threadpool-metrics** into **8a77e34365e786462b74826c724d10f0e735d5fe on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16429579#comment-16429579
]
ASF GitHub Bot commented on SCB-422:
wujimin commented on a change in pull request #633: [SCB-422] Threadpool metrics
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179932884
##
File path:
metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestDefaultRegistryInitializer.java
##
@@ -0,0 +1,44 @@
+/*
+ * 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.servicecomb.metrics.core;
+
+import org.apache.servicecomb.foundation.metrics.MetricsBootstrapConfig;
+import org.hamcrest.Matchers;
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.google.common.eventbus.EventBus;
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.ManualClock;
+import com.netflix.spectator.api.SpectatorUtils;
+import com.netflix.spectator.servo.ServoRegistry;
+
+public class TestDefaultRegistryInitializer {
+ CompositeRegistry globalRegistry =
SpectatorUtils.createCompositeRegistry(new ManualClock());
+
+ DefaultRegistryInitializer registryInitializer = new
DefaultRegistryInitializer();
+
+ @Test
+ public void init() {
+registryInitializer.init(globalRegistry, new EventBus(), new
MetricsBootstrapConfig());
+
+Assert.assertEquals(-10, registryInitializer.getOrder());
+Assert.assertThat(registryInitializer.getRegistry(),
Matchers.instanceOf(ServoRegistry.class));
+
+registryInitializer.uninit();
Review comment:
doen
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16429578#comment-16429578
]
ASF GitHub Bot commented on SCB-422:
wujimin commented on a change in pull request #633: [SCB-422] Threadpool metrics
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179932881
##
File path:
service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestClientPool.java
##
@@ -17,43 +17,16 @@
package org.apache.servicecomb.serviceregistry.client.http;
-import org.apache.servicecomb.serviceregistry.config.ServiceRegistryConfig;
import org.junit.Test;
-import io.vertx.core.http.HttpVersion;
-import mockit.Mock;
-import mockit.MockUp;
-
public class TestClientPool {
@Test
public void testHttpClientPool() {
-new MockUp() {
- @Mock
- public HttpVersion getHttpVersion() {
-return HttpVersion.HTTP_2;
- }
-
- @Mock
- public boolean isSsl() {
-return true;
- }
-};
HttpClientPool.INSTANCE.create();
Review comment:
done
add new test cases for it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16428368#comment-16428368
]
ASF GitHub Bot commented on SCB-422:
wujimin commented on a change in pull request #633: [SCB-422] Threadpool metrics
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179768362
##
File path:
metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestDefaultRegistryInitializer.java
##
@@ -0,0 +1,44 @@
+/*
+ * 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.servicecomb.metrics.core;
+
+import org.apache.servicecomb.foundation.metrics.MetricsBootstrapConfig;
+import org.hamcrest.Matchers;
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.google.common.eventbus.EventBus;
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.ManualClock;
+import com.netflix.spectator.api.SpectatorUtils;
+import com.netflix.spectator.servo.ServoRegistry;
+
+public class TestDefaultRegistryInitializer {
+ CompositeRegistry globalRegistry =
SpectatorUtils.createCompositeRegistry(new ManualClock());
+
+ DefaultRegistryInitializer registryInitializer = new
DefaultRegistryInitializer();
+
+ @Test
+ public void init() {
+registryInitializer.init(globalRegistry, new EventBus(), new
MetricsBootstrapConfig());
+
+Assert.assertEquals(-10, registryInitializer.getOrder());
+Assert.assertThat(registryInitializer.getRegistry(),
Matchers.instanceOf(ServoRegistry.class));
+
+registryInitializer.uninit();
Review comment:
just want to test init at start
when add uninit funcation, forgot add test case for it, i will add new
verification.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16428365#comment-16428365
]
ASF GitHub Bot commented on SCB-422:
wujimin commented on a change in pull request #633: [SCB-422] Threadpool metrics
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179767736
##
File path:
service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestClientPool.java
##
@@ -17,43 +17,16 @@
package org.apache.servicecomb.serviceregistry.client.http;
-import org.apache.servicecomb.serviceregistry.config.ServiceRegistryConfig;
import org.junit.Test;
-import io.vertx.core.http.HttpVersion;
-import mockit.Mock;
-import mockit.MockUp;
-
public class TestClientPool {
@Test
public void testHttpClientPool() {
-new MockUp() {
- @Mock
- public HttpVersion getHttpVersion() {
-return HttpVersion.HTTP_2;
- }
-
- @Mock
- public boolean isSsl() {
-return true;
- }
-};
HttpClientPool.INSTANCE.create();
Review comment:
oh, yes.
i only confirmed with author and deleted useless code, did not realize this
is not a effective test case
and the next case have the same problem.
i will confirmed with author again.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16428352#comment-16428352 ] ASF GitHub Bot commented on SCB-422: wujimin commented on a change in pull request #633: [SCB-422] Threadpool metrics URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179764368 ## File path: metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestAnMonitorManager.java ## @@ -41,7 +41,7 @@ private static MetricsLoader currentWindowMetricsLoader; - private static MetricsLoader nextWindowMetricsLoader; + // private static MetricsLoader nextWindowMetricsLoader; Review comment: cause random UT failed. this class, and related classes will be deleted in the next PR This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16428351#comment-16428351
]
ASF GitHub Bot commented on SCB-422:
wujimin commented on a change in pull request #633: [SCB-422] Threadpool metrics
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179764029
##
File path:
metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/DefaultRegistryInitializer.java
##
@@ -0,0 +1,64 @@
+/*
+ * 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.servicecomb.metrics.core;
+
+import org.apache.servicecomb.foundation.metrics.MetricsBootstrapConfig;
+import org.apache.servicecomb.foundation.metrics.MetricsInitializer;
+
+import com.google.common.eventbus.EventBus;
+import com.netflix.servo.DefaultMonitorRegistry;
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.Registry;
+import com.netflix.spectator.servo.ServoRegistry;
+
+public class DefaultRegistryInitializer implements MetricsInitializer {
+ public static final String METRICS_WINDOW_TIME =
"servicecomb.metrics.window_time";
+
+ public static final int DEFAULT_METRICS_WINDOW_TIME = 5000;
+
+ public static final String SERVO_POLLERS = "servo.pollers";
+
+ private CompositeRegistry globalRegistry;
+
+ private ServoRegistry registry;
+
+ // create registry before init meters
+ @Override
+ public int getOrder() {
+return -10;
+ }
+
+ @Override
+ public void init(CompositeRegistry globalRegistry, EventBus eventBus,
MetricsBootstrapConfig config) {
+this.globalRegistry = globalRegistry;
+
+System.getProperties().setProperty(SERVO_POLLERS,
String.valueOf(config.getMsPollInterval()));
+registry = new ServoRegistry();
+
Review comment:
this affect servo class static variable initialization
so clean or not, there is no difference
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16426586#comment-16426586
]
ASF GitHub Bot commented on SCB-422:
WillemJiang commented on a change in pull request #633: [SCB-422] Threadpool
metrics
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179374190
##
File path:
service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestClientPool.java
##
@@ -17,43 +17,16 @@
package org.apache.servicecomb.serviceregistry.client.http;
-import org.apache.servicecomb.serviceregistry.config.ServiceRegistryConfig;
import org.junit.Test;
-import io.vertx.core.http.HttpVersion;
-import mockit.Mock;
-import mockit.MockUp;
-
public class TestClientPool {
@Test
public void testHttpClientPool() {
-new MockUp() {
- @Mock
- public HttpVersion getHttpVersion() {
-return HttpVersion.HTTP_2;
- }
-
- @Mock
- public boolean isSsl() {
-return true;
- }
-};
HttpClientPool.INSTANCE.create();
Review comment:
There is no verification code here.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16426581#comment-16426581 ] ASF GitHub Bot commented on SCB-422: WillemJiang commented on a change in pull request #633: [SCB-422] Threadpool metrics URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179373685 ## File path: metrics/metrics-core/src/test/java/org/apache/servicecomb/metrics/core/TestAnMonitorManager.java ## @@ -41,7 +41,7 @@ private static MetricsLoader currentWindowMetricsLoader; - private static MetricsLoader nextWindowMetricsLoader; + // private static MetricsLoader nextWindowMetricsLoader; Review comment: Why did you comment out these code? If we don't need them, we can just remove them. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[
https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16426577#comment-16426577
]
ASF GitHub Bot commented on SCB-422:
WillemJiang commented on a change in pull request #633: [SCB-422] Threadpool
metrics
URL:
https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#discussion_r179373237
##
File path:
metrics/metrics-core/src/main/java/org/apache/servicecomb/metrics/core/DefaultRegistryInitializer.java
##
@@ -0,0 +1,64 @@
+/*
+ * 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.servicecomb.metrics.core;
+
+import org.apache.servicecomb.foundation.metrics.MetricsBootstrapConfig;
+import org.apache.servicecomb.foundation.metrics.MetricsInitializer;
+
+import com.google.common.eventbus.EventBus;
+import com.netflix.servo.DefaultMonitorRegistry;
+import com.netflix.spectator.api.CompositeRegistry;
+import com.netflix.spectator.api.Registry;
+import com.netflix.spectator.servo.ServoRegistry;
+
+public class DefaultRegistryInitializer implements MetricsInitializer {
+ public static final String METRICS_WINDOW_TIME =
"servicecomb.metrics.window_time";
+
+ public static final int DEFAULT_METRICS_WINDOW_TIME = 5000;
+
+ public static final String SERVO_POLLERS = "servo.pollers";
+
+ private CompositeRegistry globalRegistry;
+
+ private ServoRegistry registry;
+
+ // create registry before init meters
+ @Override
+ public int getOrder() {
+return -10;
+ }
+
+ @Override
+ public void init(CompositeRegistry globalRegistry, EventBus eventBus,
MetricsBootstrapConfig config) {
+this.globalRegistry = globalRegistry;
+
+System.getProperties().setProperty(SERVO_POLLERS,
String.valueOf(config.getMsPollInterval()));
+registry = new ServoRegistry();
+
Review comment:
Do we need to clean up the System properties setting here?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> add executor metrics, not just queue size
> -
>
> Key: SCB-422
> URL: https://issues.apache.org/jira/browse/SCB-422
> Project: Apache ServiceComb
> Issue Type: Sub-task
> Components: Java-Chassis
>Reporter: wujimin
>Assignee: wujimin
>Priority: Major
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16424866#comment-16424866 ] ASF GitHub Bot commented on SCB-422: coveralls commented on issue #633: [SCB-422] Threadpool metrics URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#issuecomment-378193360 [](https://coveralls.io/builds/16330757) Coverage increased (+0.07%) to 87.63% when pulling **e363450fc19701885bd260757d30bafa8a833668 on wujimin:threadpool-metrics** into **e0891a92a7d97f7b7a9a7402ac38128246729e83 on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16423773#comment-16423773 ] ASF GitHub Bot commented on SCB-422: coveralls commented on issue #633: [SCB-422] Threadpool metrics URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#issuecomment-378193360 [](https://coveralls.io/builds/16311837) Coverage increased (+0.04%) to 87.601% when pulling **c26e9adc2af0a6607513c4c4e6712c45bbe6a618 on wujimin:threadpool-metrics** into **e0891a92a7d97f7b7a9a7402ac38128246729e83 on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16423765#comment-16423765 ] ASF GitHub Bot commented on SCB-422: coveralls commented on issue #633: [SCB-422] Threadpool metrics URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/633#issuecomment-378193360 [](https://coveralls.io/builds/16311777) Coverage increased (+0.04%) to 87.601% when pulling **c26e9adc2af0a6607513c4c4e6712c45bbe6a618 on wujimin:threadpool-metrics** into **e0891a92a7d97f7b7a9a7402ac38128246729e83 on apache:master**. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (SCB-422) add executor metrics, not just queue size
[ https://issues.apache.org/jira/browse/SCB-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16423656#comment-16423656 ] ASF GitHub Bot commented on SCB-422: wujimin opened a new pull request #633: [SCB-422] Threadpool metrics URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/633 metrics for operation related threadpool   This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > add executor metrics, not just queue size > - > > Key: SCB-422 > URL: https://issues.apache.org/jira/browse/SCB-422 > Project: Apache ServiceComb > Issue Type: Sub-task > Components: Java-Chassis >Reporter: wujimin >Assignee: wujimin >Priority: Major > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
