Bugfix: Use metric obj for key of LongContainer to fix split bug (#12664

)

* Use metric obj for key of LongContainer to fix split bug

* fix testcase

* fix

* add hashcode&equals for ApplicationMetric

* remove unuse

* remove import

dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/data/MethodStatComposite.java

@@ -17,8 +17,6 @@
 
 package org.apache.dubbo.metrics.data;
 
-import org.apache.dubbo.common.logger.Logger;
-import org.apache.dubbo.common.logger.LoggerFactory;
 import org.apache.dubbo.common.utils.CollectionUtils;
 import org.apache.dubbo.metrics.exception.MetricsNeverHappenException;
 import org.apache.dubbo.metrics.model.MethodMetric;
@@ -44,8 +42,6 @@
  */
 public class MethodStatComposite extends AbstractMetricsExport {
 
-    private static final Logger logger = LoggerFactory.getLogger(MethodStatComposite.class);
-
     public MethodStatComposite(ApplicationModel applicationModel) {
         super(applicationModel);
     }

dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/data/RtStatComposite.java

@@ -18,7 +18,11 @@
 package org.apache.dubbo.metrics.data;
 
 import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;
+import org.apache.dubbo.metrics.model.ApplicationMetric;
+import org.apache.dubbo.metrics.model.MethodMetric;
+import org.apache.dubbo.metrics.model.Metric;
 import org.apache.dubbo.metrics.model.MetricsCategory;
+import org.apache.dubbo.metrics.model.ServiceKeyMetric;
 import org.apache.dubbo.metrics.model.container.AtomicLongContainer;
 import org.apache.dubbo.metrics.model.container.LongAccumulatorContainer;
 import org.apache.dubbo.metrics.model.container.LongContainer;
@@ -30,12 +34,10 @@
 import org.apache.dubbo.metrics.report.AbstractMetricsExport;
 import org.apache.dubbo.rpc.Invocation;
 import org.apache.dubbo.rpc.model.ApplicationModel;
-import org.apache.dubbo.rpc.support.RpcUtils;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Map;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.LongAccumulator;
 import java.util.stream.Collectors;
@@ -81,21 +83,21 @@ private List<LongContainer<? extends Number>> initStats(MetricsPlaceValue placeV
 
     public void calcApplicationRt(String registryOpType, Long responseTime) {
         for (LongContainer container : rtStats.stream().filter(longContainer -> longContainer.specifyType(registryOpType)).collect(Collectors.toList())) {
-            Number current = (Number) ConcurrentHashMapUtils.computeIfAbsent(container, getAppName(), container.getInitFunc());
+            Number current = (Number) ConcurrentHashMapUtils.computeIfAbsent(container, new ApplicationMetric(getApplicationModel()), container.getInitFunc());
             container.getConsumerFunc().accept(responseTime, current);
         }
     }
 
     public void calcServiceKeyRt(String serviceKey, String registryOpType, Long responseTime) {
         for (LongContainer container : rtStats.stream().filter(longContainer -> longContainer.specifyType(registryOpType)).collect(Collectors.toList())) {
-            Number current = (Number) ConcurrentHashMapUtils.computeIfAbsent(container, serviceKey, container.getInitFunc());
+            Number current = (Number) ConcurrentHashMapUtils.computeIfAbsent(container, new ServiceKeyMetric(getApplicationModel(), serviceKey), container.getInitFunc());
             container.getConsumerFunc().accept(responseTime, current);
         }
     }
 
     public void calcMethodKeyRt(Invocation invocation, String registryOpType, Long responseTime) {
         for (LongContainer container : rtStats.stream().filter(longContainer -> longContainer.specifyType(registryOpType)).collect(Collectors.toList())) {
-            Number current = (Number) ConcurrentHashMapUtils.computeIfAbsent(container, invocation.getTargetServiceUniqueName() + "_" + RpcUtils.getMethodName(invocation), container.getInitFunc());
+            Number current = (Number) ConcurrentHashMapUtils.computeIfAbsent(container, new MethodMetric(getApplicationModel(), invocation), container.getInitFunc());
             container.getConsumerFunc().accept(responseTime, current);
         }
     }
@@ -104,8 +106,9 @@ public List<MetricSample> export(MetricsCategory category) {
         List<MetricSample> list = new ArrayList<>();
         for (LongContainer<? extends Number> rtContainer : rtStats) {
             MetricsKeyWrapper metricsKeyWrapper = rtContainer.getMetricsKeyWrapper();
-            for (Map.Entry<String, ? extends Number> entry : rtContainer.entrySet()) {
-                list.add(new GaugeMetricSample<>(metricsKeyWrapper.targetKey(), metricsKeyWrapper.targetDesc(), metricsKeyWrapper.tagName(getApplicationModel(), entry.getKey()), category, entry.getKey().intern(), value -> rtContainer.getValueSupplier().apply(value.intern())));
+            for (Metric key : rtContainer.keySet()) {
+                // Use keySet to obtain the original key instance reference of ConcurrentHashMap to avoid early recycling of the micrometer
+                list.add(new GaugeMetricSample<>(metricsKeyWrapper.targetKey(), metricsKeyWrapper.targetDesc(), key.getTags(), category, key, value -> rtContainer.getValueSupplier().apply(value)));
             }
         }
         return list;
@@ -115,4 +118,5 @@ public List<LongContainer<? extends Number>> getRtStats() {
         return rtStats;
     }
 
+
 }

dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/model/ApplicationMetric.java

@@ -23,6 +23,7 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 
 import static org.apache.dubbo.common.constants.MetricsConstants.TAG_APPLICATION_NAME;
 import static org.apache.dubbo.common.constants.MetricsConstants.TAG_APPLICATION_VERSION_KEY;
@@ -62,4 +63,17 @@ public Map<String, String> getTags() {
         tags.put(MetricsKey.METADATA_GIT_COMMITID_METRIC.getName(), commitId);
         return tags;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof ApplicationMetric)) return false;
+        ApplicationMetric that = (ApplicationMetric) o;
+        return Objects.equals(getApplicationName(), that.applicationModel.getApplicationName());
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(getApplicationName());
+    }
 }

dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/model/MethodMetric.java

@@ -69,7 +69,7 @@ public void setVersion(String version) {
     }
 
     public Map<String, String> getTags() {
-        Map<String, String> tags = MetricsSupport.methodTags(getApplicationModel(), getInterfaceName(), methodName);
+        Map<String, String> tags = MetricsSupport.methodTags(getApplicationModel(), getServiceKey(), methodName);
         tags.put(TAG_GROUP_KEY, group);
         tags.put(TAG_VERSION_KEY, version);
         return tags;
@@ -92,7 +92,7 @@ public String toString() {
         return "MethodMetric{" +
                 "applicationName='" + getApplicationName() + '\'' +
                 ", side='" + side + '\'' +
-                ", interfaceName='" + getInterfaceName() + '\'' +
+                ", interfaceName='" + getServiceKey() + '\'' +
                 ", methodName='" + methodName + '\'' +
                 ", group='" + group + '\'' +
                 ", version='" + version + '\'' +
@@ -104,11 +104,11 @@ public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         MethodMetric that = (MethodMetric) o;
-        return Objects.equals(getApplicationName(), that.getApplicationName()) && Objects.equals(side, that.side) && Objects.equals(getInterfaceName(), that.getInterfaceName()) && Objects.equals(methodName, that.methodName) && Objects.equals(group, that.group) && Objects.equals(version, that.version);
+        return Objects.equals(getApplicationName(), that.getApplicationName()) && Objects.equals(side, that.side) && Objects.equals(getServiceKey(), that.getServiceKey()) && Objects.equals(methodName, that.methodName) && Objects.equals(group, that.group) && Objects.equals(version, that.version);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(getApplicationName(), side, getInterfaceName(), methodName, group, version);
+        return Objects.hash(getApplicationName(), side, getServiceKey(), methodName, group, version);
     }
 }

dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/model/ServiceKeyMetric.java

