Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#11337] Improve performance for Active Agent #11342

Merged
merged 1 commit into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
import com.navercorp.pinpoint.web.alarm.DataCollectorCategory;
import com.navercorp.pinpoint.web.alarm.vo.Rule;
import com.navercorp.pinpoint.web.dao.ApplicationIndexDao;
import com.navercorp.pinpoint.web.service.AgentInfoService;
import com.navercorp.pinpoint.web.service.AlarmService;
import com.navercorp.pinpoint.web.service.component.ActiveAgentValidator;
import com.navercorp.pinpoint.web.vo.Application;
import jakarta.annotation.Nonnull;
import org.springframework.batch.item.ItemProcessor;
Expand All @@ -53,21 +53,21 @@

private final ApplicationIndexDao applicationIndexDao;

private final AgentInfoService agentInfoService;
private final ActiveAgentValidator activeAgentValidator;

private final CheckerRegistry checkerRegistry;

public AlarmProcessor(
DataCollectorFactory dataCollectorFactory,
AlarmService alarmService,
ApplicationIndexDao applicationIndexDao,
AgentInfoService agentInfoService,
ActiveAgentValidator activeAgentValidator,
CheckerRegistry checkerRegistry
) {
this.dataCollectorFactory = Objects.requireNonNull(dataCollectorFactory, "dataCollectorFactory");
this.alarmService = Objects.requireNonNull(alarmService, "alarmService");
this.applicationIndexDao = Objects.requireNonNull(applicationIndexDao, "applicationIndexDao");
this.agentInfoService = Objects.requireNonNull(agentInfoService, "agentInfoService");
this.activeAgentValidator = Objects.requireNonNull(activeAgentValidator, "activeAgentValidator");
this.checkerRegistry = Objects.requireNonNull(checkerRegistry, "checkerRegistry");
}

Expand Down Expand Up @@ -102,13 +102,17 @@

private Supplier<List<String>> getAgentIdsSupplier(Application application, long now) {
Range range = Range.between(now - activeDuration, now);
return Suppliers.memoize(() -> fetchActiveAgents(application.getName(), range));
return Suppliers.memoize(() -> fetchActiveAgents(application, range));
}

