diff --git a/CHANGES.md b/CHANGES.md index 589d5a20131f30e808b5e236c9de68f44b25f021..ce8c9fa1e7d43815c69c0c04b7694b5232b05dc1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -20,6 +20,7 @@ Release Notes. * Update the `timestamp` field type for `LogQuery`. * Support Zabbix protocol to receive agent metrics. * Update the Apdex metric combine calculator. +* Enhance `MeterSystem` to allow creating metrics with same `metricName` / `function` / `scope`. #### UI * Update selector scroller to show in all pages. diff --git a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java index 457d01726d27d321212efcbbbeabe3b154432fb3..dd0181e36967f678deaa5b0153917923363b1901 100644 --- a/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java +++ b/oap-server/analyzer/meter-analyzer/src/main/java/org/apache/skywalking/oap/meter/analyzer/Analyzer.java @@ -18,7 +18,6 @@ package org.apache.skywalking.oap.meter.analyzer; -import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import io.vavr.Tuple; import io.vavr.Tuple2; @@ -208,12 +207,12 @@ public class Analyzer { metricType = MetricType.labeled; } } - Preconditions.checkState(createMetric(ctx.getScopeType(), metricType.literal, ctx.getDownsampling())); + createMetric(ctx.getScopeType(), metricType.literal, ctx.getDownsampling()); } - private boolean createMetric(final ScopeType scopeType, final String dataType, final DownsamplingType downsamplingType) { + private void createMetric(final ScopeType scopeType, final String dataType, final DownsamplingType downsamplingType) { String functionName = String.format(FUNCTION_NAME_TEMP, downsamplingType.toString().toLowerCase(), Strings.capitalize(dataType)); - return meterSystem.create(metricName, functionName, scopeType); + meterSystem.create(metricName, functionName, scopeType); } private void send(final AcceptableValue v, final long time) { diff --git a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/MeterSystem.java b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/MeterSystem.java index ccd3e98aa36a3151d9efdbb94aa9a3dc3d92720e..8a400661af7ab786cc40b64248dbff036e60c7b5 100644 --- a/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/MeterSystem.java +++ b/oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/meter/MeterSystem.java @@ -99,11 +99,10 @@ public class MeterSystem implements Service { * * @param metricsName The name used as the storage eneity and in the query stage. * @param functionName The function provided through {@link MeterFunction}. - * @return true if created, false if it exists. * @throws IllegalArgumentException if the parameter can't match the expectation. * @throws UnexpectedException if binary code manipulation fails or stream core failure. */ - public synchronized boolean create(String metricsName, + public synchronized void create(String metricsName, String functionName, ScopeType type) throws IllegalArgumentException { final Class meterFunction = functionRegister.get(functionName); @@ -123,7 +122,7 @@ public class MeterSystem implements Service { } } try { - return create(metricsName, functionName, type, Class.forName(Objects.requireNonNull(acceptance).getTypeName())); + create(metricsName, functionName, type, Class.forName(Objects.requireNonNull(acceptance).getTypeName())); } catch (ClassNotFoundException e) { throw new IllegalArgumentException(e); } @@ -135,11 +134,10 @@ public class MeterSystem implements Service { * * @param metricsName The name used as the storage eneity and in the query stage. * @param functionName The function provided through {@link MeterFunction}. - * @return true if created, false if it exists. * @throws IllegalArgumentException if the parameter can't match the expectation. * @throws UnexpectedException if binary code manipulation fails or stream core failure. */ - public synchronized boolean create(String metricsName, + public synchronized void create(String metricsName, String functionName, ScopeType type, Class dataType) throws IllegalArgumentException { @@ -187,6 +185,20 @@ public class MeterSystem implements Service { throw new IllegalArgumentException("Function " + functionName + " can't be found by javaassist."); } final String className = formatName(metricsName); + + /** + * Check whether the metrics class is already defined or not + */ + try { + CtClass existingMetric = classPool.get(METER_CLASS_PACKAGE + className); + if (existingMetric.getSuperclass() != parentClass || type != meterPrototypes.get(metricsName).getScopeType()) { + throw new IllegalArgumentException(metricsName + " has been defined, but calculate function or/are scope type is/are different."); + } + log.info("Metric {} is already defined, so skip the metric creation.", metricsName); + return ; + } catch (NotFoundException e) { + } + CtClass metricsClass = classPool.makeClass(METER_CLASS_PACKAGE + className, parentClass); /** @@ -234,7 +246,6 @@ public class MeterSystem implements Service { log.error("Can't compile/load/init " + className + ".", e); throw new UnexpectedException(e.getMessage(), e); } - return true; } /** diff --git a/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/MeterSystemTest.java b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/MeterSystemTest.java new file mode 100644 index 0000000000000000000000000000000000000000..da9f3e79e01392e4dbe7033981b31f8086c0f18f --- /dev/null +++ b/oap-server/server-core/src/test/java/org/apache/skywalking/oap/server/core/analysis/meter/MeterSystemTest.java @@ -0,0 +1,89 @@ +/* + * 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.skywalking.oap.server.core.analysis.meter; + +import java.util.Map; +import org.apache.skywalking.oap.server.core.analysis.StreamDefinition; +import org.apache.skywalking.oap.server.core.analysis.worker.MetricsStreamProcessor; +import org.apache.skywalking.oap.server.core.storage.StorageException; +import org.apache.skywalking.oap.server.library.module.ModuleManager; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; +import org.powermock.reflect.Whitebox; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.spy; + +@RunWith(MockitoJUnitRunner.Silent.class) +public class MeterSystemTest { + + @Mock + private ModuleManager moduleManager; + private MeterSystem meterSystem; + + @Before + public void setup() throws StorageException { + meterSystem = spy(new MeterSystem(moduleManager)); + Whitebox.setInternalState(MetricsStreamProcessor.class, "PROCESSOR", + Mockito.spy(MetricsStreamProcessor.getInstance())); + doNothing().when(MetricsStreamProcessor.getInstance()).create(any(), (StreamDefinition) any(), any()); + } + + @Test + public void testCreate() { + // validate with same name, function and scope types + meterSystem.create("test_meter", "avg", ScopeType.SERVICE); + validateMeterDefinition("test_meter", Long.class, ScopeType.SERVICE); + meterSystem.create("test_meter", "avg", ScopeType.SERVICE); + validateMeterDefinition("test_meter", Long.class, ScopeType.SERVICE); + + // validate with same name, difference scope type + try { + meterSystem.create("test_meter", "avg", ScopeType.SERVICE_INSTANCE); + throw new IllegalStateException(); + } catch (IllegalArgumentException e) { + // If wrong arguments is means right + } + + // validate with same name, difference function + try { + meterSystem.create("test_meter", "avgLabeled", ScopeType.SERVICE); + throw new IllegalStateException(); + } catch (IllegalArgumentException e) { + // If wrong arguments is means right + } + } + + private void validateMeterDefinition(String meterName, Class dataType, ScopeType type) { + Map meterPrototypes = Whitebox.getInternalState(meterSystem, "meterPrototypes"); + Object meterDefinition = meterPrototypes.get(meterName); + Class realDataType = Whitebox.getInternalState(meterDefinition, "dataType"); + ScopeType realScopeTypes = Whitebox.getInternalState(meterDefinition, "scopeType"); + + Assert.assertEquals(dataType, realDataType); + Assert.assertEquals(type, realScopeTypes); + } + +}