Fixed span clone concurrency issues (#520)

Co-authored-by: 钟海 <[email protected]>
sofastack · May 31, 2024 · 10639f1 · 10639f1
1 parent 478608a
commit 10639f1
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 14 deletions.
diff --git a/tracer-core/src/main/java/com/alipay/common/tracer/core/span/SofaTracerSpan.java b/tracer-core/src/main/java/com/alipay/common/tracer/core/span/SofaTracerSpan.java
@@ -36,10 +36,10 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentLinkedQueue;
 
 /**
  * SofaTracerSpan
@@ -61,7 +61,7 @@ public class SofaTracerSpan implements Span {
     /** tags for Number  */
     private final Map<String, Number>                       tagsWithNumber       = new ConcurrentHashMap<>();
 
-    private final List<LogData>                             logs                 = new LinkedList<>();
+    private final ConcurrentLinkedQueue<LogData>            logs                 = new ConcurrentLinkedQueue<>();
 
     private String                                          operationName        = StringUtils.EMPTY_STRING;
 
@@ -436,7 +436,7 @@ public SofaTracerSpanContext getSofaTracerSpanContext() {
         return sofaTracerSpanContext;
     }
 
-    public List<LogData> getLogs() {
+    public ConcurrentLinkedQueue<LogData> getLogs() {
         return logs;
     }
 

diff --git a/tracer-core/src/test/java/com/alipay/common/tracer/core/span/SofaTracerSpanTest.java b/tracer-core/src/test/java/com/alipay/common/tracer/core/span/SofaTracerSpanTest.java
@@ -26,6 +26,7 @@
 import com.alipay.common.tracer.core.tracertest.encoder.ClientSpanEncoder;
 import com.alipay.common.tracer.core.tracertest.encoder.ServerSpanEncoder;
 import com.alipay.common.tracer.core.utils.StringUtils;
+import com.google.common.collect.Lists;
 import io.opentracing.tag.Tags;
 import org.junit.After;
 import org.junit.Assert;
@@ -34,10 +35,13 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentLinkedQueue;
 
 import static org.junit.Assert.*;
 
@@ -101,7 +105,7 @@ public void testCloneInstance() {
         assertEquals(span.getTagsWithBool(), cloneSpan.getTagsWithBool());
         assertEquals(span.getStartTime(), cloneSpan.getStartTime());
         assertEquals(span.getEndTime(), cloneSpan.getEndTime());
-        assertEquals(span.getLogs(), cloneSpan.getLogs());
+        assertEquals(Lists.newArrayList(span.getLogs()), Lists.newArrayList(cloneSpan.getLogs()));
         assertEquals(span.getLogType(), cloneSpan.getLogType());
         assertEquals(span.getOperationName(), cloneSpan.getOperationName());
         assertSame(span.getParentSofaTracerSpan(), cloneSpan.getParentSofaTracerSpan());
@@ -236,7 +240,7 @@ public void testLogEventType() {
         sofaTracerSpan.log(valueStr.get(0));
         sofaTracerSpan.log(valueStr.get(1));
         sofaTracerSpan.log(valueStr.get(2));
-        List<LogData> logDataList = sofaTracerSpan.getLogs();
+        ConcurrentLinkedQueue<LogData> logDataList = sofaTracerSpan.getLogs();
         assertEquals(3, logDataList.size());
         for (LogData logData : logDataList) {
             String value = (String) logData.getFields().get(LogData.EVENT_TYPE_KEY);
@@ -259,7 +263,7 @@ public void testLogForCurrentTimeEventType() {
         sofaTracerSpan1.log(111, valueStr.get(0));
         sofaTracerSpan1.log(111, valueStr.get(1));
         sofaTracerSpan1.log(111, valueStr.get(2));
-        List<LogData> logDataList = sofaTracerSpan1.getLogs();
+        ConcurrentLinkedQueue<LogData> logDataList = sofaTracerSpan1.getLogs();
         assertEquals(3, logDataList.size());
         for (LogData logData : logDataList) {
             String value = (String) logData.getFields().get(LogData.EVENT_TYPE_KEY);
@@ -290,7 +294,8 @@ public void testLogForCurrentTimeMap() {
         testLogForCurrentTimeMapSpan.log(222, fields1);
         testLogForCurrentTimeMapSpan.log(222, fields2);
         testLogForCurrentTimeMapSpan.log(222, fields3);
-        List<LogData> logDataList = testLogForCurrentTimeMapSpan.getLogs();
+        ConcurrentLinkedQueue<LogData> queue = testLogForCurrentTimeMapSpan.getLogs();
+        ArrayList<LogData> logDataList = Lists.newArrayList(queue);
         assertEquals(4, logDataList.size());
         assertEquals(222, logDataList.get(0).getTime());
         assertTrue(logDataList.get(0).getFields().containsKey("key")
@@ -317,10 +322,10 @@ public void testLogMap() {
         Map<String, String> fields = new HashMap<>();
         fields.put("key", "value");
         testLogMap.log(222, fields);
-        List<LogData> logDataList = testLogMap.getLogs();
+        ConcurrentLinkedQueue<LogData> logDataList = testLogMap.getLogs();
         assertEquals(1, logDataList.size());
-        assertTrue(logDataList.get(0).getFields().containsKey("key")
-                   && logDataList.get(0).getFields().containsValue("value"));
+        assertTrue(logDataList.peek().getFields().containsKey("key")
+                   && logDataList.peek().getFields().containsValue("value"));
     }
 
     /**
@@ -334,7 +339,7 @@ public void testLogForEventNamePayload() {
 
         testLogForEventNamePayloadSpan.log("eventName", payload);
         //
-        Object load = testLogForEventNamePayloadSpan.getLogs().get(0).getFields().get("eventName");
+        Object load = testLogForEventNamePayloadSpan.getLogs().peek().getFields().get("eventName");
         assertSame(load, payload);
     }
 
@@ -348,7 +353,8 @@ public void testLogForCurrentTimeEventNamePayload() {
         span.log(222, "eventName222", "value222");
         span.log(333, "eventName333", "value333");
         span.log(444, "eventName444", "value444");
-        List<LogData> logDataList = span.getLogs();
+        ConcurrentLinkedQueue<LogData> queue = span.getLogs();
+        ArrayList<LogData> logDataList = Lists.newArrayList(queue);
         assertEquals(3, logDataList.size());
         assertEquals(222, logDataList.get(0).getTime());
         assertEquals(1, logDataList.get(0).getFields().size());

diff --git a/tracer-core/src/test/java/com/alipay/common/tracer/core/tracertest/SofaTracerTest.java b/tracer-core/src/test/java/com/alipay/common/tracer/core/tracertest/SofaTracerTest.java
@@ -143,10 +143,10 @@ public void testReportSpan() {
                 //Test print one only put one tag
                 assertTrue(contextStr.contains(Tags.SPAN_KIND.getKey())
                         && contextStr.contains(Tags.SPAN_KIND_CLIENT));
-            } catch (IOException e) {
+            } catch (IndexOutOfBoundsException | IOException e) {
                 throw new AssertionError(e);
             }
-        }, 500);
+        }, 5000);
     }
 
     /**