private List<String> fetchActiveAgents(String applicationId, Range activeRange) {
return applicationIndexDao.selectAgentIds(applicationId)
private List<String> fetchActiveAgents(Application application, Range activeRange) {
List<String> agentList = applicationIndexDao.selectAgentIds(application.getName());
return agentList

Check warning on line 110 in batch/src/main/java/com/navercorp/pinpoint/batch/alarm/AlarmProcessor.java

View check run for this annotation

Codecov / codecov/patch

batch/src/main/java/com/navercorp/pinpoint/batch/alarm/AlarmProcessor.java#L109-L110

Added lines #L109 - L110 were not covered by tests
.stream()
.filter(id -> agentInfoService.isActiveAgent(id, activeRange))
.filter(id -> {
Application app = new Application(id, application.getServiceType());
return activeAgentValidator.isActiveAgent(app, activeRange);

Check warning on line 114 in batch/src/main/java/com/navercorp/pinpoint/batch/alarm/AlarmProcessor.java

View check run for this annotation

Codecov / codecov/patch

batch/src/main/java/com/navercorp/pinpoint/batch/alarm/AlarmProcessor.java#L112-L114

Added lines #L112 - L114 were not covered by tests
})
.toList();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
import com.navercorp.pinpoint.batch.common.BatchProperties;
import com.navercorp.pinpoint.common.server.util.time.Range;
import com.navercorp.pinpoint.web.dao.ApplicationIndexDao;
import com.navercorp.pinpoint.web.service.AgentInfoService;
import com.navercorp.pinpoint.web.service.component.ActiveAgentValidator;
import jakarta.annotation.Nonnull;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.springframework.batch.item.ItemProcessor;

import jakarta.annotation.Nonnull;
import java.util.Objects;
import java.util.concurrent.TimeUnit;

Expand All @@ -35,16 +35,16 @@
private final Logger logger = LogManager.getLogger(this.getClass());

private final ApplicationIndexDao applicationIndexDao;
private final AgentInfoService agentInfoService;
private final ActiveAgentValidator activeAgentValidator;
private final long duration;

public AgentCountProcessor(
ApplicationIndexDao applicationIndexDao,
AgentInfoService agentInfoService,
ActiveAgentValidator activeAgentValidator,
BatchProperties batchProperties
) {
this.applicationIndexDao = Objects.requireNonNull(applicationIndexDao, "applicationIndexDao");
this.agentInfoService = Objects.requireNonNull(agentInfoService, "agentInfoService");
this.activeAgentValidator = Objects.requireNonNull(activeAgentValidator, "activeAgentValidator");

Check warning on line 47 in batch/src/main/java/com/navercorp/pinpoint/batch/job/AgentCountProcessor.java

View check run for this annotation

Codecov / codecov/patch

batch/src/main/java/com/navercorp/pinpoint/batch/job/AgentCountProcessor.java#L47

Added line #L47 was not covered by tests

long durationDays = batchProperties.getCleanupInactiveAgentsDurationDays();
this.duration = TimeUnit.DAYS.toMillis(durationDays);
Expand All @@ -63,6 +63,6 @@
private boolean isActive(String agentId) {
long now = System.currentTimeMillis();
Range range = Range.between(now - duration, now);
return agentInfoService.isActiveAgent(agentId, range);
return activeAgentValidator.isActiveAgent(agentId, range);

Check warning on line 66 in batch/src/main/java/com/navercorp/pinpoint/batch/job/AgentCountProcessor.java

View check run for this annotation

Codecov / codecov/patch

batch/src/main/java/com/navercorp/pinpoint/batch/job/AgentCountProcessor.java#L66

Added line #L66 was not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import com.navercorp.pinpoint.common.server.util.time.Range;
import com.navercorp.pinpoint.web.dao.ApplicationIndexDao;
import com.navercorp.pinpoint.web.service.AgentInfoService;
import com.navercorp.pinpoint.web.service.component.ActiveAgentValidator;

import java.util.List;
import java.util.Objects;
Expand All @@ -29,14 +29,14 @@
public class BatchAgentServiceImpl implements BatchAgentService {

private final ApplicationIndexDao applicationIndexDao;
private final AgentInfoService agentInfoService;
private final ActiveAgentValidator activeAgentService;

public BatchAgentServiceImpl(
ApplicationIndexDao applicationIndexDao,
AgentInfoService agentInfoService
ActiveAgentValidator activeAgentService
) {
this.applicationIndexDao = Objects.requireNonNull(applicationIndexDao, "applicationIndexDao");
this.agentInfoService = Objects.requireNonNull(agentInfoService, "agentInfoService");
this.activeAgentService = Objects.requireNonNull(activeAgentService, "activeAgentService");

Check warning on line 39 in batch/src/main/java/com/navercorp/pinpoint/batch/service/BatchAgentServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

batch/src/main/java/com/navercorp/pinpoint/batch/service/BatchAgentServiceImpl.java#L39

Added line #L39 was not covered by tests
}

@Override
Expand All @@ -46,7 +46,7 @@

@Override
public boolean isActive(String agentId, Range range) {
return this.agentInfoService.isActiveAgent(agentId, range);
return this.activeAgentService.isActiveAgent(agentId, range);

Check warning on line 49 in batch/src/main/java/com/navercorp/pinpoint/batch/service/BatchAgentServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

batch/src/main/java/com/navercorp/pinpoint/batch/service/BatchAgentServiceImpl.java#L49

Added line #L49 was not covered by tests
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
import com.navercorp.pinpoint.web.alarm.CheckerCategory;
import com.navercorp.pinpoint.web.alarm.vo.Rule;
import com.navercorp.pinpoint.web.dao.ApplicationIndexDao;
import com.navercorp.pinpoint.web.service.AgentInfoService;
import com.navercorp.pinpoint.web.service.AlarmService;
import com.navercorp.pinpoint.web.service.component.ActiveAgentValidator;
import com.navercorp.pinpoint.web.vo.Application;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand Down Expand Up @@ -45,7 +45,7 @@ public class AlarmProcessorTest {
private ApplicationIndexDao applicationIndexDao;

@Mock
private AgentInfoService agentInfoService;
private ActiveAgentValidator activeAgentValidator;

@Mock
private AgentStatDataCollector agentStatDataCollector;
Expand All @@ -63,7 +63,7 @@ public void shouldSkipIfNoRule() {

when(alarmService.selectRuleByApplicationId(SERVICE_NAME)).thenReturn(List.of());

AlarmProcessor proc = new AlarmProcessor(dataCollectorFactory, alarmService, applicationIndexDao, agentInfoService, checkerRegistry);
AlarmProcessor proc = new AlarmProcessor(dataCollectorFactory, alarmService, applicationIndexDao, activeAgentValidator, checkerRegistry);
AppAlarmChecker checker = proc.process(app);

assertNull(checker, "should be skipped");
Expand All @@ -82,7 +82,7 @@ public void test() {
when(agentStatDataCollector.getHeapUsageRate()).thenReturn(heapUsageRate);

// Executions
AlarmProcessor processor = new AlarmProcessor(dataCollectorFactory, alarmService, applicationIndexDao, agentInfoService, checkerRegistry);
AlarmProcessor processor = new AlarmProcessor(dataCollectorFactory, alarmService, applicationIndexDao, activeAgentValidator, checkerRegistry);
AppAlarmChecker appChecker = processor.process(application);

// Validations
Expand Down
5 changes: 5 additions & 0 deletions web/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,11 @@
<groupId>jakarta.xml.bind</groupId>
<artifactId>jakarta.xml.bind-api</artifactId>
</dependency>
<dependency>
<groupId>org.semver4j</groupId>
<artifactId>semver4j</artifactId>
<version>5.3.0</version>
</dependency>

<!-- Test dependencies -->
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.navercorp.pinpoint.common.server.util.time.Range;
import com.navercorp.pinpoint.common.util.CollectionUtils;
import com.navercorp.pinpoint.web.dao.ApplicationIndexDao;
import com.navercorp.pinpoint.web.service.component.ActiveAgentValidator;
import com.navercorp.pinpoint.web.vo.Application;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand All @@ -44,11 +45,11 @@ public class AdminServiceImpl implements AdminService {

private final ApplicationIndexDao applicationIndexDao;

private final AgentInfoService agentInfoService;
private final ActiveAgentValidator activeAgentService;

public AdminServiceImpl(ApplicationIndexDao applicationIndexDao, AgentInfoService agentInfoService) {
public AdminServiceImpl(ApplicationIndexDao applicationIndexDao, ActiveAgentValidator activeAgentService) {
this.applicationIndexDao = Objects.requireNonNull(applicationIndexDao, "applicationIndexDao");
this.agentInfoService = Objects.requireNonNull(agentInfoService, "agentInfoService");
this.activeAgentService = Objects.requireNonNull(activeAgentService, "activeAgentValidator");
}

@Override
Expand Down Expand Up @@ -184,7 +185,7 @@ private boolean isInactiveAgent(String agentId, int durationDays) {
long now = System.currentTimeMillis();
Range range = Range.between(now - TimeUnit.DAYS.toMillis(durationDays), now);

return !this.agentInfoService.isActiveAgent(agentId, range);
return !this.activeAgentService.isActiveAgent(agentId, range);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ public interface AgentInfoService {

List<Optional<AgentStatus>> getAgentStatus(AgentStatusQuery query);

boolean isActiveAgent(String agentId, Range range);

InspectorTimeline getAgentStatusTimeline(String agentId, Range range, int... excludeAgentEventTypeCodes);

boolean isExistAgentId(String agentId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,16 @@

package com.navercorp.pinpoint.web.service;

import com.navercorp.pinpoint.common.server.bo.stat.JvmGcBo;
import com.navercorp.pinpoint.common.server.util.AgentEventType;
import com.navercorp.pinpoint.common.server.util.time.BackwardRangeSplitter;
import com.navercorp.pinpoint.common.server.util.time.DateTimeUtils;
import com.navercorp.pinpoint.common.server.util.time.Range;
import com.navercorp.pinpoint.common.server.util.time.RangeSplitter;
import com.navercorp.pinpoint.web.dao.AgentInfoDao;
import com.navercorp.pinpoint.web.dao.AgentLifeCycleDao;
import com.navercorp.pinpoint.web.dao.ApplicationIndexDao;
import com.navercorp.pinpoint.web.dao.stat.AgentStatDao;
import com.navercorp.pinpoint.web.filter.agent.AgentEventFilter;
import com.navercorp.pinpoint.web.hyperlink.HyperLinkFactory;
import com.navercorp.pinpoint.web.service.component.ActiveAgentValidator;
import com.navercorp.pinpoint.web.service.stat.AgentWarningStatService;
import com.navercorp.pinpoint.web.vo.AgentEvent;
import com.navercorp.pinpoint.web.vo.Application;
Expand Down Expand Up @@ -91,25 +89,23 @@

private final AgentLifeCycleDao agentLifeCycleDao;

private final AgentStatDao<JvmGcBo> jvmGcDao;
private final HyperLinkFactory hyperLinkFactory;
private final ActiveAgentValidator activeAgentValidator;

// legacy node agent support
private final List<Integer> legacyAgent = List.of(1400, 1401);

public AgentInfoServiceImpl(AgentEventService agentEventService,
AgentWarningStatService agentWarningStatService,
ApplicationIndexDao applicationIndexDao,
AgentInfoDao agentInfoDao,
AgentLifeCycleDao agentLifeCycleDao,
AgentStatDao<JvmGcBo> jvmGcDao,
ActiveAgentValidator activeAgentValidator,
HyperLinkFactory hyperLinkFactory) {
this.agentEventService = Objects.requireNonNull(agentEventService, "agentEventService");
this.agentWarningStatService = Objects.requireNonNull(agentWarningStatService, "agentWarningStatService");
this.applicationIndexDao = Objects.requireNonNull(applicationIndexDao, "applicationIndexDao");
this.agentInfoDao = Objects.requireNonNull(agentInfoDao, "agentInfoDao");
this.agentLifeCycleDao = Objects.requireNonNull(agentLifeCycleDao, "agentLifeCycleDao");
this.jvmGcDao = Objects.requireNonNull(jvmGcDao, "jvmGcDao");
this.activeAgentValidator = Objects.requireNonNull(activeAgentValidator, "activeAgentValidator");

Check warning on line 108 in web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java#L108

Added line #L108 was not covered by tests
this.hyperLinkFactory = Objects.requireNonNull(hyperLinkFactory, "hyperLinkFactory");
}

Expand Down Expand Up @@ -198,32 +194,15 @@
logger.trace("agentStatusFilter=true");
return true;
}
if (isActiveAgentByPing(agentInfo.getAgentId(), range)) {
logger.trace("agent ping=true");
Application agent = new Application(agentInfo.getAgentId(), agentInfo.getServiceType());
String agentVersion = agentInfo.getAgentVersion();

Check warning on line 198 in web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java#L197-L198

Added lines #L197 - L198 were not covered by tests
if (activeAgentValidator.isActiveAgent(agent, agentVersion, range)) {
return true;
}
if (legacyAgentSupport(agentAndStatus)) {
if (isActiveAgentByGcStat(agentInfo.getAgentId(), range)) {
logger.trace("agent gcStat=true");
return true;
}
}
logger.trace("isActiveAgentPredicate=false");
return false;
}

private boolean legacyAgentSupport(AgentAndStatus agentAndStatus) {
AgentInfo agentInfo = agentAndStatus.getAgentInfo();
int serviceTypeCode = agentInfo.getServiceTypeCode();
if (!legacyAgent.contains(serviceTypeCode)) {
return false;
}
// String agentVersion = agentInfo.getAgentVersion();
// if (!legacyAgentVersion.contains(serviceTypeCode)) {
// return true;
// }
return true;
}

@Override
public ApplicationAgentHostList getApplicationAgentHostList(int offset, int limit, Period durationDays) {
Expand Down Expand Up @@ -303,8 +282,8 @@

List<AgentInfo> result = new ArrayList<>();
for (AgentInfo agentInfo : filteredAgentInfoList) {
String agentId = agentInfo.getAgentId();
if (isActiveAgent2(agentId, ranges)) {
Application agent = new Application(agentInfo.getAgentId(), agentInfo.getServiceType());

Check warning on line 285 in web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java#L285

Added line #L285 was not covered by tests
if (activeAgentValidator.isActiveAgent(agent, agentInfo.getAgentVersion(), ranges)) {
result.add(agentInfo);
}
}
Expand Down Expand Up @@ -351,13 +330,13 @@
// FIXME This needs to be done with a more accurate information.
// If at any time a non-java agent is introduced, or an agent that does not collect jvm data,
// this will fail
boolean dataExists = isActiveAgent(agentId, fastRange);
boolean dataExists = activeAgentValidator.isActiveAgent(agentId, fastRange);

Check warning on line 333 in web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java#L333

Added line #L333 was not covered by tests
if (dataExists) {
activeAgentIdList.add(agentId);
continue;
}

dataExists = isActiveAgent(agentId, queryRange);
dataExists = activeAgentValidator.isActiveAgent(agentId, queryRange);

Check warning on line 339 in web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java

View check run for this annotation

Codecov / codecov/patch

web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java#L339

Added line #L339 was not covered by tests
if (dataExists) {
activeAgentIdList.add(agentId);
}
Expand Down Expand Up @@ -509,37 +488,6 @@
return this.agentLifeCycleDao.getAgentStatus(query);
}

@Override
public boolean isActiveAgent(String agentId, Range range) {
Objects.requireNonNull(agentId, "agentId");
return isActiveAgentByGcStat(agentId, range) ||
isActiveAgentByPing(agentId, range);
}

public boolean isActiveAgent2(String agentId, Range range) {
Objects.requireNonNull(agentId, "agentId");
return isActiveAgentByPing(agentId, range) ||
isActiveAgentByGcStat(agentId, range);
}

private boolean isActiveAgent2(String agentId, List<Range> ranges) {
for (Range range : ranges) {
if (isActiveAgent2(agentId, range)) {
return true;
}
}
return false;
}

private boolean isActiveAgentByGcStat(String agentId, Range range) {
return this.jvmGcDao.agentStatExists(agentId, range);
}

private boolean isActiveAgentByPing(String agentId, Range range) {
return this.agentEventService.getAgentEvents(agentId, range)
.stream()
.anyMatch(e -> e.getEventTypeCode() == AgentEventType.AGENT_PING.getCode());
}

@Override
public InspectorTimeline getAgentStatusTimeline(String agentId, Range range, int... excludeAgentEventTypeCodes) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.navercorp.pinpoint.web.service.component;

import com.navercorp.pinpoint.common.server.util.time.Range;
import com.navercorp.pinpoint.web.vo.Application;

import java.util.List;

public interface ActiveAgentValidator {
boolean isActiveAgent(String agentId, Range range);

boolean isActiveAgent(Application agent, Range range);

boolean isActiveAgent(Application agent, String version, Range range);

boolean isActiveAgent(Application agent, String version, List<Range> ranges);

boolean isActiveAgentByPing(String agentId, Range range);
}
Loading