[#noissue] Refactor PingSession

collector/src/main/java/com/navercorp/pinpoint/collector/receiver/grpc/service/AgentLifecycleListener.java

@@ -20,8 +20,6 @@
 import com.navercorp.pinpoint.collector.service.AgentInfoService;
 import com.navercorp.pinpoint.collector.util.ManagedAgentLifeCycle;
 import com.navercorp.pinpoint.common.server.bo.AgentInfoBo;
-import com.navercorp.pinpoint.common.trace.ServiceType;
-import com.navercorp.pinpoint.grpc.Header;
 import com.navercorp.pinpoint.grpc.server.lifecycle.LifecycleListener;
 import com.navercorp.pinpoint.grpc.server.lifecycle.PingSession;
 import org.apache.logging.log4j.LogManager;
@@ -50,11 +48,10 @@ public AgentLifecycleListener(KeepAliveService lifecycleService, AgentInfoServic
     @Override
     public void connect(PingSession lifecycle) {
         logger.info("connect:{}", lifecycle);
-        final Header header = lifecycle.getHeader();
         try {
-            if (lifecycle.getServiceType() == ServiceType.UNDEFINED.getCode()) {
+            if (lifecycle.isUndefinedServiceType()) {
                 // fallback
-                final AgentInfoBo agentInfoBo = agentInfoService.getSimpleAgentInfo(header.getAgentId(), header.getAgentStartTime());
+                final AgentInfoBo agentInfoBo = agentInfoService.getSimpleAgentInfo(lifecycle.getAgentId(), lifecycle.getAgentStartTime());
                 logger.info("ServiceType is UNDEFINED. Fallback:AgentInfo lookup {} -> {}", lifecycle, agentInfoBo);
                 if (agentInfoBo != null) {
                     lifecycle.setServiceType(agentInfoBo.getServiceTypeCode());

collector/src/main/java/com/navercorp/pinpoint/collector/receiver/grpc/service/KeepAliveService.java

@@ -23,12 +23,10 @@
 import com.navercorp.pinpoint.collector.util.ManagedAgentLifeCycle;
 import com.navercorp.pinpoint.common.server.util.AgentEventType;
 import com.navercorp.pinpoint.common.server.util.AgentLifeCycleState;
-import com.navercorp.pinpoint.grpc.Header;
 import com.navercorp.pinpoint.grpc.server.lifecycle.PingSession;
 import com.navercorp.pinpoint.grpc.server.lifecycle.PingSessionRegistry;
-
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import java.util.Collection;
 import java.util.Objects;
@@ -60,11 +58,12 @@ public void updateState() {
         }
     }
 
-    private AgentProperty newChannelProperties(Header header, short serviceType) {
-        final String applicationName = header.getApplicationName();
-        final String agentId = header.getAgentId();
-        final long agentStartTime = header.getAgentStartTime();
-        return new DefaultAgentProperty(applicationName, serviceType, agentId, agentStartTime, header.getProperties());
+    private AgentProperty newChannelProperties(PingSession pingSession) {
+        final String applicationName = pingSession.getApplicationName();
+        final String agentId = pingSession.getAgentId();
+        final long agentStartTime = pingSession.getAgentStartTime();
+        short serviceType = pingSession.getServiceType();
+        return new DefaultAgentProperty(applicationName, serviceType, agentId, agentStartTime, pingSession.getProperties());
     }
 
     public void updateState(PingSession lifecycle, ManagedAgentLifeCycle managedAgentLifeCycle) {
@@ -75,24 +74,18 @@ public void updateState(PingSession lifecycle, ManagedAgentLifeCycle managedAgen
     }
 
     public void updateState(PingSession pingSession, boolean closeState, AgentLifeCycleState agentLifeCycleState, AgentEventType agentEventType) {
-        final Header header = pingSession.getHeader();
-        if (header == null) {
-            // TODO dump client ip for debug
-            logger.warn("Not found request header");
-            return;
-        }
 
         final long pingTimestamp = System.currentTimeMillis();
-        final long socketId = header.getSocketId();
+        final long socketId = pingSession.getSocketId();
         if (socketId == -1) {
             // TODO dump client ip for debug
-            logger.warn("SocketId not exist. header:{}", header);
+            logger.warn("SocketId not exist. pingSession:{}", pingSession);
             // skip
             return;
         }
 
         try {
-            final AgentProperty agentProperty = newChannelProperties(header, pingSession.getServiceType());
+            final AgentProperty agentProperty = newChannelProperties(pingSession);
             long eventIdentifier = AgentLifeCycleAsyncTaskService.createEventIdentifier((int)socketId, (int) pingSession.nextEventIdAllocator());
             this.agentLifeCycleAsyncTask.handleLifeCycleEvent(agentProperty , pingTimestamp, agentLifeCycleState, eventIdentifier);
             this.agentEventAsyncTask.handleEvent(agentProperty, pingTimestamp, agentEventType);
@@ -102,15 +95,8 @@ public void updateState(PingSession pingSession, boolean closeState, AgentLifeCy
     }
 
     public void updateState(PingSession pingSession) {
-        final Header header = pingSession.getHeader();
-        if (header == null) {
-            // TODO dump client ip for debug
-            logger.warn("Not found request header");
-            return;
-        }
-
         try {
-            final AgentProperty agentProperty = newChannelProperties(header, pingSession.getServiceType());
+            final AgentProperty agentProperty = newChannelProperties(pingSession);
             this.agentLifeCycleAsyncTask.handlePingEvent(agentProperty);
         } catch (Exception e) {
             logger.warn("Failed to update state. ping session={}", pingSession, e);

collector/src/main/java/com/navercorp/pinpoint/collector/service/async/AgentLifeCycleAsyncTaskService.java

@@ -24,8 +24,8 @@
 import com.navercorp.pinpoint.common.trace.ServiceType;
 import com.navercorp.pinpoint.common.util.BytesUtils;
 import com.navercorp.pinpoint.loader.service.ServiceTypeRegistryService;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.springframework.scheduling.annotation.Async;
 import org.springframework.stereotype.Service;
 
@@ -61,15 +61,7 @@ public void handleLifeCycleEvent(AgentProperty agentProperty, long eventTimestam
         Objects.requireNonNull(agentLifeCycleState, "agentLifeCycleState");
 
         final String agentId = agentProperty.getAgentId();
-        if (agentId == null) {
-            logger.warn("Failed to handle event of agent life cycle, agentId is null. agentProperty={}", agentProperty);
-            return;
-        }
         final String applicationName = agentProperty.getApplicationName();
-        if (applicationName == null) {
-            logger.warn("Failed to handle event of agent life cycle, applicationName is null. agentProperty={}", agentProperty);
-            return;
-        }
 
         final long startTimestamp = agentProperty.getStartTime();
         final AgentLifeCycleBo agentLifeCycleBo = new AgentLifeCycleBo(agentId, startTimestamp, eventTimestamp, eventIdentifier, agentLifeCycleState);

collector/src/main/java/com/navercorp/pinpoint/collector/service/async/DefaultAgentProperty.java

@@ -65,13 +65,12 @@ public Object get(String key) {
 
     @Override
     public String toString() {
-        final StringBuilder sb = new StringBuilder("DefaultAgentProperty{");
-        sb.append("applicationName='").append(applicationName).append('\'');
-        sb.append(", agentId='").append(agentId).append('\'');
-        sb.append(", agentStartTime=").append(agentStartTime);
-        sb.append(", properties=").append(properties);
-        sb.append(", serviceType=").append(serviceType);
-        sb.append('}');
-        return sb.toString();
+        return "DefaultAgentProperty{" +
+                "applicationName='" + applicationName + '\'' +
+                ", agentId='" + agentId + '\'' +
+                ", agentStartTime=" + agentStartTime +
+                ", properties=" + properties +
+                ", serviceType=" + serviceType +
+                '}';
     }
 }

grpc/src/main/java/com/navercorp/pinpoint/grpc/server/lifecycle/DefaultPingEventHandler.java

@@ -19,8 +19,8 @@
 import com.navercorp.pinpoint.grpc.Header;
 import com.navercorp.pinpoint.grpc.server.ServerContext;
 import com.navercorp.pinpoint.grpc.server.TransportMetadata;
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import java.util.Objects;
 
@@ -50,7 +50,7 @@ public void connect() {
 
         final Long transportId = transportMetadata.getTransportId();
         final Header header = ServerContext.getAgentInfo();
-        final PingSession pingSession = new PingSession(transportId, header);
+        final PingSession pingSession = PingSession.of(transportId, header);
         pingSession.setLastPingTimeMillis(System.currentTimeMillis());
         final PingSession oldSession = pingSessionRegistry.add(pingSession.getId(), pingSession);
         if (oldSession != null) {

grpc/src/main/java/com/navercorp/pinpoint/grpc/server/lifecycle/PingSession.java

@@ -19,6 +19,7 @@
 import com.navercorp.pinpoint.common.trace.ServiceType;
 import com.navercorp.pinpoint.grpc.Header;
 
+import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.atomic.AtomicLongFieldUpdater;
 
@@ -30,21 +31,36 @@ public class PingSession {
     private static final AtomicLongFieldUpdater<PingSession> UPDATER = AtomicLongFieldUpdater.newUpdater(PingSession.class, "eventIdAllocator");
 
     private final Long id;
-    private final Header header;
+    private final String applicationName;
+    private final String agentId;
+    private final long agentStartTime;
+    private final long socketId;
+    private final Map<String, Object> properties;
+
+    private short serviceType;
 
     private volatile long eventIdAllocator = 0;
 
-    private short serviceType = ServiceType.UNDEFINED.getCode();
     private boolean updated = false;
     private long lastPingTimeMillis;
 
-    public PingSession(Long id, Header header) {
-        this.id = Objects.requireNonNull(id, "transportMetadata");
-        this.header = Objects.requireNonNull(header, "header");
+    public static PingSession of(Long id, Header header) {
+        Objects.requireNonNull(id, "id");
+        Objects.requireNonNull(header, "header");
+
+        return new PingSession(id, header.getApplicationName(), header.getAgentId(), header.getAgentStartTime(),
+                (short) header.getServiceType(), header.getSocketId(), header.getProperties());
     }
 
-    public Header getHeader() {
-        return header;
+    public PingSession(Long id, String applicationName, String agentId, long agentStartTime, short serviceType, long socketId, Map<String, Object> properties) {
+        this.id = Objects.requireNonNull(id, "id");
+
+        this.applicationName = Objects.requireNonNull(applicationName, "applicationName");
+        this.agentId = Objects.requireNonNull(agentId, "agentId");
+        this.agentStartTime = agentStartTime;
+        this.serviceType = serviceType;
+        this.socketId = socketId;
+        this.properties = Objects.requireNonNull(properties, "properties");
     }
 
     public Long getId() {
@@ -56,18 +72,45 @@ public long nextEventIdAllocator() {
     }
 
     public short getServiceType() {
-        if (serviceType != ServiceType.UNDEFINED.getCode()) {
+        synchronized (this) {
             return serviceType;
         }
-        return (short) header.getServiceType();
     }
 
     public void setServiceType(short serviceType) {
-        if (header.getServiceType() == ServiceType.UNDEFINED.getCode()) {
-            this.serviceType = serviceType;
+        synchronized (this) {
+            if (this.serviceType == ServiceType.UNDEFINED.getCode()) {
+                this.serviceType = serviceType;
+            }
+        }
+    }
+
+    public boolean isUndefinedServiceType() {
+        synchronized (this) {
+            return this.serviceType == ServiceType.UNDEFINED.getCode();
         }
     }
 
+    public String getApplicationName() {
+        return applicationName;
+    }
+
+    public String getAgentId() {
+        return agentId;
+    }
+
+    public long getAgentStartTime() {
+        return agentStartTime;
+    }
+
+    public long getSocketId() {
+        return socketId;
+    }
+
+    public Map<String, Object> getProperties() {
+        return this.properties;
+    }
+
     // Flag to avoid duplication.
     public boolean isUpdated() {
         return updated;
@@ -89,7 +132,11 @@ public void setLastPingTimeMillis(long lastPingTimeMillis) {
     public String toString() {
         return "PingSession{" +
                 "id=" + id +
-                ", header=" + header +
+                ", applicationName='" + applicationName + '\'' +
+                ", agentId='" + agentId + '\'' +
+                ", agentStartTime=" + agentStartTime +
+                ", socketId=" + socketId +
+                ", properties=" + properties +
                 ", eventIdAllocator=" + eventIdAllocator +
                 ", serviceType=" + serviceType +
                 ", updated=" + updated +

grpc/src/test/java/com/navercorp/pinpoint/grpc/server/lifecycle/PingSessionTest.java

@@ -1,7 +1,6 @@
 package com.navercorp.pinpoint.grpc.server.lifecycle;
 
 import com.navercorp.pinpoint.common.trace.ServiceType;
-import com.navercorp.pinpoint.grpc.Header;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
@@ -11,9 +10,8 @@ class PingSessionTest {
 
     @Test
     void getServiceType() {
-        Header header = new Header("name", "agentId", "agentName", "appName",
-                ServiceType.SPRING.getCode(),  11, 22, Collections.emptyList());
-        PingSession session = new PingSession(1L, header);
+        PingSession session = new PingSession(1L, "name", "agentId", 1234,
+                ServiceType.SPRING.getCode(),  11, Collections.emptyMap());
 
         Assertions.assertEquals(ServiceType.SPRING.getCode(), session.getServiceType());
 
@@ -23,9 +21,8 @@ void getServiceType() {
 
     @Test
     void getServiceType_undefined() {
-        Header header = new Header("name", "agentId", "agentName", "appName",
-                ServiceType.UNDEFINED.getCode(),  11, 22, Collections.emptyList());
-        PingSession session = new PingSession(1L, header);
+        PingSession session = new PingSession(1L, "name", "agentId", 1234,
+                ServiceType.UNDEFINED.getCode(),  11, Collections.emptyMap());
 
         Assertions.assertEquals(ServiceType.UNDEFINED.getCode(), session.getServiceType());
 
@@ -35,10 +32,10 @@ void getServiceType_undefined() {
 
     @Test
     void nextEventIdAllocator() {
-        Header header = new Header("name", "agentId", "agentName", "appName",
-                ServiceType.SPRING.getCode(),  11, 22, Collections.emptyList());
 
-        PingSession session = new PingSession(1L, header);
+
+        PingSession session = new PingSession(1L, "name", "agentId", 1234,
+                ServiceType.SPRING.getCode(),  11, Collections.emptyMap());
 
         Assertions.assertEquals(1, session.nextEventIdAllocator());
         Assertions.assertEquals(2, session.nextEventIdAllocator());