@@ -25,20 +25,20 @@
  * Metric class for service.
  */
 public class ServiceKeyMetric extends ApplicationMetric {
-    private final String interfaceName;
+    private final String serviceKey;
 
     public ServiceKeyMetric(ApplicationModel applicationModel, String serviceKey) {
         super(applicationModel);
-        this.interfaceName = serviceKey;
+        this.serviceKey = serviceKey;
     }
 
     @Override
     public Map<String, String> getTags() {
-        return MetricsSupport.serviceTags(getApplicationModel(), interfaceName);
+        return MetricsSupport.serviceTags(getApplicationModel(), serviceKey);
     }
 
-    public String getInterfaceName() {
-        return interfaceName;
+    public String getServiceKey() {
+        return serviceKey;
     }
 
     @Override
@@ -55,21 +55,21 @@ public boolean equals(Object o) {
         if (!getApplicationName().equals(that.getApplicationName())) {
             return false;
         }
-        return interfaceName.equals(that.interfaceName);
+        return serviceKey.equals(that.serviceKey);
     }
 
     @Override
     public int hashCode() {
         int result = getApplicationName().hashCode();
-        result = 31 * result + interfaceName.hashCode();
+        result = 31 * result + serviceKey.hashCode();
         return result;
     }
 
     @Override
     public String toString() {
         return "ServiceKeyMetric{" +
                 "applicationName='" + getApplicationName() + '\'' +
-                ", serviceKey='" + interfaceName + '\'' +
+                ", serviceKey='" + serviceKey + '\'' +
                 '}';
     }
 }

dubbo-metrics/dubbo-metrics-api/src/main/java/org/apache/dubbo/metrics/model/container/LongContainer.java

@@ -17,6 +17,7 @@
 
 package org.apache.dubbo.metrics.model.container;
 
+import org.apache.dubbo.metrics.model.Metric;
 import org.apache.dubbo.metrics.model.key.MetricsKeyWrapper;
 import org.apache.dubbo.metrics.model.key.MetricsKey;
 import org.apache.dubbo.metrics.model.sample.GaugeMetricSample;
@@ -30,7 +31,7 @@
  * Long type data container
  * @param <N>
  */
-public class LongContainer<N extends Number> extends ConcurrentHashMap<String, N> {
+public class LongContainer<N extends Number> extends ConcurrentHashMap<Metric, N> {
 
     /**
      * Provide the metric type name
@@ -39,15 +40,15 @@ public class LongContainer<N extends Number> extends ConcurrentHashMap<String, N
     /**
      * The initial value corresponding to the key is generally 0 of different data types
      */
-    private final transient Function<String, N> initFunc;
+    private final transient Function<Metric, N> initFunc;
     /**
      * Statistical data calculation function, which can be self-increment, self-decrement, or more complex avg function
      */
     private final transient BiConsumer<Long, N> consumerFunc;
     /**
      * Data output function required by  {@link GaugeMetricSample GaugeMetricSample}
      */
-    private transient Function<String, Long> valueSupplier;
+    private transient Function<Metric, Long> valueSupplier;
 
 
     public LongContainer(MetricsKeyWrapper metricsKeyWrapper, Supplier<N> initFunc, BiConsumer<Long, N> consumerFunc) {
@@ -69,19 +70,19 @@ public boolean isKeyWrapper(MetricsKey metricsKey, String registryOpType) {
         return metricsKeyWrapper.isKey(metricsKey, registryOpType);
     }
 
-    public Function<String, N> getInitFunc() {
+    public Function<Metric, N> getInitFunc() {
         return initFunc;
     }
 
     public BiConsumer<Long, N> getConsumerFunc() {
         return consumerFunc;
     }
 
-    public Function<String, Long> getValueSupplier() {
+    public Function<Metric, Long> getValueSupplier() {
         return valueSupplier;
     }
 
-    public void setValueSupplier(Function<String, Long> valueSupplier) {
+    public void setValueSupplier(Function<Metric, Long> valueSupplier) {
         this.valueSupplier = valueSupplier;
     }
 

dubbo-metrics/dubbo-metrics-default/src/test/java/org/apache/dubbo/metrics/filter/MetricsFilterTest.java

@@ -71,7 +71,7 @@ class MetricsFilterTest {
     private static final String INTERFACE_NAME = "org.apache.dubbo.MockInterface";
     private static final String METHOD_NAME = "mockMethod";
     private static final String GROUP = "mockGroup";
-    private static final String VERSION = "1.0.0";
+    private static final String VERSION = "1.0.0_BETA";
     private String side;
 
     private AtomicBoolean initApplication = new AtomicBoolean(false);

dubbo-metrics/dubbo-metrics-default/src/test/java/org/apache/dubbo/metrics/metrics/model/MethodMetricTest.java

@@ -73,7 +73,7 @@ public static void setup() {
     @Test
     void test() {
         MethodMetric metric = new MethodMetric(applicationModel, invocation);
-        Assertions.assertEquals(metric.getInterfaceName(), interfaceName);
+        Assertions.assertEquals(metric.getServiceKey(), interfaceName);
         Assertions.assertEquals(metric.getMethodName(), methodName);
         Assertions.assertEquals(metric.getGroup(), group);
         Assertions.assertEquals(metric.getVersion(), version);

dubbo-metrics/dubbo-metrics-metadata/src/test/java/org/apache/dubbo/metrics/metadata/MetadataStatCompositeTest.java

@@ -22,6 +22,8 @@
 import org.apache.dubbo.metrics.data.BaseStatComposite;
 import org.apache.dubbo.metrics.data.RtStatComposite;
 import org.apache.dubbo.metrics.data.ServiceStatComposite;
+import org.apache.dubbo.metrics.model.ApplicationMetric;
+import org.apache.dubbo.metrics.model.Metric;
 import org.apache.dubbo.metrics.model.container.LongContainer;
 import org.apache.dubbo.metrics.model.key.MetricsKey;
 import org.apache.dubbo.rpc.model.ApplicationModel;
@@ -80,7 +82,7 @@ void testInit() {
             Assertions.assertEquals(v.get(), new AtomicLong(0L).get())));
         statComposite.getRtStatComposite().getRtStats().forEach(rtContainer ->
         {
-            for (Map.Entry<String, ? extends Number> entry : rtContainer.entrySet()) {
+            for (Map.Entry<Metric, ? extends Number> entry : rtContainer.entrySet()) {
                 Assertions.assertEquals(0L, rtContainer.getValueSupplier().apply(entry.getKey()));
             }
         });
@@ -95,9 +97,9 @@ void testIncrement() {
 
     @Test
     void testCalcRt() {
-        statComposite.calcApplicationRt( OP_TYPE_SUBSCRIBE.getType(), 10L);
+        statComposite.calcApplicationRt(OP_TYPE_SUBSCRIBE.getType(), 10L);
         Assertions.assertTrue(statComposite.getRtStatComposite().getRtStats().stream().anyMatch(longContainer -> longContainer.specifyType(OP_TYPE_SUBSCRIBE.getType())));
         Optional<LongContainer<? extends Number>> subContainer = statComposite.getRtStatComposite().getRtStats().stream().filter(longContainer -> longContainer.specifyType(OP_TYPE_SUBSCRIBE.getType())).findFirst();
-        subContainer.ifPresent(v -> Assertions.assertEquals(10L, v.get(applicationModel.getApplicationName()).longValue()));
+        subContainer.ifPresent(v -> Assertions.assertEquals(10L, v.get(new ApplicationMetric(applicationModel)).longValue()));
     }
 }

dubbo-metrics/dubbo-metrics-registry/src/test/java/org/apache/dubbo/metrics/registry/metrics/collector/RegistryStatCompositeTest.java

@@ -22,6 +22,8 @@
 import org.apache.dubbo.metrics.data.BaseStatComposite;
 import org.apache.dubbo.metrics.data.RtStatComposite;
 import org.apache.dubbo.metrics.data.ServiceStatComposite;
+import org.apache.dubbo.metrics.model.ApplicationMetric;
+import org.apache.dubbo.metrics.model.Metric;
 import org.apache.dubbo.metrics.model.MetricsCategory;
 import org.apache.dubbo.metrics.model.container.LongContainer;
 import org.apache.dubbo.metrics.model.sample.GaugeMetricSample;
@@ -92,7 +94,7 @@ void testInit() {
             Assertions.assertEquals(v.get(), new AtomicLong(0L).get())));
         statComposite.getRtStatComposite().getRtStats().forEach(rtContainer ->
         {
-            for (Map.Entry<String, ? extends Number> entry : rtContainer.entrySet()) {
+            for (Map.Entry<Metric, ? extends Number> entry : rtContainer.entrySet()) {
                 Assertions.assertEquals(0L, rtContainer.getValueSupplier().apply(entry.getKey()));
             }
         });
@@ -109,7 +111,7 @@ void testCalcRt() {
         statComposite.calcApplicationRt(OP_TYPE_NOTIFY.getType(), 10L);
         Assertions.assertTrue(statComposite.getRtStatComposite().getRtStats().stream().anyMatch(longContainer -> longContainer.specifyType(OP_TYPE_NOTIFY.getType())));
         Optional<LongContainer<? extends Number>> subContainer = statComposite.getRtStatComposite().getRtStats().stream().filter(longContainer -> longContainer.specifyType(OP_TYPE_NOTIFY.getType())).findFirst();
-        subContainer.ifPresent(v -> Assertions.assertEquals(10L, v.get(applicationName).longValue()));
+        subContainer.ifPresent(v -> Assertions.assertEquals(10L, v.get(new ApplicationMetric(applicationModel)).longValue()));
     }
 
     @Test