Repository: pinpoint-apm/pinpoint
Issue: change AGENT_NAME_MAX_LEN cause error
Labels: bug
module:web

**What version of pinpoint are you using?**
v1.7.3

**Describe the bug**
when i change the param AGENT_NAME_MAX_LEN to 32 and APPLICATION_NAME_MAX_LEN remain 24 ,
com.navercorp.pinpoint.web.mapper.TraceIndexScatterMapper3#createDot the Dot reverseAcceptedTime read was 0, beacuse this
`long reverseAcceptedTime = BytesUtils.bytesToLong(cell.getRowArray(), cell.getRowOffset() + HBaseTables.APPLICATION_NAME_MAX_LEN + HBaseTables.APPLICATION_TRACE_INDEX_ROW_DISTRIBUTE_SIZE);
        `
there why use HBaseTables.APPLICATION_NAME_MAX_LEN  it is bug? i think there should use AGENT_NAME_MAX_LEN

my english no good,do you know what i mean?



Issue comments:
Hi @zfaster 
It might be a bug, though it may be the collector's serialization part that is the issue.
We'll look into it, thanks for reporting.

That said, I am not sure if changing **AGENT_NAME_MAX_LEN** to 32 is safe enough to ensure proper operation. (It will break any data that has been stored using the default length for one thing)
Please share your experience if you do decide to go with it anyway.
I want to send http header Pinpoint-TraceID、Pinpoint-SpanID from my browser client , that i can trace from browser to server ,I need to  Pinpoint-TraceID unique so have to use uuid and change AGENT_NAME_MAX_LEN to 32, do you have some other suggest if i want to do this 

Pull request review comments:

Patch that resolved the issue:
diff --git a/commons-server/src/main/java/com/navercorp/pinpoint/common/server/util/SpanUtils.java b/commons-server/src/main/java/com/navercorp/pinpoint/common/server/util/SpanUtils.java
index 85af2741620c..96e8614817c3 100644
--- a/commons-server/src/main/java/com/navercorp/pinpoint/common/server/util/SpanUtils.java
+++ b/commons-server/src/main/java/com/navercorp/pinpoint/common/server/util/SpanUtils.java
@@ -20,13 +20,11 @@
 
 import com.navercorp.pinpoint.common.buffer.AutomaticBuffer;
 import com.navercorp.pinpoint.common.buffer.Buffer;
-import com.navercorp.pinpoint.common.server.bo.BasicSpan;
 import com.navercorp.pinpoint.common.util.BytesUtils;
 import com.navercorp.pinpoint.common.util.TimeUtils;
 import com.navercorp.pinpoint.common.util.TransactionId;
 import com.navercorp.pinpoint.common.util.TransactionIdUtils;
 import com.navercorp.pinpoint.thrift.dto.TSpan;
