You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/12/17 10:20:43 UTC
[GitHub] [ignite] timoninmaxim commented on a change in pull request #9457: IGNITE-12464 : Metrics for the Java Services V3.
timoninmaxim commented on a change in pull request #9457:
URL: https://github.com/apache/ignite/pull/9457#discussion_r770690434
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/service/GridServiceMetricsTest.java
##########
@@ -0,0 +1,336 @@
+/*
+ * 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.ignite.internal.processors.service;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import com.google.common.collect.Iterables;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.metric.GridMetricManager;
+import org.apache.ignite.internal.processors.service.inner.MyService;
+import org.apache.ignite.internal.processors.service.inner.MyServiceFactory;
+import org.apache.ignite.services.Service;
+import org.apache.ignite.services.ServiceConfiguration;
+import org.apache.ignite.spi.metric.HistogramMetric;
+import org.apache.ignite.spi.metric.Metric;
+import org.apache.ignite.spi.metric.ReadOnlyMetricRegistry;
+import org.apache.ignite.spi.metric.jmx.JmxMetricExporterSpi;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.serviceMetricRegistryName;
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.sumHistogramEntries;
+import static org.apache.ignite.internal.processors.service.IgniteServiceProcessor.SERVICE_METRIC_REGISTRY;
+
+/**
+ * Tests metrics of service invocations.
+ */
+public class GridServiceMetricsTest extends GridCommonAbstractTest {
+ /** Number of service invocations. */
+ private static final int INVOKE_CNT = 50;
+
+ /** Service name used in the tests. */
+ private static final String SRVC_NAME = "TestService";
+
+ /** Error message of created metrics. */
+ private static final String METRICS_MUST_NOT_BE_CREATED = "Service metric registry must not be created.";
+
+ /** Utility holder of current grid number. */
+ private int gridNum;
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+
+ super.afterTest();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ // JMX metrics exposition to see actual namings and placement of the metrics.
+ cfg.setMetricExporterSpi(new JmxMetricExporterSpi());
+
+ return cfg;
+ }
+
+ /** Checks service metrics are enabled / disabled properly. */
+ @Test
+ public void testServiceMetricsEnabledDisabled() throws Exception {
+ IgniteEx ignite = startGrid();
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), 0, 1);
+
+ srvcCfg.setStatisticsEnabled(false);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+
+ ignite.services().cancel(SRVC_NAME);
+
+ srvcCfg.setStatisticsEnabled(true);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNotNull("Service metric registry must be created.",
+ findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+ }
+
+ /** Checks metric behaviour when launched several service instances. */
+ @Test
+ public void testMultipleDeployment() throws Throwable {
+ List<IgniteEx> servers = new ArrayList<>();
+
+ servers.add(startGrid(gridNum++));
+
+ IgniteEx server = servers.get(0);
+
+ IgniteEx client = startClientGrid(gridNum++);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(server.context().metric(), SERVICE_METRIC_REGISTRY));
+
+ int totalInstance = 2;
+
+ int perNode = 2;
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), totalInstance, perNode);
+
+ server.services().deploy(srvcCfg);
+
+ awaitPartitionMapExchange();
+
+ // Call proxies on the clients.
+ Stream.generate(() -> client.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .limit(totalInstance).forEach(srvc -> ((MyService)srvc).hello());
+
+ ReadOnlyMetricRegistry metrics = findMetricRegistry(server.context().metric(), SRVC_NAME);
+
+ // Total service calls number.
+ int callsCnt = 0;
+
+ for (Metric m : metrics) {
+ if (m instanceof HistogramMetric)
+ callsCnt += sumHistogramEntries((HistogramMetric)m);
+ }
+
+ assertEquals(callsCnt, totalInstance);
+
+ // Add servers more than service instances.
+ servers.add(startGrid(gridNum++));
+
+ servers.add(startGrid(gridNum++));
+
+ awaitPartitionMapExchange();
+
+ int deployedCnt = 0;
+
+ int metricsCnt = 0;
+
+ for (IgniteEx ignite : servers) {
+ if (ignite.services().service(SRVC_NAME) != null)
Review comment:
You deprecate `IgniteServices#service` and use it here. Can we avoid usage of this method, as it marked as deprecated?
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/service/GridServiceMetricsTest.java
##########
@@ -0,0 +1,336 @@
+/*
+ * 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.ignite.internal.processors.service;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import com.google.common.collect.Iterables;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.metric.GridMetricManager;
+import org.apache.ignite.internal.processors.service.inner.MyService;
+import org.apache.ignite.internal.processors.service.inner.MyServiceFactory;
+import org.apache.ignite.services.Service;
+import org.apache.ignite.services.ServiceConfiguration;
+import org.apache.ignite.spi.metric.HistogramMetric;
+import org.apache.ignite.spi.metric.Metric;
+import org.apache.ignite.spi.metric.ReadOnlyMetricRegistry;
+import org.apache.ignite.spi.metric.jmx.JmxMetricExporterSpi;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.serviceMetricRegistryName;
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.sumHistogramEntries;
+import static org.apache.ignite.internal.processors.service.IgniteServiceProcessor.SERVICE_METRIC_REGISTRY;
+
+/**
+ * Tests metrics of service invocations.
+ */
+public class GridServiceMetricsTest extends GridCommonAbstractTest {
+ /** Number of service invocations. */
+ private static final int INVOKE_CNT = 50;
+
+ /** Service name used in the tests. */
+ private static final String SRVC_NAME = "TestService";
+
+ /** Error message of created metrics. */
+ private static final String METRICS_MUST_NOT_BE_CREATED = "Service metric registry must not be created.";
+
+ /** Utility holder of current grid number. */
+ private int gridNum;
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+
+ super.afterTest();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ // JMX metrics exposition to see actual namings and placement of the metrics.
+ cfg.setMetricExporterSpi(new JmxMetricExporterSpi());
+
+ return cfg;
+ }
+
+ /** Checks service metrics are enabled / disabled properly. */
+ @Test
+ public void testServiceMetricsEnabledDisabled() throws Exception {
+ IgniteEx ignite = startGrid();
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), 0, 1);
+
+ srvcCfg.setStatisticsEnabled(false);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+
+ ignite.services().cancel(SRVC_NAME);
+
+ srvcCfg.setStatisticsEnabled(true);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNotNull("Service metric registry must be created.",
+ findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+ }
+
+ /** Checks metric behaviour when launched several service instances. */
+ @Test
+ public void testMultipleDeployment() throws Throwable {
+ List<IgniteEx> servers = new ArrayList<>();
+
+ servers.add(startGrid(gridNum++));
+
+ IgniteEx server = servers.get(0);
+
+ IgniteEx client = startClientGrid(gridNum++);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(server.context().metric(), SERVICE_METRIC_REGISTRY));
+
+ int totalInstance = 2;
+
+ int perNode = 2;
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), totalInstance, perNode);
+
+ server.services().deploy(srvcCfg);
+
+ awaitPartitionMapExchange();
+
+ // Call proxies on the clients.
+ Stream.generate(() -> client.services().serviceProxy(SRVC_NAME, MyService.class, true))
Review comment:
Underthis logic it just executes `#hello` on the same node twice, as `sticky == true`. What do we test there?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/GridServiceProxy.java
##########
@@ -455,12 +463,39 @@ T proxy() {
/**
* @param mtd Method to invoke.
*/
- String methodName(Method mtd) {
+ static String methodName(Method mtd) {
PlatformServiceMethod ann = mtd.getDeclaredAnnotation(PlatformServiceMethod.class);
return ann == null ? mtd.getName() : ann.value();
}
+ /**
+ * Calls the target, measures and registers its duration.
+ *
+ * @param srvCtx Service context.
+ * @param mtdName Related method name.
+ * @param target Target to call and measure.
+ */
+ private static <T> T measureCall(
+ ServiceContextImpl srvCtx,
+ String mtdName,
+ Callable<T> target
+ ) throws Exception {
+ long startTime = System.nanoTime();
+
+ try {
+ return target.call();
+ }
+ finally {
+ long duration = System.nanoTime() - startTime;
+
+ HistogramMetricImpl histogram = srvCtx.metrics() == null ? null : srvCtx.metrics().findMetric(mtdName);
+
+ if (histogram != null)
Review comment:
What is an example of the situation when `metric == null`? If there are some, WDYT if we move this check on previous level with the `isStatisticsEnabled` call?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/impl/MetricUtils.java
##########
@@ -173,6 +174,34 @@ private static boolean ensureAllNamesNotEmpty(String... names) {
return names;
}
+ /**
+ * Gives proper name for service metric registry.
+ *
+ * @param srvcName Name of the service.
+ * @return registry name for service {@code srvcName}.
+ */
+ public static String serviceMetricRegistryName(String srvcName) {
Review comment:
This method used only within `IgniteServiceProcessor`, and use variable `SERVICE_METRIC_REGISTRY` from this class. I think we should move this method to the processor.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/IgniteServiceProcessor.java
##########
@@ -1967,4 +2008,34 @@ private SecurityException checkDeployPermissionDuringJoin(ClusterNode node, List
return null;
}
+
+ /**
+ * Creates metrics registry for the invocation histograms.
+ *
+ * @param srvcCtx ServiceContext.
+ * @return Created metric registry.
+ */
+ private ReadOnlyMetricRegistry createServiceMetrics(ServiceContextImpl srvcCtx) {
+ MetricRegistry metricRegistry = ctx.metric().registry(serviceMetricRegistryName(srvcCtx.name()));
+
+ for (Class<?> itf : allInterfaces(srvcCtx.service().getClass())) {
+ for (Method mtd : itf.getMethods()) {
+ if (metricIgnored(mtd.getDeclaringClass()) || metricRegistry.findMetric(mtd.getName()) != null)
+ continue;
+
+ metricRegistry.histogram(mtd.getName(), DEFAULT_INVOCATION_BOUNDS, DESCRIPTION_OF_INVOCATION_METRIC_PREF +
Review comment:
Why doesn't it check args of method name?
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/service/GridServiceMetricsTest.java
##########
@@ -0,0 +1,336 @@
+/*
+ * 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.ignite.internal.processors.service;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import com.google.common.collect.Iterables;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.metric.GridMetricManager;
+import org.apache.ignite.internal.processors.service.inner.MyService;
+import org.apache.ignite.internal.processors.service.inner.MyServiceFactory;
+import org.apache.ignite.services.Service;
+import org.apache.ignite.services.ServiceConfiguration;
+import org.apache.ignite.spi.metric.HistogramMetric;
+import org.apache.ignite.spi.metric.Metric;
+import org.apache.ignite.spi.metric.ReadOnlyMetricRegistry;
+import org.apache.ignite.spi.metric.jmx.JmxMetricExporterSpi;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.serviceMetricRegistryName;
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.sumHistogramEntries;
+import static org.apache.ignite.internal.processors.service.IgniteServiceProcessor.SERVICE_METRIC_REGISTRY;
+
+/**
+ * Tests metrics of service invocations.
+ */
+public class GridServiceMetricsTest extends GridCommonAbstractTest {
+ /** Number of service invocations. */
+ private static final int INVOKE_CNT = 50;
+
+ /** Service name used in the tests. */
+ private static final String SRVC_NAME = "TestService";
+
+ /** Error message of created metrics. */
+ private static final String METRICS_MUST_NOT_BE_CREATED = "Service metric registry must not be created.";
+
+ /** Utility holder of current grid number. */
+ private int gridNum;
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+
+ super.afterTest();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ // JMX metrics exposition to see actual namings and placement of the metrics.
+ cfg.setMetricExporterSpi(new JmxMetricExporterSpi());
+
+ return cfg;
+ }
+
+ /** Checks service metrics are enabled / disabled properly. */
+ @Test
+ public void testServiceMetricsEnabledDisabled() throws Exception {
+ IgniteEx ignite = startGrid();
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), 0, 1);
+
+ srvcCfg.setStatisticsEnabled(false);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+
+ ignite.services().cancel(SRVC_NAME);
+
+ srvcCfg.setStatisticsEnabled(true);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNotNull("Service metric registry must be created.",
+ findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+ }
+
+ /** Checks metric behaviour when launched several service instances. */
+ @Test
+ public void testMultipleDeployment() throws Throwable {
+ List<IgniteEx> servers = new ArrayList<>();
+
+ servers.add(startGrid(gridNum++));
+
+ IgniteEx server = servers.get(0);
+
+ IgniteEx client = startClientGrid(gridNum++);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(server.context().metric(), SERVICE_METRIC_REGISTRY));
+
+ int totalInstance = 2;
+
+ int perNode = 2;
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), totalInstance, perNode);
+
+ server.services().deploy(srvcCfg);
+
+ awaitPartitionMapExchange();
+
+ // Call proxies on the clients.
+ Stream.generate(() -> client.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .limit(totalInstance).forEach(srvc -> ((MyService)srvc).hello());
+
+ ReadOnlyMetricRegistry metrics = findMetricRegistry(server.context().metric(), SRVC_NAME);
+
+ // Total service calls number.
+ int callsCnt = 0;
+
+ for (Metric m : metrics) {
+ if (m instanceof HistogramMetric)
+ callsCnt += sumHistogramEntries((HistogramMetric)m);
+ }
+
+ assertEquals(callsCnt, totalInstance);
+
+ // Add servers more than service instances.
+ servers.add(startGrid(gridNum++));
+
+ servers.add(startGrid(gridNum++));
+
+ awaitPartitionMapExchange();
+
+ int deployedCnt = 0;
+
+ int metricsCnt = 0;
+
+ for (IgniteEx ignite : servers) {
+ if (ignite.services().service(SRVC_NAME) != null)
+ deployedCnt++;
+
+ if (findMetricRegistry(ignite.context().metric(), SRVC_NAME) != null)
+ metricsCnt++;
+ }
+
+ assertEquals(deployedCnt, metricsCnt);
+
+ assertEquals(metricsCnt, totalInstance);
+ }
+
+ /** Checks metric are created when service is deployed and removed when service is undeployed. */
+ @Test
+ public void testMetricsOnServiceDeployAndCancel() throws Exception {
+ List<IgniteEx> servers = startGrids(3, false);
+
+ // 2 services per node.
+ servers.get(0).services().deploy(serviceCfg(MyServiceFactory.create(), servers.size(), 2));
+
+ awaitPartitionMapExchange();
+
+ int expectedCnt = Arrays.stream(MyService.class.getDeclaredMethods()).map(Method::getName).collect(Collectors.toSet()).size();
+
+ // Make sure metrics are registered.
+ for (IgniteEx ignite : servers)
+ assertEquals(metricsCnt(ignite, SRVC_NAME), expectedCnt);
+
+ servers.get(0).services().cancel(SRVC_NAME);
+
+ awaitPartitionMapExchange();
+
+ for (IgniteEx ignite : servers)
+ assertEquals(metricsCnt(ignite, SRVC_NAME), 0);
+ }
+
+ /** Tests service metrics for single service instance. */
+ @Test
+ public void testServiceMetricsSingle() throws Throwable {
+ testServiceMetrics(1, 1, 1, 1);
+ }
+
+ /** Tests service metrics for multy service instance: one per server. */
+ @Test
+ public void testServiceMetricsMulty() throws Throwable {
+ testServiceMetrics(3, 3, 3, 1);
+ }
+
+ /** Tests service metrics for multy service instance: fewer that servers and clients. */
+ @Test
+ public void testServiceMetricsMultyFew() throws Throwable {
+ testServiceMetrics(4, 3, 2, 1);
+ }
+
+ /** Tests service metrics for multy service instance: serveral instances per node. */
+ @Test
+ public void testServiceMetricsMultyDuplicated() throws Throwable {
+ testServiceMetrics(3, 2, 3, 3);
+ }
+
+ /** Tests service metrics for multy service instance: serveral instances per node, total fewer that servers. */
+ @Test
+ public void testServiceMetricsMultyFewDuplicated() throws Throwable {
+ testServiceMetrics(5, 4, 3, 2);
+ }
+
+ /**
+ * Invokes service in various ways: from clients, servers, etc. Checks these calls reflect in the metrics.
+ *
+ * @param serverCnt Number of server nodes.
+ * @param clientCnt Number of client nodes.
+ * @param perClusterCnt Number of service instances per cluster.
+ * @param perNodeCnt Number of service instances per node.
+ */
+ private void testServiceMetrics(int serverCnt, int clientCnt, int perClusterCnt, int perNodeCnt) throws Throwable {
+ List<IgniteEx> servers = startGrids(serverCnt, false);
+
+ List<IgniteEx> clients = startGrids(clientCnt, true);
+
+ servers.get(0).services().deploy(serviceCfg(MyServiceFactory.create(), perClusterCnt, perNodeCnt));
+
+ awaitPartitionMapExchange();
+
+ List<MyService> serverStickyProxies = servers.stream()
+ .map(ignite -> (MyService)ignite.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .collect(Collectors.toList());
+
+ List<MyService> clientStickyProxies = clients.stream()
+ .map(ignite -> (MyService)ignite.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .collect(Collectors.toList());
+
+ long invokeCollector = 0;
+
+ // Call service through the server proxies.
+ for (int i = 0; i < INVOKE_CNT; ++i) {
+ // Call from server.
+ IgniteEx ignite = servers.get(i % servers.size());
+
+ callService4Times(ignite, serverStickyProxies.get(i % serverStickyProxies.size()));
+
+ // Call from client.
+ ignite = clients.get(i % clients.size());
+
+ callService4Times(ignite, clientStickyProxies.get(i % clientStickyProxies.size()));
+
+ invokeCollector += 8;
+ }
+
+ long invokesInMetrics = 0;
+
+ // Calculate and check invocations within the metrics.
+ for (IgniteEx ignite : servers) {
+ ReadOnlyMetricRegistry metrics = findMetricRegistry(ignite.context().metric(), SRVC_NAME);
+
+ // Metrics may not be deployed on this server node.
+ if (metrics == null)
+ continue;
+
+ for (Metric metric : metrics) {
+ if (metric instanceof HistogramMetric)
+ invokesInMetrics += sumHistogramEntries((HistogramMetric)metric);
Review comment:
Also, could we test long executions here?
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/service/GridServiceMetricsTest.java
##########
@@ -0,0 +1,336 @@
+/*
+ * 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.ignite.internal.processors.service;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import com.google.common.collect.Iterables;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.metric.GridMetricManager;
+import org.apache.ignite.internal.processors.service.inner.MyService;
+import org.apache.ignite.internal.processors.service.inner.MyServiceFactory;
+import org.apache.ignite.services.Service;
+import org.apache.ignite.services.ServiceConfiguration;
+import org.apache.ignite.spi.metric.HistogramMetric;
+import org.apache.ignite.spi.metric.Metric;
+import org.apache.ignite.spi.metric.ReadOnlyMetricRegistry;
+import org.apache.ignite.spi.metric.jmx.JmxMetricExporterSpi;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.serviceMetricRegistryName;
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.sumHistogramEntries;
+import static org.apache.ignite.internal.processors.service.IgniteServiceProcessor.SERVICE_METRIC_REGISTRY;
+
+/**
+ * Tests metrics of service invocations.
+ */
+public class GridServiceMetricsTest extends GridCommonAbstractTest {
+ /** Number of service invocations. */
+ private static final int INVOKE_CNT = 50;
+
+ /** Service name used in the tests. */
+ private static final String SRVC_NAME = "TestService";
+
+ /** Error message of created metrics. */
+ private static final String METRICS_MUST_NOT_BE_CREATED = "Service metric registry must not be created.";
+
+ /** Utility holder of current grid number. */
+ private int gridNum;
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+
+ super.afterTest();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ // JMX metrics exposition to see actual namings and placement of the metrics.
+ cfg.setMetricExporterSpi(new JmxMetricExporterSpi());
+
+ return cfg;
+ }
+
+ /** Checks service metrics are enabled / disabled properly. */
+ @Test
+ public void testServiceMetricsEnabledDisabled() throws Exception {
+ IgniteEx ignite = startGrid();
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), 0, 1);
+
+ srvcCfg.setStatisticsEnabled(false);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+
+ ignite.services().cancel(SRVC_NAME);
+
+ srvcCfg.setStatisticsEnabled(true);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNotNull("Service metric registry must be created.",
+ findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+ }
+
+ /** Checks metric behaviour when launched several service instances. */
+ @Test
+ public void testMultipleDeployment() throws Throwable {
+ List<IgniteEx> servers = new ArrayList<>();
+
+ servers.add(startGrid(gridNum++));
+
+ IgniteEx server = servers.get(0);
+
+ IgniteEx client = startClientGrid(gridNum++);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(server.context().metric(), SERVICE_METRIC_REGISTRY));
+
+ int totalInstance = 2;
+
+ int perNode = 2;
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), totalInstance, perNode);
+
+ server.services().deploy(srvcCfg);
+
+ awaitPartitionMapExchange();
+
+ // Call proxies on the clients.
+ Stream.generate(() -> client.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .limit(totalInstance).forEach(srvc -> ((MyService)srvc).hello());
+
+ ReadOnlyMetricRegistry metrics = findMetricRegistry(server.context().metric(), SRVC_NAME);
+
+ // Total service calls number.
+ int callsCnt = 0;
+
+ for (Metric m : metrics) {
+ if (m instanceof HistogramMetric)
+ callsCnt += sumHistogramEntries((HistogramMetric)m);
+ }
+
+ assertEquals(callsCnt, totalInstance);
+
+ // Add servers more than service instances.
+ servers.add(startGrid(gridNum++));
+
+ servers.add(startGrid(gridNum++));
+
+ awaitPartitionMapExchange();
+
+ int deployedCnt = 0;
+
+ int metricsCnt = 0;
+
+ for (IgniteEx ignite : servers) {
+ if (ignite.services().service(SRVC_NAME) != null)
+ deployedCnt++;
+
+ if (findMetricRegistry(ignite.context().metric(), SRVC_NAME) != null)
+ metricsCnt++;
+ }
+
+ assertEquals(deployedCnt, metricsCnt);
+
+ assertEquals(metricsCnt, totalInstance);
+ }
+
+ /** Checks metric are created when service is deployed and removed when service is undeployed. */
+ @Test
+ public void testMetricsOnServiceDeployAndCancel() throws Exception {
+ List<IgniteEx> servers = startGrids(3, false);
+
+ // 2 services per node.
+ servers.get(0).services().deploy(serviceCfg(MyServiceFactory.create(), servers.size(), 2));
+
+ awaitPartitionMapExchange();
+
+ int expectedCnt = Arrays.stream(MyService.class.getDeclaredMethods()).map(Method::getName).collect(Collectors.toSet()).size();
+
+ // Make sure metrics are registered.
+ for (IgniteEx ignite : servers)
+ assertEquals(metricsCnt(ignite, SRVC_NAME), expectedCnt);
+
+ servers.get(0).services().cancel(SRVC_NAME);
+
+ awaitPartitionMapExchange();
+
+ for (IgniteEx ignite : servers)
+ assertEquals(metricsCnt(ignite, SRVC_NAME), 0);
+ }
+
+ /** Tests service metrics for single service instance. */
+ @Test
+ public void testServiceMetricsSingle() throws Throwable {
+ testServiceMetrics(1, 1, 1, 1);
+ }
+
+ /** Tests service metrics for multy service instance: one per server. */
+ @Test
+ public void testServiceMetricsMulty() throws Throwable {
+ testServiceMetrics(3, 3, 3, 1);
+ }
+
+ /** Tests service metrics for multy service instance: fewer that servers and clients. */
+ @Test
+ public void testServiceMetricsMultyFew() throws Throwable {
+ testServiceMetrics(4, 3, 2, 1);
+ }
+
+ /** Tests service metrics for multy service instance: serveral instances per node. */
+ @Test
+ public void testServiceMetricsMultyDuplicated() throws Throwable {
+ testServiceMetrics(3, 2, 3, 3);
+ }
+
+ /** Tests service metrics for multy service instance: serveral instances per node, total fewer that servers. */
+ @Test
+ public void testServiceMetricsMultyFewDuplicated() throws Throwable {
+ testServiceMetrics(5, 4, 3, 2);
+ }
+
+ /**
+ * Invokes service in various ways: from clients, servers, etc. Checks these calls reflect in the metrics.
+ *
+ * @param serverCnt Number of server nodes.
+ * @param clientCnt Number of client nodes.
+ * @param perClusterCnt Number of service instances per cluster.
+ * @param perNodeCnt Number of service instances per node.
+ */
+ private void testServiceMetrics(int serverCnt, int clientCnt, int perClusterCnt, int perNodeCnt) throws Throwable {
+ List<IgniteEx> servers = startGrids(serverCnt, false);
+
+ List<IgniteEx> clients = startGrids(clientCnt, true);
+
+ servers.get(0).services().deploy(serviceCfg(MyServiceFactory.create(), perClusterCnt, perNodeCnt));
+
+ awaitPartitionMapExchange();
+
+ List<MyService> serverStickyProxies = servers.stream()
+ .map(ignite -> (MyService)ignite.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .collect(Collectors.toList());
+
+ List<MyService> clientStickyProxies = clients.stream()
+ .map(ignite -> (MyService)ignite.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .collect(Collectors.toList());
+
+ long invokeCollector = 0;
+
+ // Call service through the server proxies.
+ for (int i = 0; i < INVOKE_CNT; ++i) {
+ // Call from server.
+ IgniteEx ignite = servers.get(i % servers.size());
+
+ callService4Times(ignite, serverStickyProxies.get(i % serverStickyProxies.size()));
+
+ // Call from client.
+ ignite = clients.get(i % clients.size());
+
+ callService4Times(ignite, clientStickyProxies.get(i % clientStickyProxies.size()));
+
+ invokeCollector += 8;
+ }
+
+ long invokesInMetrics = 0;
+
+ // Calculate and check invocations within the metrics.
+ for (IgniteEx ignite : servers) {
+ ReadOnlyMetricRegistry metrics = findMetricRegistry(ignite.context().metric(), SRVC_NAME);
+
+ // Metrics may not be deployed on this server node.
+ if (metrics == null)
+ continue;
+
+ for (Metric metric : metrics) {
+ if (metric instanceof HistogramMetric)
+ invokesInMetrics += sumHistogramEntries((HistogramMetric)metric);
Review comment:
Metric description writes "hello() in milliseconds", but actually we count invokations. Is it expected behavior?
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/service/GridServiceMetricsTest.java
##########
@@ -0,0 +1,336 @@
+/*
+ * 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.ignite.internal.processors.service;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import com.google.common.collect.Iterables;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.metric.GridMetricManager;
+import org.apache.ignite.internal.processors.service.inner.MyService;
+import org.apache.ignite.internal.processors.service.inner.MyServiceFactory;
+import org.apache.ignite.services.Service;
+import org.apache.ignite.services.ServiceConfiguration;
+import org.apache.ignite.spi.metric.HistogramMetric;
+import org.apache.ignite.spi.metric.Metric;
+import org.apache.ignite.spi.metric.ReadOnlyMetricRegistry;
+import org.apache.ignite.spi.metric.jmx.JmxMetricExporterSpi;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.serviceMetricRegistryName;
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.sumHistogramEntries;
+import static org.apache.ignite.internal.processors.service.IgniteServiceProcessor.SERVICE_METRIC_REGISTRY;
+
+/**
+ * Tests metrics of service invocations.
+ */
+public class GridServiceMetricsTest extends GridCommonAbstractTest {
+ /** Number of service invocations. */
+ private static final int INVOKE_CNT = 50;
+
+ /** Service name used in the tests. */
+ private static final String SRVC_NAME = "TestService";
+
+ /** Error message of created metrics. */
+ private static final String METRICS_MUST_NOT_BE_CREATED = "Service metric registry must not be created.";
+
+ /** Utility holder of current grid number. */
+ private int gridNum;
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+
+ super.afterTest();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ // JMX metrics exposition to see actual namings and placement of the metrics.
+ cfg.setMetricExporterSpi(new JmxMetricExporterSpi());
+
+ return cfg;
+ }
+
+ /** Checks service metrics are enabled / disabled properly. */
+ @Test
+ public void testServiceMetricsEnabledDisabled() throws Exception {
+ IgniteEx ignite = startGrid();
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), 0, 1);
+
+ srvcCfg.setStatisticsEnabled(false);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+
+ ignite.services().cancel(SRVC_NAME);
+
+ srvcCfg.setStatisticsEnabled(true);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNotNull("Service metric registry must be created.",
+ findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+ }
+
+ /** Checks metric behaviour when launched several service instances. */
+ @Test
+ public void testMultipleDeployment() throws Throwable {
+ List<IgniteEx> servers = new ArrayList<>();
+
+ servers.add(startGrid(gridNum++));
+
+ IgniteEx server = servers.get(0);
+
+ IgniteEx client = startClientGrid(gridNum++);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(server.context().metric(), SERVICE_METRIC_REGISTRY));
+
+ int totalInstance = 2;
+
+ int perNode = 2;
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), totalInstance, perNode);
+
+ server.services().deploy(srvcCfg);
+
+ awaitPartitionMapExchange();
+
+ // Call proxies on the clients.
+ Stream.generate(() -> client.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .limit(totalInstance).forEach(srvc -> ((MyService)srvc).hello());
+
+ ReadOnlyMetricRegistry metrics = findMetricRegistry(server.context().metric(), SRVC_NAME);
+
+ // Total service calls number.
+ int callsCnt = 0;
+
+ for (Metric m : metrics) {
+ if (m instanceof HistogramMetric)
+ callsCnt += sumHistogramEntries((HistogramMetric)m);
+ }
+
+ assertEquals(callsCnt, totalInstance);
+
+ // Add servers more than service instances.
+ servers.add(startGrid(gridNum++));
+
+ servers.add(startGrid(gridNum++));
+
+ awaitPartitionMapExchange();
+
+ int deployedCnt = 0;
+
+ int metricsCnt = 0;
+
+ for (IgniteEx ignite : servers) {
+ if (ignite.services().service(SRVC_NAME) != null)
+ deployedCnt++;
+
+ if (findMetricRegistry(ignite.context().metric(), SRVC_NAME) != null)
+ metricsCnt++;
+ }
+
+ assertEquals(deployedCnt, metricsCnt);
+
+ assertEquals(metricsCnt, totalInstance);
+ }
+
+ /** Checks metric are created when service is deployed and removed when service is undeployed. */
+ @Test
+ public void testMetricsOnServiceDeployAndCancel() throws Exception {
+ List<IgniteEx> servers = startGrids(3, false);
+
+ // 2 services per node.
+ servers.get(0).services().deploy(serviceCfg(MyServiceFactory.create(), servers.size(), 2));
+
+ awaitPartitionMapExchange();
+
+ int expectedCnt = Arrays.stream(MyService.class.getDeclaredMethods()).map(Method::getName).collect(Collectors.toSet()).size();
Review comment:
expectedCnt equals 2, while actually we have 3 methods. Is it expected behavior?
##########
File path: modules/core/src/main/java/org/apache/ignite/services/ServiceConfiguration.java
##########
@@ -256,6 +261,31 @@ public ServiceConfiguration setNodeFilter(IgnitePredicate<ClusterNode> nodeFilte
return this;
}
+ /**
+ * Enables or disables statistics for the service. If enabled, durations of the service's methods are measured and
Review comment:
What do `durations` means, which dimension is it?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/IgniteServiceProcessor.java
##########
@@ -127,6 +135,21 @@
/** */
public static final String SVCS_VIEW_DESC = "Services";
+ /** Base name domain for invocation metrics. */
+ public static final String SERVICE_METRIC_REGISTRY = "Services";
+
+ /** Description for the service method invocation metric. */
+ private static final String DESCRIPTION_OF_INVOCATION_METRIC_PREF = "Duration in milliseconds of ";
+
+ /** Default bounds of invocation histogram in nanoseconds. */
+ public static final long[] DEFAULT_INVOCATION_BOUNDS = new long[] {
Review comment:
Is it possible to configure those bounds?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metric/impl/MetricUtils.java
##########
@@ -173,6 +174,34 @@ private static boolean ensureAllNamesNotEmpty(String... names) {
return names;
}
+ /**
+ * Gives proper name for service metric registry.
+ *
+ * @param srvcName Name of the service.
+ * @return registry name for service {@code srvcName}.
+ */
+ public static String serviceMetricRegistryName(String srvcName) {
+ return metricName(SERVICE_METRIC_REGISTRY, srvcName);
+ }
+
+ /**
+ * Count total of histogram values.
+ *
+ * @param histogram Histogram to traverse.
+ * @return Sum of all entries of {@code histogram} buckets.
+ */
+ public static long sumHistogramEntries(HistogramMetric histogram) {
Review comment:
As I can see, you use this method only within single test class. Let's move this logic to this test
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/service/GridServiceMetricsTest.java
##########
@@ -0,0 +1,336 @@
+/*
+ * 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.ignite.internal.processors.service;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import com.google.common.collect.Iterables;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.metric.GridMetricManager;
+import org.apache.ignite.internal.processors.service.inner.MyService;
+import org.apache.ignite.internal.processors.service.inner.MyServiceFactory;
+import org.apache.ignite.services.Service;
+import org.apache.ignite.services.ServiceConfiguration;
+import org.apache.ignite.spi.metric.HistogramMetric;
+import org.apache.ignite.spi.metric.Metric;
+import org.apache.ignite.spi.metric.ReadOnlyMetricRegistry;
+import org.apache.ignite.spi.metric.jmx.JmxMetricExporterSpi;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.serviceMetricRegistryName;
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.sumHistogramEntries;
+import static org.apache.ignite.internal.processors.service.IgniteServiceProcessor.SERVICE_METRIC_REGISTRY;
+
+/**
+ * Tests metrics of service invocations.
+ */
+public class GridServiceMetricsTest extends GridCommonAbstractTest {
+ /** Number of service invocations. */
+ private static final int INVOKE_CNT = 50;
+
+ /** Service name used in the tests. */
+ private static final String SRVC_NAME = "TestService";
+
+ /** Error message of created metrics. */
+ private static final String METRICS_MUST_NOT_BE_CREATED = "Service metric registry must not be created.";
+
+ /** Utility holder of current grid number. */
+ private int gridNum;
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+
+ super.afterTest();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ // JMX metrics exposition to see actual namings and placement of the metrics.
+ cfg.setMetricExporterSpi(new JmxMetricExporterSpi());
+
+ return cfg;
+ }
+
+ /** Checks service metrics are enabled / disabled properly. */
+ @Test
+ public void testServiceMetricsEnabledDisabled() throws Exception {
+ IgniteEx ignite = startGrid();
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), 0, 1);
+
+ srvcCfg.setStatisticsEnabled(false);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+
+ ignite.services().cancel(SRVC_NAME);
+
+ srvcCfg.setStatisticsEnabled(true);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNotNull("Service metric registry must be created.",
+ findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+ }
+
+ /** Checks metric behaviour when launched several service instances. */
+ @Test
+ public void testMultipleDeployment() throws Throwable {
+ List<IgniteEx> servers = new ArrayList<>();
+
+ servers.add(startGrid(gridNum++));
+
+ IgniteEx server = servers.get(0);
+
+ IgniteEx client = startClientGrid(gridNum++);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(server.context().metric(), SERVICE_METRIC_REGISTRY));
+
+ int totalInstance = 2;
+
+ int perNode = 2;
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), totalInstance, perNode);
+
+ server.services().deploy(srvcCfg);
+
+ awaitPartitionMapExchange();
+
+ // Call proxies on the clients.
+ Stream.generate(() -> client.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .limit(totalInstance).forEach(srvc -> ((MyService)srvc).hello());
+
+ ReadOnlyMetricRegistry metrics = findMetricRegistry(server.context().metric(), SRVC_NAME);
+
+ // Total service calls number.
+ int callsCnt = 0;
+
+ for (Metric m : metrics) {
+ if (m instanceof HistogramMetric)
+ callsCnt += sumHistogramEntries((HistogramMetric)m);
+ }
+
+ assertEquals(callsCnt, totalInstance);
+
+ // Add servers more than service instances.
+ servers.add(startGrid(gridNum++));
+
+ servers.add(startGrid(gridNum++));
+
+ awaitPartitionMapExchange();
+
+ int deployedCnt = 0;
+
+ int metricsCnt = 0;
+
+ for (IgniteEx ignite : servers) {
+ if (ignite.services().service(SRVC_NAME) != null)
+ deployedCnt++;
+
+ if (findMetricRegistry(ignite.context().metric(), SRVC_NAME) != null)
+ metricsCnt++;
+ }
+
+ assertEquals(deployedCnt, metricsCnt);
+
+ assertEquals(metricsCnt, totalInstance);
+ }
+
+ /** Checks metric are created when service is deployed and removed when service is undeployed. */
Review comment:
I don't find tests for redeploying of service. Let's add some if there isn't any.
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/service/GridServiceMetricsTest.java
##########
@@ -0,0 +1,336 @@
+/*
+ * 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.ignite.internal.processors.service;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import com.google.common.collect.Iterables;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.metric.GridMetricManager;
+import org.apache.ignite.internal.processors.service.inner.MyService;
+import org.apache.ignite.internal.processors.service.inner.MyServiceFactory;
+import org.apache.ignite.services.Service;
+import org.apache.ignite.services.ServiceConfiguration;
+import org.apache.ignite.spi.metric.HistogramMetric;
+import org.apache.ignite.spi.metric.Metric;
+import org.apache.ignite.spi.metric.ReadOnlyMetricRegistry;
+import org.apache.ignite.spi.metric.jmx.JmxMetricExporterSpi;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.serviceMetricRegistryName;
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.sumHistogramEntries;
+import static org.apache.ignite.internal.processors.service.IgniteServiceProcessor.SERVICE_METRIC_REGISTRY;
+
+/**
+ * Tests metrics of service invocations.
+ */
+public class GridServiceMetricsTest extends GridCommonAbstractTest {
+ /** Number of service invocations. */
+ private static final int INVOKE_CNT = 50;
+
+ /** Service name used in the tests. */
+ private static final String SRVC_NAME = "TestService";
+
+ /** Error message of created metrics. */
+ private static final String METRICS_MUST_NOT_BE_CREATED = "Service metric registry must not be created.";
+
+ /** Utility holder of current grid number. */
+ private int gridNum;
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+
+ super.afterTest();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ // JMX metrics exposition to see actual namings and placement of the metrics.
+ cfg.setMetricExporterSpi(new JmxMetricExporterSpi());
+
+ return cfg;
+ }
+
+ /** Checks service metrics are enabled / disabled properly. */
+ @Test
+ public void testServiceMetricsEnabledDisabled() throws Exception {
+ IgniteEx ignite = startGrid();
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), 0, 1);
+
+ srvcCfg.setStatisticsEnabled(false);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+
+ ignite.services().cancel(SRVC_NAME);
+
+ srvcCfg.setStatisticsEnabled(true);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNotNull("Service metric registry must be created.",
+ findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+ }
+
+ /** Checks metric behaviour when launched several service instances. */
+ @Test
+ public void testMultipleDeployment() throws Throwable {
+ List<IgniteEx> servers = new ArrayList<>();
+
+ servers.add(startGrid(gridNum++));
+
+ IgniteEx server = servers.get(0);
+
+ IgniteEx client = startClientGrid(gridNum++);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(server.context().metric(), SERVICE_METRIC_REGISTRY));
+
+ int totalInstance = 2;
+
+ int perNode = 2;
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), totalInstance, perNode);
+
+ server.services().deploy(srvcCfg);
+
+ awaitPartitionMapExchange();
+
+ // Call proxies on the clients.
+ Stream.generate(() -> client.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .limit(totalInstance).forEach(srvc -> ((MyService)srvc).hello());
+
+ ReadOnlyMetricRegistry metrics = findMetricRegistry(server.context().metric(), SRVC_NAME);
+
+ // Total service calls number.
+ int callsCnt = 0;
+
+ for (Metric m : metrics) {
+ if (m instanceof HistogramMetric)
+ callsCnt += sumHistogramEntries((HistogramMetric)m);
+ }
+
+ assertEquals(callsCnt, totalInstance);
+
+ // Add servers more than service instances.
+ servers.add(startGrid(gridNum++));
+
+ servers.add(startGrid(gridNum++));
+
+ awaitPartitionMapExchange();
+
+ int deployedCnt = 0;
+
+ int metricsCnt = 0;
+
+ for (IgniteEx ignite : servers) {
Review comment:
Can we use `G.allGrids()` instead of `servers` list?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/IgniteServiceProcessor.java
##########
@@ -1455,6 +1493,9 @@ void undeploy(@NotNull IgniteUuid srvcId) {
if (ctxs != null) {
synchronized (ctxs) {
+ if (!ctxs.isEmpty())
+ ctx.metric().remove(serviceMetricRegistryName(ctxs.iterator().next().name()));
Review comment:
Should we made this a part of the `cancel` method, before looping over service contexts? Currently you removing registry twice in your code, and both times before cancellation of services.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/IgniteServiceProcessor.java
##########
@@ -1967,4 +2008,34 @@ private SecurityException checkDeployPermissionDuringJoin(ClusterNode node, List
return null;
}
+
+ /**
+ * Creates metrics registry for the invocation histograms.
+ *
+ * @param srvcCtx ServiceContext.
+ * @return Created metric registry.
+ */
+ private ReadOnlyMetricRegistry createServiceMetrics(ServiceContextImpl srvcCtx) {
+ MetricRegistry metricRegistry = ctx.metric().registry(serviceMetricRegistryName(srvcCtx.name()));
Review comment:
In which cases do we need proceed when `metricRegistry != null`? Should we create a new one registry instead, if methods changed?
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/service/GridServiceMetricsTest.java
##########
@@ -0,0 +1,336 @@
+/*
+ * 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.ignite.internal.processors.service;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import com.google.common.collect.Iterables;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.metric.GridMetricManager;
+import org.apache.ignite.internal.processors.service.inner.MyService;
+import org.apache.ignite.internal.processors.service.inner.MyServiceFactory;
+import org.apache.ignite.services.Service;
+import org.apache.ignite.services.ServiceConfiguration;
+import org.apache.ignite.spi.metric.HistogramMetric;
+import org.apache.ignite.spi.metric.Metric;
+import org.apache.ignite.spi.metric.ReadOnlyMetricRegistry;
+import org.apache.ignite.spi.metric.jmx.JmxMetricExporterSpi;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.serviceMetricRegistryName;
+import static org.apache.ignite.internal.processors.metric.impl.MetricUtils.sumHistogramEntries;
+import static org.apache.ignite.internal.processors.service.IgniteServiceProcessor.SERVICE_METRIC_REGISTRY;
+
+/**
+ * Tests metrics of service invocations.
+ */
+public class GridServiceMetricsTest extends GridCommonAbstractTest {
+ /** Number of service invocations. */
+ private static final int INVOKE_CNT = 50;
+
+ /** Service name used in the tests. */
+ private static final String SRVC_NAME = "TestService";
+
+ /** Error message of created metrics. */
+ private static final String METRICS_MUST_NOT_BE_CREATED = "Service metric registry must not be created.";
+
+ /** Utility holder of current grid number. */
+ private int gridNum;
+
+ /** {@inheritDoc} */
+ @Override protected void afterTest() throws Exception {
+ stopAllGrids();
+
+ super.afterTest();
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ // JMX metrics exposition to see actual namings and placement of the metrics.
+ cfg.setMetricExporterSpi(new JmxMetricExporterSpi());
+
+ return cfg;
+ }
+
+ /** Checks service metrics are enabled / disabled properly. */
+ @Test
+ public void testServiceMetricsEnabledDisabled() throws Exception {
+ IgniteEx ignite = startGrid();
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), 0, 1);
+
+ srvcCfg.setStatisticsEnabled(false);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+
+ ignite.services().cancel(SRVC_NAME);
+
+ srvcCfg.setStatisticsEnabled(true);
+
+ ignite.services().deploy(srvcCfg);
+
+ assertNotNull("Service metric registry must be created.",
+ findMetricRegistry(ignite.context().metric(), SRVC_NAME));
+ }
+
+ /** Checks metric behaviour when launched several service instances. */
+ @Test
+ public void testMultipleDeployment() throws Throwable {
+ List<IgniteEx> servers = new ArrayList<>();
+
+ servers.add(startGrid(gridNum++));
+
+ IgniteEx server = servers.get(0);
+
+ IgniteEx client = startClientGrid(gridNum++);
+
+ assertNull(METRICS_MUST_NOT_BE_CREATED, findMetricRegistry(server.context().metric(), SERVICE_METRIC_REGISTRY));
+
+ int totalInstance = 2;
+
+ int perNode = 2;
+
+ ServiceConfiguration srvcCfg = serviceCfg(MyServiceFactory.create(), totalInstance, perNode);
+
+ server.services().deploy(srvcCfg);
+
+ awaitPartitionMapExchange();
+
+ // Call proxies on the clients.
+ Stream.generate(() -> client.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .limit(totalInstance).forEach(srvc -> ((MyService)srvc).hello());
+
+ ReadOnlyMetricRegistry metrics = findMetricRegistry(server.context().metric(), SRVC_NAME);
+
+ // Total service calls number.
+ int callsCnt = 0;
+
+ for (Metric m : metrics) {
+ if (m instanceof HistogramMetric)
+ callsCnt += sumHistogramEntries((HistogramMetric)m);
+ }
+
+ assertEquals(callsCnt, totalInstance);
+
+ // Add servers more than service instances.
+ servers.add(startGrid(gridNum++));
+
+ servers.add(startGrid(gridNum++));
+
+ awaitPartitionMapExchange();
+
+ int deployedCnt = 0;
+
+ int metricsCnt = 0;
+
+ for (IgniteEx ignite : servers) {
+ if (ignite.services().service(SRVC_NAME) != null)
+ deployedCnt++;
+
+ if (findMetricRegistry(ignite.context().metric(), SRVC_NAME) != null)
+ metricsCnt++;
+ }
+
+ assertEquals(deployedCnt, metricsCnt);
+
+ assertEquals(metricsCnt, totalInstance);
+ }
+
+ /** Checks metric are created when service is deployed and removed when service is undeployed. */
+ @Test
+ public void testMetricsOnServiceDeployAndCancel() throws Exception {
+ List<IgniteEx> servers = startGrids(3, false);
+
+ // 2 services per node.
+ servers.get(0).services().deploy(serviceCfg(MyServiceFactory.create(), servers.size(), 2));
+
+ awaitPartitionMapExchange();
+
+ int expectedCnt = Arrays.stream(MyService.class.getDeclaredMethods()).map(Method::getName).collect(Collectors.toSet()).size();
+
+ // Make sure metrics are registered.
+ for (IgniteEx ignite : servers)
+ assertEquals(metricsCnt(ignite, SRVC_NAME), expectedCnt);
+
+ servers.get(0).services().cancel(SRVC_NAME);
+
+ awaitPartitionMapExchange();
+
+ for (IgniteEx ignite : servers)
+ assertEquals(metricsCnt(ignite, SRVC_NAME), 0);
+ }
+
+ /** Tests service metrics for single service instance. */
+ @Test
+ public void testServiceMetricsSingle() throws Throwable {
+ testServiceMetrics(1, 1, 1, 1);
+ }
+
+ /** Tests service metrics for multy service instance: one per server. */
+ @Test
+ public void testServiceMetricsMulty() throws Throwable {
+ testServiceMetrics(3, 3, 3, 1);
+ }
+
+ /** Tests service metrics for multy service instance: fewer that servers and clients. */
+ @Test
+ public void testServiceMetricsMultyFew() throws Throwable {
+ testServiceMetrics(4, 3, 2, 1);
+ }
+
+ /** Tests service metrics for multy service instance: serveral instances per node. */
+ @Test
+ public void testServiceMetricsMultyDuplicated() throws Throwable {
+ testServiceMetrics(3, 2, 3, 3);
+ }
+
+ /** Tests service metrics for multy service instance: serveral instances per node, total fewer that servers. */
+ @Test
+ public void testServiceMetricsMultyFewDuplicated() throws Throwable {
+ testServiceMetrics(5, 4, 3, 2);
+ }
+
+ /**
+ * Invokes service in various ways: from clients, servers, etc. Checks these calls reflect in the metrics.
+ *
+ * @param serverCnt Number of server nodes.
+ * @param clientCnt Number of client nodes.
+ * @param perClusterCnt Number of service instances per cluster.
+ * @param perNodeCnt Number of service instances per node.
+ */
+ private void testServiceMetrics(int serverCnt, int clientCnt, int perClusterCnt, int perNodeCnt) throws Throwable {
+ List<IgniteEx> servers = startGrids(serverCnt, false);
+
+ List<IgniteEx> clients = startGrids(clientCnt, true);
+
+ servers.get(0).services().deploy(serviceCfg(MyServiceFactory.create(), perClusterCnt, perNodeCnt));
+
+ awaitPartitionMapExchange();
+
+ List<MyService> serverStickyProxies = servers.stream()
+ .map(ignite -> (MyService)ignite.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .collect(Collectors.toList());
+
+ List<MyService> clientStickyProxies = clients.stream()
+ .map(ignite -> (MyService)ignite.services().serviceProxy(SRVC_NAME, MyService.class, true))
+ .collect(Collectors.toList());
+
+ long invokeCollector = 0;
+
+ // Call service through the server proxies.
+ for (int i = 0; i < INVOKE_CNT; ++i) {
+ // Call from server.
+ IgniteEx ignite = servers.get(i % servers.size());
+
+ callService4Times(ignite, serverStickyProxies.get(i % serverStickyProxies.size()));
+
+ // Call from client.
+ ignite = clients.get(i % clients.size());
+
+ callService4Times(ignite, clientStickyProxies.get(i % clientStickyProxies.size()));
+
+ invokeCollector += 8;
+ }
+
+ long invokesInMetrics = 0;
+
+ // Calculate and check invocations within the metrics.
+ for (IgniteEx ignite : servers) {
+ ReadOnlyMetricRegistry metrics = findMetricRegistry(ignite.context().metric(), SRVC_NAME);
+
+ // Metrics may not be deployed on this server node.
+ if (metrics == null)
+ continue;
+
+ for (Metric metric : metrics) {
+ if (metric instanceof HistogramMetric)
+ invokesInMetrics += sumHistogramEntries((HistogramMetric)metric);
+ }
+ }
+
+ // Compare calls number and metrics number.
+ assertEquals("Calculated wrong service invocation number.", invokesInMetrics, invokeCollector);
Review comment:
Can we add tests that some public exporter sees our metrics correctly?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org