-import com.navercorp.pinpoint.thrift.dto.TSpanChunk;
 
 /**
  * @author emeroad
@@ -35,28 +33,19 @@ public final class SpanUtils {
     private SpanUtils() {
     }
 
-    @Deprecated
-    public static byte[] getAgentIdTraceIndexRowKey(String agentId, long timestamp) {
-        if (agentId == null) {
-            throw new IllegalArgumentException("agentId must not null");
-        }
-        final byte[] bAgentId = BytesUtils.toBytes(agentId);
-        return RowKeyUtils.concatFixedByteAndLong(bAgentId, AGENT_NAME_MAX_LEN, TimeUtils.reverseTimeMillis(timestamp));
-    }
-
     public static byte[] getApplicationTraceIndexRowKey(String applicationName, long timestamp) {
         if (applicationName == null) {
-            throw new IllegalArgumentException("agentId must not null");
+            throw new IllegalArgumentException("applicationName must not null");
         }
         final byte[] bApplicationName = BytesUtils.toBytes(applicationName);
-        return RowKeyUtils.concatFixedByteAndLong(bApplicationName, AGENT_NAME_MAX_LEN, TimeUtils.reverseTimeMillis(timestamp));
+        return RowKeyUtils.concatFixedByteAndLong(bApplicationName, APPLICATION_NAME_MAX_LEN, TimeUtils.reverseTimeMillis(timestamp));
     }
 
-    public static byte[] getTraceIndexRowKey(byte[] agentId, long timestamp) {
-        if (agentId == null) {
-            throw new NullPointerException("agentId must not be null");
+    public static byte[] getApplicationTraceIndexRowKey(byte[] applicationName, long timestamp) {
+        if (applicationName == null) {
+            throw new NullPointerException("applicationName must not be null");
         }
-        return RowKeyUtils.concatFixedByteAndLong(agentId, AGENT_NAME_MAX_LEN, TimeUtils.reverseTimeMillis(timestamp));
+        return RowKeyUtils.concatFixedByteAndLong(applicationName, APPLICATION_NAME_MAX_LEN, TimeUtils.reverseTimeMillis(timestamp));
     }
 
     public static byte[] getVarTransactionId(TSpan span) {
@@ -76,42 +65,4 @@ public static byte[] getVarTransactionId(TSpan span) {
         buffer.putVLong(transactionId.getTransactionSequence());
         return buffer.getBuffer();
     }
-
-    @Deprecated
-    public static byte[] getTransactionId(TSpan span) {
-        if (span == null) {
-            throw new NullPointerException("span must not be null");
-        }
-        final byte[] transactionIdBytes = span.getTransactionId();
-        TransactionId transactionId = TransactionIdUtils.parseTransactionId(transactionIdBytes);
-        String agentId = transactionId.getAgentId();
-        if (agentId == null) {
-            agentId = span.getAgentId();
-        }
-        return BytesUtils.stringLongLongToBytes(agentId, AGENT_NAME_MAX_LEN, transactionId.getAgentStartTime(), transactionId.getTransactionSequence());
-
-    }
-
-    @Deprecated
-    public static byte[] getTransactionId(TSpanChunk spanChunk) {
-        if (spanChunk == null) {
-            throw new NullPointerException("spanChunk must not be null");
-        }
-        final byte[] transactionIdBytes = spanChunk.getTransactionId();
-        final TransactionId transactionId = TransactionIdUtils.parseTransactionId(transactionIdBytes);
-        String agentId = transactionId.getAgentId();
-        if (agentId == null) {
-            agentId = spanChunk.getAgentId();
-        }
-        return BytesUtils.stringLongLongToBytes(agentId, AGENT_NAME_MAX_LEN, transactionId.getAgentStartTime(), transactionId.getTransactionSequence());
-    }
-
-    @Deprecated
-    public static byte[] getTransactionId(BasicSpan basicSpan) {
-        if (basicSpan == null) {
-            throw new NullPointerException("basicSpan must not be null");
-        }
-        TransactionId transactionId = basicSpan.getTransactionId();
-        return BytesUtils.stringLongLongToBytes(transactionId.getAgentId(), AGENT_NAME_MAX_LEN, transactionId.getAgentStartTime(), transactionId.getTransactionSequence());
-    }
 }
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseApplicationTraceIndexDao.java b/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseApplicationTraceIndexDao.java
index 77e2215a474f..2bdc90f85a5d 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseApplicationTraceIndexDao.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseApplicationTraceIndexDao.java
@@ -232,8 +232,8 @@ private Scan createScan(String applicationName, Range range, boolean scanBackwar
         scan.setCaching(this.scanCacheSize);
 
         byte[] bApplicationName = Bytes.toBytes(applicationName);
-        byte[] traceIndexStartKey = SpanUtils.getTraceIndexRowKey(bApplicationName, range.getFrom());
-        byte[] traceIndexEndKey = SpanUtils.getTraceIndexRowKey(bApplicationName, range.getTo());
+        byte[] traceIndexStartKey = SpanUtils.getApplicationTraceIndexRowKey(bApplicationName, range.getFrom());
+        byte[] traceIndexEndKey = SpanUtils.getApplicationTraceIndexRowKey(bApplicationName, range.getTo());
 
         if (scanBackward) {
             // start key is replaced by end key because key has been reversed


Test patch:
diff --git a/commons-server/src/test/java/com/navercorp/pinpoint/common/server/util/SpanUtilsTest.java b/commons-server/src/test/java/com/navercorp/pinpoint/common/server/util/SpanUtilsTest.java
index de46906729f5..e89fddbdf82f 100644
--- a/commons-server/src/test/java/com/navercorp/pinpoint/common/server/util/SpanUtilsTest.java
+++ b/commons-server/src/test/java/com/navercorp/pinpoint/common/server/util/SpanUtilsTest.java
@@ -20,11 +20,8 @@
 
 import com.google.common.primitives.Longs;
 import com.navercorp.pinpoint.common.PinpointConstants;
-import com.navercorp.pinpoint.common.server.bo.SpanBo;
-import com.navercorp.pinpoint.common.server.bo.serializer.trace.v2.TraceRowKeyDecoderV2;
 import com.navercorp.pinpoint.common.util.BytesUtils;
 import com.navercorp.pinpoint.common.util.TimeUtils;
-import com.navercorp.pinpoint.common.util.TransactionId;
 import com.navercorp.pinpoint.thrift.dto.TSpan;
 
 import org.junit.Assert;
@@ -35,71 +32,57 @@
  */
 public class SpanUtilsTest {
     @Test
-    public void testGetTraceIndexRowKeyWhiteSpace() throws Exception {
-        String agentId = "test test";
+    public void testGetTraceIndexRowKeyWhiteSpace() {
+        String applicationName = "test test";
         long time = System.currentTimeMillis();
-        check(agentId, time);
+        check(applicationName, time);
     }
 
     @Test
-    public void testGetTraceIndexRowKey1() throws Exception {
-        String agentId = "test";
+    public void testGetTraceIndexRowKey1() {
+        String applicationName = "test";
         long time = System.currentTimeMillis();
-        check(agentId, time);
+        check(applicationName, time);
     }
 
     @Test
-    public void testGetTraceIndexRowKey2() throws Exception {
-        String agentId = "";
-        for (int i = 0; i < PinpointConstants.AGENT_NAME_MAX_LEN; i++) {
-            agentId += "1";
+    public void testGetTraceIndexRowKey2() {
+        String applicationName = "";
+        for (int i = 0; i < PinpointConstants.APPLICATION_NAME_MAX_LEN; i++) {
+            applicationName += "1";
         }
 
         long time = System.currentTimeMillis();
-        check(agentId, time);
+        check(applicationName, time);
     }
 
     @Test
-    public void testGetTraceIndexRowKey3() throws Exception {
-        String agentId = "";
-        for (int i = 0; i < PinpointConstants.AGENT_NAME_MAX_LEN + 1; i++) {
-            agentId += "1";
+    public void testGetTraceIndexRowKey3() {
+        String applicationName = "";
+        for (int i = 0; i < PinpointConstants.APPLICATION_NAME_MAX_LEN + 1; i++) {
+            applicationName += "1";
         }
 
         long time = System.currentTimeMillis();
         try {
-            check(agentId, time);
+            check(applicationName, time);
             Assert.fail("error");
         } catch (IndexOutOfBoundsException ignore) {
         }
     }
 
-    private void check(String agentId0, long l1) {
+    private void check(String applicationName, long l1) {
         TSpan span = new TSpan();
-        span.setAgentId(agentId0);
+        span.setApplicationName(applicationName);
         span.setStartTime(l1);
 
-        byte[] traceIndexRowKey = SpanUtils.getAgentIdTraceIndexRowKey(span.getAgentId(), span.getStartTime());
+        byte[] traceIndexRowKey = SpanUtils.getApplicationTraceIndexRowKey(span.getApplicationName(), span.getStartTime());
 
-        String agentId = BytesUtils.toString(traceIndexRowKey, 0, PinpointConstants.AGENT_NAME_MAX_LEN).trim();
-        Assert.assertEquals(agentId0, agentId);
+        String agentId = BytesUtils.toString(traceIndexRowKey, 0, PinpointConstants.APPLICATION_NAME_MAX_LEN).trim();
+        Assert.assertEquals(applicationName, agentId);
 
-        long time = Longs.fromByteArray(Arrays.copyOfRange(traceIndexRowKey, PinpointConstants.AGENT_NAME_MAX_LEN, PinpointConstants.AGENT_NAME_MAX_LEN + 8));
+        long time = Longs.fromByteArray(Arrays.copyOfRange(traceIndexRowKey, PinpointConstants.APPLICATION_NAME_MAX_LEN, PinpointConstants.APPLICATION_NAME_MAX_LEN + 8));
         time = TimeUtils.recoveryTimeMillis(time);
         Assert.assertEquals(time, l1);
     }
-
-    @Test
-    public void testGetTransactionId_BasicSpan() {
-        SpanBo spanBo = new SpanBo();
-        TransactionId spanTransactionId = new TransactionId("traceAgentId", System.currentTimeMillis(), 1111);
-        spanBo.setTransactionId(spanTransactionId);
-
-        byte[] transactionIdRowkey = SpanUtils.getTransactionId(spanBo);
-
-        TraceRowKeyDecoderV2 decoder = new TraceRowKeyDecoderV2();
-        TransactionId transactionId = decoder.readTransactionId(transactionIdRowkey);
-
-        Assert.assertEquals(transactionId, spanBo.getTransactionId());
-    }
 }